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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4A43B4A6.8050001@novell.com>
Date:	Thu, 25 Jun 2009 13:32:22 -0400
From:	Gregory Haskins <ghaskins@...ell.com>
To:	avi@...hat.com
CC:	Davide Libenzi <davidel@...ilserver.org>,
	Rusty Russell <rusty@...tcorp.com.au>, mst@...hat.com,
	kvm@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	paulmck@...ux.vnet.ibm.com, Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH 3/3] eventfd: add internal reference counting to fix notifier
 race conditions

Davide Libenzi wrote:
> On Thu, 25 Jun 2009, Rusty Russell wrote:
>
>   
>> On Thu, 25 Jun 2009 08:15:11 am Davide Libenzi wrote:
>>     
>>> Some components would like to know if userspace dropped the fd, and take
>>> proper action accordingly (release resources, drop module instances,
>>> etc...).
>>>       
>> Like to know?  Possibly.  Need to know?  Not anything I've seen so far.
>>
>> If userspace creates the fd, component grab a ref and if userspace wants that 
>> fd completely freed must close the fd *and* tell component.  Simple, race free 
>> and explicit.  All wins.
>>
>> As this discussion shows, doing some kind of implies non-reference is hard, 
>> complex and racy.
>>     
>
> Easier, we agree. Not doing anything is always easier, provided the 
> userspace interface allows for it.
> Cleaner, I'm not sure. Again, it depends from the userspace interface, but 
> usually when you close(2) something, you expect the kernel to react 
> accordingly, and not on relying on userspace issuing extra calls in order 
> to proper cleanup the kernel context.
> This is even more true when the eventfd is the sole handle to the visisble 
> userspace interface.
> In such cases, not taking proper action on close(2) and requiring extra 
> calls, would lead to designing interface with the close-no-really-i-mean-it
> patterns.
>   

Avi,
  Davide's argument is compelling in favor of considering the latest
irqfd fixes I pushed.  You could run into weirdness if we tried to
revert to the old requirement on explicit DEASSIGN ioctl once we start
doing things like handing the fd to other apps/components.  Its cleaner
IMO to retain the implicit-deassign-on-close behavior.

Since v5 also restores optional explicit DEASSIGN support, the only
downside to accepting my patches (vs reverting POLLHUP completely) is
the complexity required to deal with POLLHUP.  But this has been
substantially reduced and clarified since yesterdays review, and I am
reasonably confident the protocol is sound.

Please take a close look at it and consider for merging, if you would.

Thanks,
-Greg



Download attachment "signature.asc" of type "application/pgp-signature" (267 bytes)