[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z-iB5QkZyayj0Sua@codewreck.org>
Date: Sun, 30 Mar 2025 08:27:33 +0900
From: asmadeus@...ewreck.org
To: Oleg Nesterov <oleg@...hat.com>
Cc: syzbot <syzbot+62262fdc0e01d99573fc@...kaller.appspotmail.com>,
brauner@...nel.org, dhowells@...hat.com, ericvh@...nel.org,
jack@...e.cz, jlayton@...nel.org, kprateek.nayak@....com,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
linux_oss@...debyte.com, lucho@...kov.net, mjguzik@...il.com,
netfs@...ts.linux.dev, swapnil.sapkal@....com,
syzkaller-bugs@...glegroups.com, v9fs@...ts.linux.dev,
viro@...iv.linux.org.uk
Subject: Re: [syzbot] [netfs?] INFO: task hung in netfs_unbuffered_write_iter
Oleg Nesterov wrote on Sat, Mar 29, 2025 at 03:21:39PM +0100:
> First of all, let me remind that I know nothing about 9p or netfs ;)
> And I am not sure that my patch is the right solution.
>
> I am not even sure we need the fix, according to syzbot testing the
> problem goes away with the fixes from David
> https://web.git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=netfs-fixes
> but I didn't even try to read them, this is not my area.
(gah, I hate emails when one gets added to thread later.. I've just now
opened the thread on lore and seen David's test :/)
> > - due to the new optimization (aaec5a95d59615 "pipe_read: don't wake up
> > the writer if the pipe is still full"), that 'if there is room to send'
> > check started failing and tx thread doesn't start?
>
> Again, I can be easily wrong, but no.
>
> With or without the optimization above, it doesn't make sense to start
> the tx thread when the pipe is full, p9_fd_poll() can't report EPOLLOUT.
>
> Lets recall that the idle read worker did kernel_read() -> pipe_read().
> Before this optimization, pipe_read() did the unnecessary
>
> wake_up_interruptible_sync_poll(&pipe->wr_wait);
>
> when the pipe was full before the reading _and_ is still full after the
> reading.
>
> This wakeup calls p9_pollwake() which kicks p9_poll_workfn().
Aah, that's the bit I didn't get, thank you!
> This no longer happens after the optimization. So in some sense the
> p9_fd_request() -> p9_poll_mux() hack (which wakes the rx thread in this
> case) restores the old behaviour.
>
> But again, again, quite possibly I completely misread this (nontrivial)
> code.
Yes, this totally makes sense; I agree with your analysis.
So basically 9p was optimizing for this impossible (on a normal server)
behaviour in the 9p side (it doesn't make any sense for the tx pipe to
be full with 0 in flight request, and tx pipe never goes unfull, and
reply comes (was there) before the actual write happened!!), but this
old behaviour made it work anyway...
So part of me wants to just leave it there and if anything try to make
this kind of usage impossible by adding more checks to mount -o
trans=fd, but I don't think it's possible to lock down all kind of weird
behaviour root users (=syzbot) can engage in...
OTOH syzbot does find some useful bugs so I guess it might be worth
fixing, I don't know.
If David's patch also happens to fix it I guess we can also just wait
for that?
Thanks,
--
Dominique Martinet | Asmadeus
Powered by blists - more mailing lists