linux-kernel - Re: [PATCH] random: Fix signal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YrAsgZC3iEHI+nu3@zx2c4.com>
Date:   Mon, 20 Jun 2022 10:14:57 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Sebastian Siewior <bigeasy@...utronix.de>
Cc:     Jann Horn <jannh@...gle.com>, Theodore Ts'o <tytso@....edu>,
        LKML <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] random: Fix signal_pending() usage

Hi Sebastian,

On Mon, Jun 20, 2022 at 09:43:56AM +0200, Sebastian Siewior wrote:
> > As for your suggestion to drop it entirely: that'd be nice, in that it'd
> > add a guarantee that currently doesn't exist. But it can lead to
> > somewhat large delays if somebody tries to read 2 gigabytes at a time
> > and hits Ctrl+C during it. That seems potentially bad?
> 
> So on my x86 box which runs a Debian kernel (based on v5.18.2):
> 
> | ~$ dd if=/dev/random of=/dev/null bs=2147483648 count=1
> | 0+1 records in
> | 0+1 records out
> | 2147479552 bytes (2,1 GB, 2,0 GiB) copied, 5,97452 s, 359 MB/s
> 
> almost 6 secs. On a smaller box it might take 12s or more. Your
> implementation change ensured that it does not block for unpredicted
> amount of time. Previously it would block until the random pool is
> filled with enough entropy and this could take an unforeseen amount of
> time. That read now makes more or less constant progress since it
> depends only on CPU time.
> Based on that, I don't see a problem dropping that signal check
> especially that requests larger than 4KiB are most likely exotic.

I don't have a huge objection to that, but I also don't really know
what's normal and expected behavior here. What do you make of the usage
of should_stop_iteration() in drivers/char/mem.c, for example? Would you
also remove that? If you send a patch to change this in random.c,
perhaps you should also change it in mem.c and elsewhere, so that some
broader consensus forms (or doesn't form) on what the expected behavior
is.

Jason