linux-kernel - Re: [PATCH] pipe: honor IOCB

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: <20200911011205.GG1236603@ZenIV.linux.org.uk>
Date:   Fri, 11 Sep 2020 02:12:05 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pipe: honor IOCB_NOWAIT

On Mon, Sep 07, 2020 at 09:21:02AM -0600, Jens Axboe wrote:
> Pipe only looks at O_NONBLOCK for non-blocking operation, which means that
> io_uring can't easily poll for it or attempt non-blocking issues. Check for
> IOCB_NOWAIT in locking the pipe for reads and writes, and ditto when we
> decide on whether or not to block or return -EAGAIN.
> 
> Signed-off-by: Jens Axboe <axboe@...nel.dk>
> 
> ---
> 
> If this is acceptable, then I can add S_ISFIFO to the whitelist on file
> descriptors we can IOCB_NOWAIT try for, then poll if we get -EAGAIN
> instead of using thread offload.

Will check.  In the meanwhile, blacklist eventpoll again.  Because your
attempts at "nonblocking" there had been both ugly as hell *AND* fail
to prevent blocking.  And frankly, I'm very tempted to rip that crap
out entirely.  Seriously, *look* at the code you've modified in
do_epoll_ctl().  And tell me why the hell is grabbing ->mtx in that
function needs to be infested with trylocks, while exact same mutex
taken in loop_check_proc() called under those is fine with mutex_lock().
Ditto for calls of vfs_poll() inside ep_insert(), GFP_KERNEL allocations
in ep_ptable_queue_proc(), synchronize_rcu() callable from ep_modify()
(from the same function), et sodding cetera.

No, this is _not_ an invitation to spread the same crap over even more
places in there; I just want to understand where had that kind of voodoo
approach comes from.  And that's directly relevant for this patch,
because it looks like the same kind of thing.

What is your semantics for IOCB_NOWAIT?  What should and what should _not_
be waited for?