public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	syzbot <syzbot+8b41a1365f1106fd0f33@syzkaller.appspotmail.com>,
	Christian Schoenebeck <linux_oss@crudebyte.com>,
	Dominique Martinet <asmadeus@codewreck.org>,
	Sasha Levin <sashal@kernel.org>,
	ericvh@gmail.com, lucho@ionkov.net, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org
Subject: [PATCH AUTOSEL 5.4 12/13] 9p/trans_fd: always use O_NONBLOCK read/write
Date: Mon, 17 Oct 2022 20:11:01 -0400	[thread overview]
Message-ID: <20221018001102.2731930-12-sashal@kernel.org> (raw)
In-Reply-To: <20221018001102.2731930-1-sashal@kernel.org>

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit ef575281b21e9a34dfae544a187c6aac2ae424a9 ]

syzbot is reporting hung task at p9_fd_close() [1], for p9_mux_poll_stop()
 from p9_conn_destroy() from p9_fd_close() is failing to interrupt already
started kernel_read() from p9_fd_read() from p9_read_work() and/or
kernel_write() from p9_fd_write() from p9_write_work() requests.

Since p9_socket_open() sets O_NONBLOCK flag, p9_mux_poll_stop() does not
need to interrupt kernel_read()/kernel_write(). However, since p9_fd_open()
does not set O_NONBLOCK flag, but pipe blocks unless signal is pending,
p9_mux_poll_stop() needs to interrupt kernel_read()/kernel_write() when
the file descriptor refers to a pipe. In other words, pipe file descriptor
needs to be handled as if socket file descriptor.

We somehow need to interrupt kernel_read()/kernel_write() on pipes.

A minimal change, which this patch is doing, is to set O_NONBLOCK flag
 from p9_fd_open(), for O_NONBLOCK flag does not affect reading/writing
of regular files. But this approach changes O_NONBLOCK flag on userspace-
supplied file descriptors (which might break userspace programs), and
O_NONBLOCK flag could be changed by userspace. It would be possible to set
O_NONBLOCK flag every time p9_fd_read()/p9_fd_write() is invoked, but still
remains small race window for clearing O_NONBLOCK flag.

If we don't want to manipulate O_NONBLOCK flag, we might be able to
surround kernel_read()/kernel_write() with set_thread_flag(TIF_SIGPENDING)
and recalc_sigpending(). Since p9_read_work()/p9_write_work() works are
processed by kernel threads which process global system_wq workqueue,
signals could not be delivered from remote threads when p9_mux_poll_stop()
 from p9_conn_destroy() from p9_fd_close() is called. Therefore, calling
set_thread_flag(TIF_SIGPENDING)/recalc_sigpending() every time would be
needed if we count on signals for making kernel_read()/kernel_write()
non-blocking.

Link: https://lkml.kernel.org/r/345de429-a88b-7097-d177-adecf9fed342@I-love.SAKURA.ne.jp
Link: https://syzkaller.appspot.com/bug?extid=8b41a1365f1106fd0f33 [1]
Reported-by: syzbot <syzbot+8b41a1365f1106fd0f33@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: syzbot <syzbot+8b41a1365f1106fd0f33@syzkaller.appspotmail.com>
Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>
[Dominique: add comment at Christian's suggestion]
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/9p/trans_fd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 70bcacb3bbdc..b691871d9a02 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -821,11 +821,14 @@ static int p9_fd_open(struct p9_client *client, int rfd, int wfd)
 		goto out_free_ts;
 	if (!(ts->rd->f_mode & FMODE_READ))
 		goto out_put_rd;
+	/* prevent workers from hanging on IO when fd is a pipe */
+	ts->rd->f_flags |= O_NONBLOCK;
 	ts->wr = fget(wfd);
 	if (!ts->wr)
 		goto out_put_rd;
 	if (!(ts->wr->f_mode & FMODE_WRITE))
 		goto out_put_wr;
+	ts->wr->f_flags |= O_NONBLOCK;
 
 	client->trans = ts;
 	client->status = Connected;
-- 
2.35.1


  parent reply	other threads:[~2022-10-18  0:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-18  0:10 [PATCH AUTOSEL 5.4 01/13] crypto: qcom-rng - Fix qcom_rng_of_match unused warning Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 02/13] gfs2: Switch from strlcpy to strscpy Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 03/13] cgroup: Remove data-race around cgrp_dfl_visible Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 04/13] of/fdt: Don't calculate initrd size from DT if start > end Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 05/13] objtool,x86: Teach decode about LOOP* instructions Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 06/13] gfs2: Check sb_bsize_shift after reading superblock Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 07/13] powerpc/64: don't refer nr_cpu_ids in asm code when it's undefined Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 08/13] m68knommu: fix non-specific 68328 choice interrupt build failure Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 09/13] m68knommu: fix non-mmu classic 68000 legacy timer tick selection Sasha Levin
2022-10-18  0:10 ` [PATCH AUTOSEL 5.4 10/13] kmsan: disable physical page merging in biovec Sasha Levin
2022-10-18  0:22   ` Eric Biggers
2022-10-18  9:23     ` Vlastimil Babka
2022-10-18  0:11 ` [PATCH AUTOSEL 5.4 11/13] 9p: trans_fd/p9_conn_cancel: drop client lock earlier Sasha Levin
2022-10-18  0:11 ` Sasha Levin [this message]
2022-10-18  0:11 ` [PATCH AUTOSEL 5.4 13/13] virtio_pci: don't try to use intxif pin is zero Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221018001102.2731930-12-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=asmadeus@codewreck.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ericvh@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+8b41a1365f1106fd0f33@syzkaller.appspotmail.com \
    --cc=v9fs-developer@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox