mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sebastian Siewior <bigeasy@linutronix.de>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Jann Horn <jannh@google.com>, Theodore Ts'o <tytso@mit.edu>,
	LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] random: Fix signal_pending() usage
Date: Fri, 17 Jun 2022 18:48:42 +0200	[thread overview]
Message-ID: <YqywapDM7NPC/X+E@linutronix.de> (raw)
In-Reply-To: <CAHmME9pW6heXtPrfCP7J6ODgSc8sotsv6E3dnJoVBaPi+Ph=HA@mail.gmail.com>

On 2022-04-05 20:07:27 [+0200], Jason A. Donenfeld wrote:
> One funny aspect of the fact that signal_pending() hasn't worked right
> since the genesis commit is that this has probably led to a lot of
> userspace code that doesn't check the result from read() or
> getrandom(), and that code has worked mostly fine.

:)

> I wonder if we should do something about that. Worth noting is that
> we're no longer contending with /dev/random periodically blocking as
> the "entropy runs out" nonsense. I can think of two possible changes,
> which maybe aren't mutually exclusive:
> 
> 1) Turn signal_pending() into fatal_signal_pending() throughout the file.
> 2) Rather than not checking signal_pending() for reads of length <=
> 256, we could not check for signal_pending() for the first 256 bytes
> of any length read.
> 
> Both of these would be changing userspace behavior, so it should
> probably be considered carefully. Any thoughts?

You are not doing any blocking as far as I can tell so there won't be
any wake up via TASK_INTERRUPTIBLE for you here.
You check for the signal_pending() every PAGE_SIZE so there will be at
least 4KiB of data, not sure where this 256 is coming from.
Since you always return the number of bytes, there won't be any visible
change for requests < PAGE_SIZE. And for requests > PAGE_SIZE your
getrandom() invocation may return less than asked for. This is _now_.

If you drop that signal_pending() check then the user will always get
the number of bytes he asked for. Given that this is *quick* as in
no blocking, then there should be no harm in dropping this signal check.

> Jason

Sebastian

  reply	other threads:[~2022-06-17 16:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-05 16:39 Jann Horn
2022-04-05 18:07 ` Jason A. Donenfeld
2022-06-17 16:48   ` Sebastian Siewior [this message]
2022-06-17 22:47     ` Jason A. Donenfeld
2022-06-20  7:43       ` Sebastian Siewior
2022-06-20  8:14         ` Jason A. Donenfeld
2022-06-20 19:00         ` Linus Torvalds
2022-06-20 22:41           ` Jason A. Donenfeld
2022-06-21  7:03           ` Sebastian Siewior

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=YqywapDM7NPC/X+E@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=Jason@zx2c4.com \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®