Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 12 May 2022 10:40:42 -0700
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        Lucas De Marchi <lucas.demarchi@intel.com>
Cc:     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()
Message-ID: <Yn1GmlWKIvuoJJby@bombadil.infradead.org>
References: <20220502140602.130373-1-Jason@zx2c4.com>
 <Ym/5EEYHbk56hV1H@zx2c4.com>
 <Ym/8GTW2RfhnbqiF@gardel-login>
 <YnERsPIsiOCa8cty@zx2c4.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YnERsPIsiOCa8cty@zx2c4.com>
Sender: Luis Chamberlain <mcgrof@infradead.org>
Precedence: bulk

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. :(

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?

  Luis