2.6.37 patch and kmalloc() looks broken

[arekm]

kmalloc() can be called this way:

sl->data = kmalloc(slsize = sl->len+1, GFP_KERNEL);

which causes:

linux-2.6.37/drivers/staging/autofs/root.c:311:2: error: lvalue required as left operand of assignment

and likely doesn't work as autor intented for constructs like:

linux-2.6.37/arch/ia64/sn/kernel/bte.c: bteBlock_unaligned = kmalloc(len + 3 * L1_CACHE_BYTES, GFP_KERNEL);

Looks like kmalloc* macros are broken, I hope this fix is correct in all aspects:

Code: Select all

--- linux-2.6.37/include/linux/slab.h~  2011-01-17 11:48:00.934382737 +0100
+++ linux-2.6.37/include/linux/slab.h   2011-01-17 12:38:01.843508841 +0100
@@ -344,7 +344,7 @@
 #define kmalloc(x, y)                                  \
 ({                                                     \
        void *___retval;                                \
-       intoverflow_t ___x = (intoverflow_t)x;          \
+       intoverflow_t ___x = (intoverflow_t)(x);                \
        if (WARN(___x > ULONG_MAX, "kmalloc size overflow\n"))\
                ___retval = NULL;                       \
        else                                            \
@@ -355,7 +355,7 @@
 #define kmalloc_node(x, y, z)                                  \
 ({                                                             \
        void *___retval;                                        \
-       intoverflow_t ___x = (intoverflow_t)x;                  \
+       intoverflow_t ___x = (intoverflow_t)(x);                        \
        if (WARN(___x > ULONG_MAX, "kmalloc_node size overflow\n"))\
                ___retval = NULL;                               \
        else                                                    \
@@ -366,7 +366,7 @@
 #define kzalloc(x, y)                                  \
 ({                                                     \
        void *___retval;                                \
-       intoverflow_t ___x = (intoverflow_t)x;          \
+       intoverflow_t ___x = (intoverflow_t)(x);                \
        if (WARN(___x > ULONG_MAX, "kzalloc size overflow\n"))\
                ___retval = NULL;                       \
        else                                            \


[PaX Team]

sl->data = kmalloc(slsize = sl->len+1, GFP_KERNEL);

which causes:

linux-2.6.37/drivers/staging/autofs/root.c:311:2: error: lvalue required as left operand of assignment

the correct solution here is to move the assignment out of the kmalloc parameter.

and likely doesn't work as autor intented for constructs like:

linux-2.6.37/arch/ia64/sn/kernel/bte.c: bteBlock_unaligned = kmalloc(len + 3 * L1_CACHE_BYTES, GFP_KERNEL);

the macros work exactly as intended in this case . it's probably worth a blog post on its own, but the quick description is this: we want to detect integer overflows in expressions used as *alloc size arguments (think sizeof(something()*user_supplied_size). the correct solution would be to teach the compiler to do it, the poor man's solution is to slightly abuse the C precedence rules and type casting so that when the compiler sees the typecast, it'll promote the leftmost part of the expression to the special intoverflow_t type and that in turn will make the expression be computed in that type as well. so if we have int/long types in the expression, we'll detect any multiplication overflow since the resulting type is twice the size of long. i hope you can now see why your supposed 'fix' would completely kill the purpose of these macros .