On Sat, Jun 14, 2014 at 7:13 AM, Tobias Stoeckmann <tob...@stoeckmann.org> wrote:
> Hi, > > the howmany macro as used in param.h and select.h is prone to an integer > overflow. It adds divisor-1 to the base value, which means that it > COULD overflow. > > Most of the times, the howmany macro is used with file descriptors and > polling, would be hard to overflow it. Especially due to C language > specifications and implicit casts. But there are other use cases... > > In setup.c of fsck_ffs, howmany is used to calculate the required size > to hold a block map for the file system to be checked: > > bmapsize = roundup(howmany(maxfsblock, NBBY), sizeof(int16_t)); > blockmap = calloc((unsigned)bmapsize, sizeof(char)); > > The value of maxfsblock is read from superblock and is 64 bit, adding 7 > can let it overflow which means bmapsize becomes 0 and later access to > the blockmap will result in out of boundary access. (FYI: fsck_ffs > would fail after calloc without bmapsize's overflow, map would be too > large for memory.) > > I chose the fsck_ffs example because it's easier to modify a filesystem > to be "broken" compared to having a storage that is large enough. ;) > > This diff only adjusts param.h and is merely an idea I would like to > discuss. Using a modulo operation, the overflow can be prevented. > But I don't know how significant this operation will be when it comes > to performance. Or if howmany should not be used for these operations > at all. > > Thoughts, anyone? > > Condition are bad. did you check the usage of it to see if x and y could be bigger than the half of maxint in the code ? #define howmany(x, y) ((x/y) + (y-1)/y) > > Tobias > > Index: param.h > =================================================================== > RCS file: /cvs/src/sys/sys/param.h,v > retrieving revision 1.106 > diff -u -p -r1.106 param.h > --- param.h 2 May 2014 19:03:06 -0000 1.106 > +++ param.h 14 Jun 2014 10:33:06 -0000 > @@ -207,9 +207,9 @@ > > /* Macros for counting and rounding. */ > #ifndef howmany > -#define howmany(x, y) (((x)+((y)-1))/(y)) > +#define howmany(x, y) ((x)/(y)+((x)%(y)?1:0) > #endif > -#define roundup(x, y) ((((x)+((y)-1))/(y))*(y)) > +#define roundup(x, y) (howmany((x),(y))*(y)) > #define powerof2(x) ((((x)-1)&(x))==0) > > /* Macros for min/max. */ > > -- --------------------------------------------------------------------------------------------------------------------- () ascii ribbon campaign - against html e-mail /\
