linux-kernel - Re: [PATCH] random: add fork_event sysctl for polling VM forks

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: <CAG48ez2X72XkpxaEDmzykewreuhk8=5t5L5b2Qdr1dn8LcFutw@mail.gmail.com>
Date:   Tue, 19 Apr 2022 21:44:31 +0200
From:   Jann Horn <jannh@...gle.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Alexander Graf <graf@...zon.com>,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Theodore Ts'o" <tytso@....edu>,
        Colm MacCarthaigh <colmmacc@...zon.com>
Subject: Re: [PATCH] random: add fork_event sysctl for polling VM forks

On Tue, Apr 19, 2022 at 6:42 PM Jason A. Donenfeld <Jason@...c4.com> wrote:
> Hey Jann,
>
> On Tue, Apr 19, 2022 at 6:38 PM Jann Horn <jannh@...gle.com> wrote:
> > This is a bit of a weird API, because normally .poll is supposed to be
> > level-triggered rather than edge-triggered... and AFAIK things like
> > epoll also kinda assume that ->poll() doesn't modify state (but that
> > only _really_ matters in weird cases). But at the same time, it looks
> > like the existing proc_sys_poll() already goes against that? So I
> > don't know what the right thing to do there is...
>
> Doesn't the level vs edge distinction apply to POLLIN/POLLOUT events?

I don't see why it would be limited to that.

> In this case, the event generated is actually POLLERR. On one hand,
> this is sort of weird. On the other hand, it perhaps makes sense,
> since nothing changes respect to its readability/writeability. And it
> also happens to be how the sysctl poll() infrastructure was designed;
> I didn't need to change anything for this behavior, and it comes as a
> result of this rather trivial commit only. Looking at where else it's
> used, it appears to be the intended use case for changes to
> hostname/domainname. So while it's unusual, it also appears to be the
> usual way that sysctl poll() works. So perhaps we're quite lucky here
> in that sysctl poll() winds up being the correct interface for what we
> want?

AFAIK this also means that if you make an epoll watch for
/proc/sys/kernel/random/fork_event, and then call poll() *on the epoll
fd* for some reason, that will probably already consume the event; and
if you then try to actually receive the epoll event via epoll_wait(),
it'll already be gone (because epoll tries to re-poll the "ready"
files to figure out what state those files are at now). Similarly if
you try to create an epoll watch for an FD that already has an event
pending: Installing the watch will call the ->poll handler once,
resetting the file's state, and the following epoll_wait() will call
->poll again and think the event is already gone. See the call paths
to vfs_poll() in fs/eventpoll.c.

Maybe we don't care about such exotic usage, and are willing to accept
the UAPI inconsistency and slight epoll breakage of plumbing
edge-triggered polling through APIs designed for level-triggered
polling. IDK.