Messages in this thread | | | Date | Wed, 24 Jan 2007 13:02:35 +0100 | From | Sébastien Dugué <> | Subject | Re: [PATCH -mm 5/5][AIO] - Add listio syscall support |
| |
On Tue, 23 Jan 2007 22:04:33 -0800 Andrew Morton <akpm@osdl.org> wrote:
> On Wed, 17 Jan 2007 10:55:54 +0100 > Sébastien Dugué <sebastien.dugue@bull.net> wrote: > > > +void lio_check(struct lio_event *lio) > > +{ > > + int ret; > > + > > + ret = atomic_dec_and_test(&lio->lio_users); > > + > > + if (unlikely(ret) && lio->lio_notify.notify != SIGEV_NONE) { > > + /* last one -> notify process */ > > + if (aio_send_signal(&lio->lio_notify)) > > + sigqueue_free(lio->lio_notify.sigq); > > + kfree(lio); > > + } > > +} > > That's a scary function. It may (or may not) free the memory at lio, > returning no indication to the caller whether or not that memory is still > allocated. This is most peculiar - are you really sure there's no > potential for a use-after-free here?
Right again, this patch definitely needs more eyes peering over.
> > The function is poorly named: I'd expect something called "foo_check" to > not have any side-effects. This one has gross side-effects. Want to think > up a better name, please? > > And given that this function has global scope, perhaps a little explanatory > comment is in order? > > > +struct lio_event *lio_create(struct sigevent __user *user_event, > > + int mode) > > Here too.
OK, will look into this. In the meantime, maybe you should drop this one patch entirely.
Thanks,
Sébastien. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |