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

Jamie Lokier
2008-08-27T16:21:47Z

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