Re: [Bug #11342] Linux 2.6.27-rc3: kernel BUG at mm/vmalloc.c - bisected

[Jamie Lokier]

Linus Torvalds wrote:
> > Most LOCs of the kernel are not written by people like you or Al Viro or 
> > David Miller, and the average kernel developer is unlikely to do it as 
> > good as gcc.
> 
> Sure. But we do have tools. We do have checkstack.pl, it's just that it 
> hasn't been an issue in a long time, so I suspect many people didn't even 
> _realize_ we have it, and I certainly can attest to the fact that even 
> people who remember it - like me - don't actually tend to run it all that 
> often.

Sounds like what's really desired here isn't more worry and
unpredictability, but for GCC+Binutils to gain the ability to
calculate the stack depth over all callchains (doesn't have to be
exact, just an upper bound; annotate recursions) in a way that's good
enough to do on every compile, complain if a depth is exceeded
statically (or it can't be proven), and to gain the
architecture-independent option "optimise to reduce stack usage".

> > BTW:
> > I just ran checkstack on a (roughly) allyesconfig kernel, and we have a 
> > new driver that allocates "unsigned char recvbuf[1500];" on the stack...
> 
> Yeah, it's _way_ too easy to do bad things.

In my userspace code, I have macros tmp_alloc and tmp_free.  They must
be matched in the same function:

     unsigned char * recvbuf = tmp_alloc(1500);
     ....
     tmp_free(recvbuf);

When stack is plentiful, it maps to alloca() which is roughly
equivalent to using a stack variable.

When stack is constrained (as it is on my little devices), that maps
to xmalloc/free.  The kernel equivalent would be kmalloc GFP_ATOMIC
(perhaps).

With different macros to mine, it may be possible to map small
fixed-size requests exactly onto local variables, and large ones to
kmalloc().  A stab at it (not tested):

    #define LOCAL_ALLOC_THRESHOLD     128

    #define LOCAL_ALLOC(type, ptr)                                        \
        __typeof__(type) __attribute__((__unused__)) ptr##_local_struct;  \
        __typeof__(type) * ptr =                                          \
              ((__builtin_constant_p(sizeof(type))                        \
                && sizeof(type) <= LOCAL_ALLOC_THRESHOLD)                 \
               ? &ptr##_local_struct : kmalloc(sizeof(type), GFP_ATOMIC))

    #define LOCAL_FREE(ptr)                           \
        ((__builtin_constant_p(sizeof (*(ptr)))       \
          && sizeof(*(ptr)) <= LOCAL_ALLOC_THRESHOLD) \
         ? (void) 0 : kfree(ptr))

Would that be useful in the kernel?

I'm thinking if it were a commonly used pattern for temporary buffers,
unknown structures and arrays of macro-determined size, the "new
driver" author would be less likely to accidentally drop a big object
on the stack.

Obviously it would be nicer for GCC to code such a thing
automatically, but that really is wishful thinking.

-- Jamie
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/