Re: select(0, ..) is valid ?

From: Andrew Morton
Date: Tue May 15 2007 - 14:30:55 EST


On Tue, 15 May 2007 11:10:22 -0700 (PDT)
Christoph Lameter <clameter@xxxxxxx> wrote:

> On Tue, 15 May 2007, Andrew Morton wrote:
>
> > I _think_ we can just do
> >
> > --- a/fs/compat.c~a
> > +++ a/fs/compat.c
> > @@ -1566,9 +1566,13 @@ int compat_core_sys_select(int n, compat
> > */
> > ret = -ENOMEM;
> > size = FDS_BYTES(n);
> > - bits = kmalloc(6 * size, GFP_KERNEL);
> > - if (!bits)
> > - goto out_nofds;
> > + if (likely(size)) {
> > + bits = kmalloc(6 * size, GFP_KERNEL);
> > + if (!bits)
> > + goto out_nofds;
> > + } else {
> > + bits = NULL;
> > + }
> > fds.in = (unsigned long *) bits;
> > fds.out = (unsigned long *) (bits + size);
> > fds.ex = (unsigned long *) (bits + 2*size);
> > _
> >
> > I mean, if that oopses then I'd be very interested in finding out why.
> >
> > But I'm starting to suspect that it would be better to permit kmalloc(0) in
> > slub. It depends on how many more of these things need fixing.
> >
> > otoh, a kmalloc(0) could be a sign of some buggy/inefficient/weird code, so
> > there's some value in forcing us to go look at all the callsites.
>
> Hmmm... We could have kmalloc(0) return a pointer to the zero page? That
> would catch any writers?

Returning NULL would have the same effect..

But the problem is that we won't get 100% coverage of all codepaths
for ages, so any oopses we added won't get found.

otoh, any code which does dereference that pointer is buggy anwyay.

The problem here is that code which does

kmalloc(some-expression-which-returns-0)

will go and assume that the kmalloc(0) got an ENOMEM and it'll take the
error path.

Oh well, let's persist with things as they now are.

Perhaps putting a size=0 detector into slab also would speed this
process up.
-
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/