From: Greg KH <greg@kroah.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Al Viro <viro@zeniv.linux.org.uk>,
stable <stable@vger.kernel.org>
Subject: Re: [PATCH v2] select: Fix indefinitely sleeping task in poll_schedule_timeout()
Date: Tue, 11 Jan 2022 19:00:38 +0100 [thread overview]
Message-ID: <Yd3Fxn22YA1cLpme@kroah.com> (raw)
In-Reply-To: <CAHk-=wj39rpqNZX99dJUpErT+yX9aZN-Z1Lyfx8tbUqFUFeEqw@mail.gmail.com>
On Tue, Jan 11, 2022 at 09:12:08AM -0800, Linus Torvalds wrote:
> On Mon, Jan 10, 2022 at 10:19 AM Jan Kara <jack@suse.cz> wrote:
> >
> > A task can end up indefinitely sleeping in do_select() ->
> > poll_schedule_timeout() when the following race happens:
> > {...]
>
> Ok, I decided to just take this as-is right now, and get it in early
> in the merge window, and see if anybody hollers.
>
> I don't think the stable people will try to apply it until after the
> merge window closes anyway, but it's worth pointing out that this
> change (commit 68514dacf271: "select: Fix indefinitely sleeping task
> in poll_schedule_timeout()" in my tree now) is very much a change of
> behavior, and we may have to revert it if it causes any issues.
>
> The most likely issue it would cause is that some program uses
> select() with an fd mask with extra garbage in it, and stale fd bits
> that pointed to closed file descriptors used to just be ignored. Now
> they'll cause select() to return immediately with those bits set.
>
> And that might then cause a program to perhaps still work, but
> busy-spin on select(), wasting CPU time. Or it will walk the result
> bits, see them set, try to read/write to them, get EBADF, and clear
> them. Or not clear them and just be very unhappy indeed.
>
> So while I think this version of the patch is still safer than the
> EBADF one - and I think better semantics that happen to match poll()
> too - I think this is a patch that could expose existing bad user
> space.
>
> We'll see. I considered adding a WARN_ON_ONCE() just to make the
> change in behavior more visible, but ended up not really feeling it.
>
> End result: I took this patch eagerly not because I was happy to do
> it, but simply because the earlier we test this, the earlier we'll
> know of any problems. Let's hope there are none.
Thanks for the heads-up for the stable tree relevance, I'll watch out
for this one.
greg k-h
next prev parent reply other threads:[~2022-01-11 18:00 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-10 18:19 [PATCH v2] select: Fix indefinitely sleeping task in poll_schedule_timeout() Jan Kara
2022-01-11 17:12 ` Linus Torvalds
2022-01-11 18:00 ` Greg KH [this message]
2022-01-11 21:27 ` Jan Kara
2022-01-11 21:43 ` Linus Torvalds
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=Yd3Fxn22YA1cLpme@kroah.com \
--to=greg@kroah.com \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).