Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 728580 - sys-apps/portage: _writer coroutine has unsafe remove_writer call in finally block
Summary: sys-apps/portage: _writer coroutine has unsafe remove_writer call in finally ...
Status: CONFIRMED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core (show other bugs)
Hardware: All All
: Normal normal (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: PATCH
Depends on:
Blocks:
 
Reported: 2020-06-17 16:26 UTC by Zac Medico
Modified: 2020-06-18 18:06 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zac Medico gentoo-dev 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 gentoo-dev 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 3 Larry the Git Cow gentoo-dev 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 <zmedico@gentoo.org>
AuthorDate: 2020-06-18 05:33:26 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
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 <dolsen@gentoo.org>
    Signed-off-by: Zac Medico <zmedico@gentoo.org>

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