From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: Eric Van Hensbergen <ericvh@gmail.com>,
Latchesar Ionkov <lucho@ionkov.net>,
Dominique Martinet <asmadeus@codewreck.org>,
Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: syzbot <syzbot+8b41a1365f1106fd0f33@syzkaller.appspotmail.com>,
v9fs-developer@lists.sourceforge.net,
syzkaller-bugs@googlegroups.com, netdev@vger.kernel.org
Subject: [PATCH v2] 9p/trans_fd: perform read/write with TIF_SIGPENDING set
Date: Sat, 27 Aug 2022 15:11:48 +0900 [thread overview]
Message-ID: <4293faaf-8279-77e2-8b1a-aff765416980@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <345de429-a88b-7097-d177-adecf9fed342@I-love.SAKURA.ne.jp>
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,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,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,write}() on pipes.
If we can tolerate "possibility of breaking userspace program by setting
O_NONBLOCK flag on userspace-supplied file descriptors" and "possibility
of race window that userspace program clears O_NONBLOCK flag between after
automatically setting O_NONBLOCK flag and before calling
kernel_{read,write}()", we could automatically set O_NONBLOCK flag
immediately before calling kernel_{read,write}().
A different approach, which this patch is doing, is to surround
kernel_{read,write}() with set_thread_flag(TIF_SIGPENDING) and
recalc_sigpending(). This might be ugly and bit costly, but does not
touch userspace-supplied file descriptors.
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>
---
Although syzbot tested that this patch solves hung task problem, syzbot
cannot verify that this patch will not break functionality of p9 users.
Please test before applying this patch.
net/9p/trans_fd.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index e758978b44be..e2f4e3245a80 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -256,11 +256,13 @@ static int p9_fd_read(struct p9_client *client, void *v, int len)
if (!ts)
return -EREMOTEIO;
- if (!(ts->rd->f_flags & O_NONBLOCK))
- p9_debug(P9_DEBUG_ERROR, "blocking read ...\n");
-
pos = ts->rd->f_pos;
+ /* Force non-blocking read() even without O_NONBLOCK. */
+ set_thread_flag(TIF_SIGPENDING);
ret = kernel_read(ts->rd, v, len, &pos);
+ spin_lock_irq(¤t->sighand->siglock);
+ recalc_sigpending();
+ spin_unlock_irq(¤t->sighand->siglock);
if (ret <= 0 && ret != -ERESTARTSYS && ret != -EAGAIN)
client->status = Disconnected;
return ret;
@@ -423,10 +425,12 @@ static int p9_fd_write(struct p9_client *client, void *v, int len)
if (!ts)
return -EREMOTEIO;
- if (!(ts->wr->f_flags & O_NONBLOCK))
- p9_debug(P9_DEBUG_ERROR, "blocking write ...\n");
-
+ /* Force non-blocking write() even without O_NONBLOCK. */
+ set_thread_flag(TIF_SIGPENDING);
ret = kernel_write(ts->wr, v, len, &ts->wr->f_pos);
+ spin_lock_irq(¤t->sighand->siglock);
+ recalc_sigpending();
+ spin_unlock_irq(¤t->sighand->siglock);
if (ret <= 0 && ret != -ERESTARTSYS && ret != -EAGAIN)
client->status = Disconnected;
return ret;
--
2.18.4
next prev parent reply other threads:[~2022-08-27 6:11 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-30 19:28 INFO: task hung in p9_fd_close syzbot
2019-09-21 16:19 ` syzbot
2022-08-26 15:27 ` [PATCH] 9p/trans_fd: always use O_NONBLOCK read/write Tetsuo Handa
2022-08-27 6:11 ` Tetsuo Handa [this message]
2022-09-01 15:23 ` [PATCH v2] 9p/trans_fd: perform read/write with TIF_SIGPENDING set Christian Schoenebeck
2022-09-01 22:25 ` Tetsuo Handa
2022-09-03 23:39 ` Dominique Martinet
2022-09-04 0:27 ` Tetsuo Handa
2022-10-07 1:40 ` Dominique Martinet
2022-10-07 11:52 ` Tetsuo Handa
2022-10-06 14:55 ` [PATCH] 9p/trans_fd: always use O_NONBLOCK read/write Christian Schoenebeck
2022-10-07 1:03 ` Dominique Martinet
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=4293faaf-8279-77e2-8b1a-aff765416980@I-love.SAKURA.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=asmadeus@codewreck.org \
--cc=ericvh@gmail.com \
--cc=linux_oss@crudebyte.com \
--cc=lucho@ionkov.net \
--cc=netdev@vger.kernel.org \
--cc=syzbot+8b41a1365f1106fd0f33@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.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;
as well as URLs for NNTP newsgroup(s).