On June 14, 2014 1:13:56 PM CEST, 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? > > >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))
Where else could howmany come from? Can you be sure it's safe to use here? /Alexander > #define powerof2(x) ((((x)-1)&(x))==0) > > /* Macros for min/max. */
