linux-kernel - Re: [PATCH v4 0/3] random: convert to using iters, for Al Viro

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]

Date:   Fri, 20 May 2022 09:51:06 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     Theodore Ts'o <tytso@....edu>, Christoph Hellwig <hch@....de>,
        LKML <linux-kernel@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH v4 0/3] random: convert to using iters, for Al Viro

On 5/20/22 9:46 AM, Jason A. Donenfeld wrote:
> Hi Jens,
> 
> On Fri, May 20, 2022 at 09:34:46AM -0600, Jens Axboe wrote:
>> On 5/20/22 9:25 AM, Jason A. Donenfeld wrote:
>>> Are we sure we really want to do this and need to do this?
>>
>> I'm very sure, otherwise we're just accepting that we're breaking real
>> world applications.
> 
> Would we really? I always thought splice() and copy_file_range() and
> sendfile() were all kind of "special" in that they mostly do not work
> for many things, and so all userspaces need fallback code. And the state
> of "mostly not working" has always just been the norm. So broken today,
> working tomorrow, broken next week would be par for the course? I might
> be *super* wrong here, so feel free to say so, but this has been my
> general impression.

No, I think that is exactly the wrong impression. If you have an
application that is written using eg splice from /dev/urandom, then it
should indeed be safe to expect that it will indeed continue working. If
we have one core tenet in the kernel it's that you should ALWAYS be able
to upgrade your kernel and not have any breakage in terms of userspace
ABI. Obviously that can happen sometimes, but I think this one is
exactly the poster child of breakage that should NOT happen. We took
away a feature that someone depended on.

This situation of splice being flakily availably is new from when it was
decided that it was OK to only have it be available if ->read_iter() and
->splice_read() is set. And that is very unfortunate.

> Anyway, I do like the idea of supporting splice() and sendfile(). The
> performance hit is just kind of sad.

I like the idea too, but this is deeper than that. We simply cannot just
break existing use cases like that.

Thankfully we do have an option here as I outlined in the previous email
- keep the ->read() and just add ->read_iter() on the side so that
splice still works. Is that ideal? No, it needs more code to support.
But hopefully that can die at some point when the performance gap is
such that we no longer need to worry about having ->read() for those
cases.

-- 
Jens Axboe