Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <69bd18e6-d216-dfb3-201b-f6a285deb0e7@kernel.dk>
Date:   Fri, 20 May 2022 10:24:36 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH v4 0/3] random: convert to using iters, for Al Viro
Content-Language: en-US
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     "Jason A. Donenfeld" <Jason@zx2c4.com>,
        Theodore Ts'o <tytso@mit.edu>, Christoph Hellwig <hch@lst.de>,
        LKML <linux-kernel@vger.kernel.org>
References: <20220520094459.116240-1-Jason@zx2c4.com>
 <Yoey+FOYO69lS5qP@zx2c4.com> <bbdeeca7-9c99-e661-6596-f04ea783a30b@kernel.dk>
 <Yoe4HP9iTA6l/For@zeniv-ca.linux.org.uk>
 <7e78d6a7-1cc3-2408-fe91-0329f5020e77@kernel.dk>
 <Yoe+lK8RIRbK6lDZ@zeniv-ca.linux.org.uk>
From:   Jens Axboe <axboe@kernel.dk>
In-Reply-To: <Yoe+lK8RIRbK6lDZ@zeniv-ca.linux.org.uk>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/20/22 10:15 AM, Al Viro wrote:
> On Fri, May 20, 2022 at 09:53:30AM -0600, Jens Axboe wrote:
>> On 5/20/22 9:47 AM, Al Viro wrote:
>>> On Fri, May 20, 2022 at 09:34:46AM -0600, Jens Axboe wrote:
>>>
>>>> I'm very sure, otherwise we're just accepting that we're breaking real
>>>> world applications.
>>>
>>> "Breaking" as in "it used to work with earlier kernels, doesn't work with
>>> recent ones"?  Details, please...
>>
>> Yes, as in exactly that. This is what drove this addition of
>> ->read_iter() for urandom. See commit:
>>
>> ommit 36e2c7421f02a22f71c9283e55fdb672a9eb58e7
>> Author: Christoph Hellwig <hch@lst.de>
>> Date:   Thu Sep 3 16:22:34 2020 +0200
>>
>>     fs: don't allow splice read/write without explicit ops
>>
>> related to the set_fs() changes, and now go look for any commit that
>> has:
>>
>> Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
>>
>> in it and see that this isn't an isolated incident at all.
>>
>> tldr - splice from /dev/urandom used to work, and I recently got a
>> report internally on an application that broke on upgrade from 5.6 to
>> 5.12 exactly because it now just just -EINVAL's instead.
> 
> IIRC, Linus' position at the time had been along the lines of
> "splice is not so good ABI anyway, so let's do it and fix up
> the places that do get real-world complaints once such appear".
> So /dev/urandom is one such place...

That's what Christoph said too. Honestly that's a very odd way to
attempt to justify breakage like this, even if it is tempting to
facilitate the set_fs() removal. But then be honest about it and say
it like it is, rather than some hand wavy explanation that frankly
doesn't make any sense.

The referenced change doesn't change the splice ABI at all, hence the
justification seems very random to me. It kept what we already have,
except we randomly break some use cases.

-- 
Jens Axboe