Re: [PATCH 3/3] eventfd: add internal reference counting to fix notifier race conditions

Davide Libenzi
2009-06-24T22:50:39Z

On Wed, 24 Jun 2009, Rusty Russell wrote:

> On Tue, 23 Jun 2009 03:33:22 am Davide Libenzi wrote:
> > What you're doing there, is setting up a kernel-to-kernel (since
> > userspace only role is to create the eventfd) communication, using a file*
> > as accessory. That IMO is plain wrong.
>
> The most sensible is that userspace can use these fds; an in-kernel variant is
> possible too, but not primary IMHO.
>
> It's nice that userspace create the fds; it can then use the same fd for
> multiple event sources.
>
> But I didn't see anything wrong with the way eventfd used to work: you have a
> kvm ioctl to say "attach this eventfd to this guest notification" and that does
> the eventfd_fget. A detach ioctl does the fput (as does release of the kvm
> fd).
>
> If they close the eventfd and don't do the detach ioctl, it's their problem.

Some components would like to know if userspace dropped the fd, and take
proper action accordingly (release resources, drop module instances, etc...).
The POLLHUP helps with that, but w/out decoupling file* memory from
eventfd_ctx memory, it becomes pretty tricky (if feasible at all) to
handle the event in a race-free way.
Once the file* is decoupled from the eventfd_ctx, it becomes saner to
expose the internal kernel API via the eventfd_ctx.
Another thing that comes in my mind (that for some components might not
matter) is considering the effect of userspace doing things like:

for (;;) {
fd = eventfd(...);
ioctl(xfd, XXX_ADD, fd);
close(fd);
}

That might lead to unprivileged users drawing kernel memory w/out any
userspace accountability, if not properly handled.

- Davide