All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jann Horn <jannh@google.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	Alexander Graf <graf@amazon.com>,
	Dominik Brodowski <linux@dominikbrodowski.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Colm MacCarthaigh <colmmacc@amazon.com>
Subject: Re: [PATCH] random: add fork_event sysctl for polling VM forks
Date: Tue, 19 Apr 2022 21:44:31 +0200	[thread overview]
Message-ID: <CAG48ez2X72XkpxaEDmzykewreuhk8=5t5L5b2Qdr1dn8LcFutw@mail.gmail.com> (raw)
In-Reply-To: <CAHmME9r7Vt1XFzceHhy7O67iVMhtpLJ-d0p8UGgV4Srd4Dt2Hg@mail.gmail.com>

On Tue, Apr 19, 2022 at 6:42 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hey Jann,
>
> On Tue, Apr 19, 2022 at 6:38 PM Jann Horn <jannh@google.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.

  reply	other threads:[~2022-04-19 19:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-19 16:04 [PATCH] random: add fork_event sysctl for polling VM forks Jason A. Donenfeld
2022-04-19 16:37 ` Jann Horn
2022-04-19 16:42   ` Jason A. Donenfeld
2022-04-19 19:44     ` Jann Horn [this message]
2022-04-20  0:15       ` Jason A. Donenfeld
2022-04-20 13:24         ` Jason A. Donenfeld
2022-04-20 14:23           ` Jann Horn
2022-04-20 16:37             ` Jason A. Donenfeld
2022-04-21 13:35 ` [PATCH v2] " Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAG48ez2X72XkpxaEDmzykewreuhk8=5t5L5b2Qdr1dn8LcFutw@mail.gmail.com' \
    --to=jannh@google.com \
    --cc=Jason@zx2c4.com \
    --cc=colmmacc@amazon.com \
    --cc=graf@amazon.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@dominikbrodowski.net \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.