netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: unlisted-recipients:; (no To-header on input)
Cc: Dominique Martinet <dominique.martinet@cea.fr>,
	v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
	Eric Van Hensbergen <ericvh@gmail.com>,
	Latchesar Ionkov <lucho@ionkov.net>
Subject: [PATCH 1/2] 9p/trans_fd: abort p9_read_work if req status changed
Date: Tue,  9 Oct 2018 06:05:55 +0200	[thread overview]
Message-ID: <1539057956-23741-1-git-send-email-asmadeus@codewreck.org> (raw)
In-Reply-To: <20181009020949.GA29622@nautica>

From: Dominique Martinet <dominique.martinet@cea.fr>

p9_read_work would try to handle an errored req even if it got put to
error state by another thread between the lookup (that worked) and the
time it had been fully read.
The request itself is safe to use because we hold a ref to it from the
lookup (for m->rreq, so it was safe to read into the request data buffer
until this point), but the req_list has been deleted at the same time
status changed, and client_cb already has been called as well, so we
should not do either.

Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
Reported-by: syzbot+2222c34dc40b515f30dc@syzkaller.appspotmail.com
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Cc: Latchesar Ionkov <lucho@ionkov.net>
---

As written in reply to the bug report I'm not sure it's what syzbot
complained about exactly, but it feels like a correct thing to do.
The second patch is unrelated to the syzbot report, but something that
occured to me while looking at this ; I'll take both into linux-next
around the start of next week after getting some proper testing done
unless remarks happen.
(they pass basic tests already)

 net/9p/trans_fd.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 12559c474dde..a0317d459cde 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -292,7 +292,6 @@ static void p9_read_work(struct work_struct *work)
 	__poll_t n;
 	int err;
 	struct p9_conn *m;
-	int status = REQ_STATUS_ERROR;
 
 	m = container_of(work, struct p9_conn, rq);
 
@@ -375,11 +374,17 @@ static void p9_read_work(struct work_struct *work)
 		p9_debug(P9_DEBUG_TRANS, "got new packet\n");
 		m->rreq->rc.size = m->rc.offset;
 		spin_lock(&m->client->lock);
-		if (m->rreq->status != REQ_STATUS_ERROR)
-			status = REQ_STATUS_RCVD;
-		list_del(&m->rreq->req_list);
-		/* update req->status while holding client->lock  */
-		p9_client_cb(m->client, m->rreq, status);
+		if (m->rreq->status == REQ_STATUS_SENT) {
+			list_del(&m->rreq->req_list);
+			p9_client_cb(m->client, m->rreq, REQ_STATUS_RCVD);
+		} else {
+			spin_unlock(&m->client->lock);
+			p9_debug(P9_DEBUG_ERROR,
+				 "Request tag %d errored out while we were reading the reply\n",
+				 m->rc.tag);
+			err = -EIO;
+			goto error;
+		}
 		spin_unlock(&m->client->lock);
 		m->rc.sdata = NULL;
 		m->rc.offset = 0;
-- 
2.19.1

  reply	other threads:[~2018-10-09  4:05 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-16  5:59 BUG: corrupted list in p9_read_work syzbot
2018-10-09  1:07 ` syzbot
2018-10-09  2:09   ` Dominique Martinet
2018-10-09  4:05     ` Dominique Martinet [this message]
2018-10-09  4:05       ` [PATCH 2/2] 9p/trans_fd: put worker reqs on destroy Dominique Martinet
2018-10-09 13:19         ` Tomas Bortoli
2018-10-15 10:46           ` Dominique Martinet
2018-10-10 14:03     ` BUG: corrupted list in p9_read_work Dmitry Vyukov
2018-10-10 14:40       ` Dominique Martinet
2018-10-10 14:51         ` Dmitry Vyukov
2018-10-10 15:58           ` Dominique Martinet
2018-10-11 12:33             ` Dmitry Vyukov
2018-10-11 13:10               ` Dominique Martinet
2018-10-11 13:27                 ` Dmitry Vyukov
2018-10-11 13:40                   ` Dmitry Vyukov
2018-10-11 14:28                     ` 9p/RDMA for syzkaller (Was: BUG: corrupted list in p9_read_work) Dominique Martinet
2018-10-12 14:42                       ` Dmitry Vyukov
2018-10-11 14:19                   ` Dominique Martinet
2018-10-12 14:50                     ` Dmitry Vyukov
2018-10-12 15:08                       ` Dominique Martinet
2018-11-17  8:46                         ` Dominique Martinet
2018-11-20 11:20                           ` Dmitry Vyukov
2018-11-20 11:28                             ` Dominique Martinet
2018-10-10 14:29     ` BUG: corrupted list in p9_read_work Dmitry Vyukov
2018-10-10 14:48       ` Dominique Martinet
2018-10-10 14:49         ` syzbot
2018-10-10 16:00           ` Dominique Martinet
2018-10-10 16:02             ` syzbot
2018-10-10 16:10             ` Dominique Martinet
2018-10-10 16:29               ` syzbot
2018-10-10 16:36               ` Dmitry Vyukov
2018-10-10 22:55                 ` Dominique Martinet
2018-10-10 14:42     ` Dmitry Vyukov

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=1539057956-23741-1-git-send-email-asmadeus@codewreck.org \
    --to=asmadeus@codewreck.org \
    --cc=dominique.martinet@cea.fr \
    --cc=ericvh@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --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).