linux-kernel - Re: [GIT PULL] pipe: nonblocking rw for io

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:   Mon, 24 Apr 2023 16:07:21 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Christian Brauner <brauner@...nel.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] pipe: nonblocking rw for io_uring

On 4/24/23 3:58?PM, Linus Torvalds wrote:
> On Mon, Apr 24, 2023 at 2:37?PM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> And I completely refuse to add that trylock hack to paper that over.
>> The pipe lock is *not* meant for IO.
> 
> If you want to paper it over, do it other ways.
> 
> I'd love to just magically fix splice, but hey, that might not be possible.

Don't think it is... At least not trivially.

> But possible fixes papering this over might be to make splice "poison
> a pipe, and make io_uring falls back on io workers only on pipes that
> do splice. Make any normal pipe read/write load sane.
> 
> And no, don't worry about races. If you have the same pipe used for
> io_uring IO *and* somebody else then doing splice on it and racing,
> just take the loss and tell people that they might hit a slow case if
> they do stupid things.
> 
> Basically, the patch might look like something like
> 
>  - do_pipe() sets FMODE_NOWAIT by default when creating a pipe
> 
>  - splice then clears FMODE_NOWAIT on pipes as they are used
> 
> and now io_uring sees whether the pipe is playing nice or not.
> 
> As far as I can tell, something like that would make the
> 'pipe_buf_confirm()' part unnecessary too, since that's only relevant
> for splice.
> 
> A fancier version might be to only do that "splice then clears
> FMODE_NOWAIT" thing if the other side of the splice has not set
> FMODE_NOWAIT.
> 
> Honestly, if the problem is "pipe IO is slow", then splice should not
> be the thing you optimize for.

I think that'd be an acceptable approach, and would at least fix the
pure pipe case which I suspect is 99.9% of them, if not more. And yes,
it'd mean that we don't need to do the ->confirm() change either, as the
pipe is already tainted at that point.

I'll respin a v2, post, and send in later this merge window.

-- 
Jens Axboe