Re: [PATCH 1/2] sysctl: read() must consume poll events, not poll()

From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	Lennart Poettering <mzxreary@0pointer.de>,
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org
Subject: Re: [PATCH 1/2] sysctl: read() must consume poll events, not poll()
Date: Thu, 12 May 2022 13:29:04 -0500	[thread overview]
Message-ID: <87bkw2hafj.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <Yn1GmlWKIvuoJJby@bombadil.infradead.org> (Luis Chamberlain's message of "Thu, 12 May 2022 10:40:42 -0700")

Luis Chamberlain <mcgrof@kernel.org> writes:

> On Tue, May 03, 2022 at 01:27:44PM +0200, Jason A. Donenfeld wrote:
>> On Mon, May 02, 2022 at 05:43:21PM +0200, Lennart Poettering wrote:
>> > On Mo, 02.05.22 17:30, Jason A. Donenfeld (Jason@zx2c4.com) wrote:
>> > 
>> > > Just wanted to double check with you that this change wouldn't break how
>> > > you're using it in systemd for /proc/sys/kernel/hostname:
>> > >
>> > > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/journal/journald-server.c#L1832
>> > > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/resolve/resolved-manager.c#L465
>> > >
>> > > I couldn't find anybody else actually polling on it. Interestingly, it
>> > > looks like sd_event_add_io uses epoll() inside, but you're not hitting
>> > > the bug that Jann pointed out (because I suppose you're not poll()ing on
>> > > an epoll fd).
>> > 
>> > Well, if you made sure this still works, I am fine either way ;-)
>> 
>> Actually... ugh. It doesn't work. systemd uses uname() to read the host
>> name, and doesn't actually read() the file descriptor after receiving
>> the poll event on it. So I guess I'll forget this, and maybe we'll have
>> to live with sysctl's poll() being broken. :(

We should be able to modify calling uname() to act the same as reading
the file descriptor. 

> A kconfig option may let you do what you want, and allow older kernels
> to not break, however I am more curious how sysctl's approach to poll
> went unnnoticed for so long. But also, I'm curious if it was based on
> another poll implementation which may have been busted.
>
> But more importantly, how do we avoid this in the future?

Poll on files is weird and generally doesn't work (because files are
always read to read or write).  What did we do to make it work on these
sysctl files?

Eric