Re: [Bug-gnulib] linebreak.c proposed patches for size-calculation overflows

[Bruno Haible]

Paul Eggert wrote:
> >              bool overflow = false;
> >              size_t size = xsum3 (xtimes (n, sizeof (size_t), &overflow),
> >                                   m,
> >                                   m,
> >                                   &overflow);
> >              if (!overflow)
> >                {
> >                  char *memory = malloc (size);
>
> I'd rather reserve (size_t) -1 to represent overflow

Yes, that's a good idea.

> Currently linebreak.c doesn't include xalloc.h, but if you don't mind
> depending on xalloc.h

linebreak.c doesn't want to depend on xalloc.h, since it doesn't want
to call xalloc_die() in case of memory allocation failure. (Not all
programs like this.) And the size_t overflow checking is actually a
different facility than xmalloc(), therefore I think it deserves its own
.h file.

> +      if (n <= (size_t)(-1) / (sizeof (size_t) + 2 * MB_LEN_MAX)

This way is not the right path: the code becomes ununderstandable.
(In this case, I even doubt the correctness: why MB_LEN_MAX? MB_LEN_MAX
is 6 in glibc, but iconv() can also apply to encodings, like ISO-2022-JP-1,
where a single character can lead to 8 bytes or more.)

I propose a new header file, like this.

Bruno

============================= xsize.h =============================
/* xsize.h -- Checked size_t computations.

   Copyright (C) 2003 Free Software Foundation, Inc.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2, or (at your option)
   any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software Foundation,
   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */

#ifndef _XSIZE_H
#define _XSIZE_H

#include <stddef.h>

#ifndef SIZE_MAX
# define SIZE_MAX ((size_t) -1)
#endif

/* The size of memory objects is often computed through expressions of
   type size_t. Example:
      void* p = malloc (header_size + n * element_size).
   These computations can lead to overflow.  When this happens, malloc()
   returns a piece of memory that is way too small, and the program then
   crashes while attempting to fill the memory.
   To avoid this, the functions and macros in this file check for overflow.
   The convention is that SIZE_MAX represents overflow.
   malloc (SIZE_MAX) is not guaranteed to fail -- think of a malloc
   implementation that uses mmap --, it's recommended to use SIZE_OVERFLOW_P
   before invoking malloc().
   The example thus becomes:
      size_t size = xsum (header_size, xtimes (n, element_size));
      void *p = (!SIZE_OVERFLOW_P (size) ? malloc (size) : NULL);
*/

/* Sum of two sizes, with overflow check.  */
static inline size_t
xsum (size_t size1, size_t size2)
{
  size_t sum = size1 + size2;
  return (sum >= size1 ? sum : SIZE_MAX);
}

/* Sum of three sizes, with overflow check.  */
static inline size_t
xsum3 (size_t size1, size_t size2, size_t size3)
{
  return xsum (xsum (size1, size2), size3);
}

/* Sum of four sizes, with overflow check.  */
static inline size_t
xsum4 (size_t size1, size_t size2, size_t size3, size_t size4)
{
  return xsum (xsum (xsum (size1, size2), size3), size4);
}

/* Multiplication of a count with an element size, with overflow check.
   The count must be >= 0 and the element size must be > 0.
   This is a macro, not an inline function, so that it works correctly even
   when N is of a wider tupe and N > SIZE_MAX.  */
#define xtimes(N, ELSIZE) \
  ((N) <= SIZE_MAX / (ELSIZE) ? (size_t) (N) * (ELSIZE) : SIZE_MAX)

/* Check for overflow.  */
#define SIZE_OVERFLOW_P(SIZE) \
  ((SIZE) == SIZE_MAX)

#endif /* _XSIZE_H */