Bug 728580 – sys-apps/portage: _writer coroutine has unsafe remove_writer call in finally block

• Product: Portage Development
• Status: CONFIRMED ---

[Description Zac Medico 2020-06-17 16:26:17 UTC]

It's unsafe to call remove_writer in a coroutine finally block, since the finally block can execute after the file descriptor has been closed and reopened for another purpose.

We've got this problem in portage.util.futures._asyncio.streams._writer coroutine.

[Comment 1 Zac Medico 2020-06-17 19:30:23 UTC]

In https://idea.popcount.org/2017-03-20-epoll-is-fundamentally-broken-22/ I've found this potentially relevant note about unregistration of closed file descriptors:

> epoll_ctl(EPOLL_CTL_ADD) doesn't actually register a file
> descriptor. Instead it registers a tuple1 of a file descriptor
> and a pointer to underlying kernel object. Most confusingly the
> lifetime of an epoll subscription is not tied to the lifetime of
> a file descriptor. It's tied to the life of the kernel object.
> 
> Due to this implementation quirk calling close() on a file
> descriptor might or might not trigger epoll unsubscription. If
> the close call removes the last pointer to kernel object
> and causes the object to be freed, then it will cause epoll
> subscription cleanup. But if there are more pointers to kernel
> object, more file descriptors, in any process on the system,
> then close will not cause the epoll subscription cleanup. It
> is totally possible to receive events on previously closed
> file descriptors.

[Comment 2 Zac Medico 2020-06-18 07:42:48 UTC]

Patch posted for review:

https://archives.gentoo.org/gentoo-portage-dev/message/bd3968b7cc42d7be415adcd7303e9293
https://github.com/gentoo/portage/pull/560

[Comment 3 Larry the Git Cow 2020-06-18 18:06:34 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=92be5a02e452eb0810d2974bc7fa5ee2056ef8e7

commit 92be5a02e452eb0810d2974bc7fa5ee2056ef8e7
Author:     Zac Medico <[e-mail]>
AuthorDate: 2020-06-18 05:33:26 +0000
Commit:     Zac Medico <[e-mail]>
CommitDate: 2020-06-18 17:54:37 +0000

    _writer: fix unsafe finally clause (bug 728580)
    
    In the coroutine finally clause, do not call remove_writer in cases
    where fd has been closed and then re-allocated to a concurrent
    coroutine as in bug 716636.
    
    Also, assume that the caller will put the file in non-blocking mode
    and close the file when done, so that this function is suitable for
    use within a loop.
    
    Bug: https://bugs.gentoo.org/728580
    Reviewed-by: Brian Dolbec <[e-mail]>
    Signed-off-by: Zac Medico <[e-mail]>

 lib/portage/util/futures/_asyncio/process.py | 11 ++++--
 lib/portage/util/futures/_asyncio/streams.py | 50 +++++++++++++---------------
 2 files changed, 33 insertions(+), 28 deletions(-)