Date:   Sun, 29 Jul 2018 15:02:48 +0200
From:   Tomas Bortoli <tomasbortoli@...il.com>
To:     ericvh@...il.com, rminnich@...dia.gov, lucho@...kov.net
Cc:     asmadeus@...ewreck.org, davem@...emloft.net,
        v9fs-developer@...ts.sourceforge.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, syzkaller@...glegroups.com,
        Tomas Bortoli <tomasbortoli@...il.com>
Subject: [PATCH] 9p: fix Use-After-Free in p9_write_work()

There is a race condition between p9_free_req() and p9_write_work().
A request might still need to be processed while p9_free_req() is called.

To fix it, flush the read/write work before freeing any request.

Signed-off-by: Tomas Bortoli <tomasbortoli@...il.com>
Reported-by: syzbot+467050c1ce275af2a5b8@...kaller.appspotmail.com
---

To be able to flush the r/w work from client.c we need the p9_conn and
p9_trans_fd definitions. Therefore this commit moves most of the declarations in
trans_fd.c to trans_fd.h and import such file in client.c

Moreover, a couple of identifiers were altered to avoid name conflicts with the
new import.

 include/net/9p/trans_fd.h | 139 ++++++++++++++++++++++++++++++++++++++++++++++
 net/9p/client.c           |   8 ++-
 net/9p/trans_fd.c         | 113 +------------------------------------
 3 files changed, 148 insertions(+), 112 deletions(-)
 create mode 100644 include/net/9p/trans_fd.h

diff --git a/include/net/9p/trans_fd.h b/include/net/9p/trans_fd.h
new file mode 100644
index 000000000000..cfd4457c40fb
--- /dev/null
+++ b/include/net/9p/trans_fd.h
@@ -0,0 +1,139 @@
+/*
+ * include/fs/9p/trans_fd.h
+ *
+ * Fd transport layer definitions.
+ *
+ *  Copyright (C) 2006 by Russ Cox <rsc@...ch.com>
+ *  Copyright (C) 2004-2005 by Latchesar Ionkov <lucho@...kov.net>
+ *  Copyright (C) 2004-2008 by Eric Van Hensbergen <ericvh@...il.com>
+ *  Copyright (C) 1997-2002 by Ron Minnich <rminnich@...noff.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to:
+ *  Free Software Foundation
+ *  51 Franklin Street, Fifth Floor
+ *  Boston, MA  02111-1301  USA
+ *
+ */
+
+#ifndef P9_TRANS_FD_H
+#define P9_TRANS_FD_H
+
+/**
+ * struct p9_fd_opts - per-transport options
+ * @rfd: file descriptor for reading (trans=fd)
+ * @wfd: file descriptor for writing (trans=fd)
+ * @port: port to connect to (trans=tcp)
+ *
+ */
+
+#define P9_PORT 564
+#define MAX_SOCK_BUF (64*1024)
+#define MAXPOLLWADDR	2
+
+struct p9_fd_opts {
+	int rfd;
+	int wfd;
+	u16 port;
+	bool privport;
+};
+
+/*
+  * Option Parsing (code inspired by NFS code)
+  *  - a little lazy - parse all fd-transport options
+  */
+
+enum {
+	/* Options that take integer arguments */
+	Opt_port, Opt_rfdno, Opt_wfdno, Opt_err,
+	/* Options that take no arguments */
+	Opt_privport,
+};
+
+static const match_table_t trans_tokens = {
+	{Opt_port, "port=%u"},
+	{Opt_rfdno, "rfdno=%u"},
+	{Opt_wfdno, "wfdno=%u"},
+	{Opt_privport, "privport"},
+	{Opt_err, NULL},
+};
+
+enum {
+	Rworksched = 1,		/* read work scheduled or running */
+	Rpending = 2,		/* can read */
+	Wworksched = 4,		/* write work scheduled or running */
+	Wpending = 8,		/* can write */
+};
+
+struct p9_poll_wait {
+	struct p9_conn *conn;
+	wait_queue_entry_t wait;
+	wait_queue_head_t *wait_addr;
+};
+
+/**
+ * struct p9_conn - fd mux connection state information
+ * @mux_list: list link for mux to manage multiple connections (?)
+ * @client: reference to client instance for this connection
+ * @err: error state
+ * @req_list: accounting for requests which have been sent
+ * @unsent_req_list: accounting for requests that haven't been sent
+ * @req: current request being processed (if any)
+ * @tmp_buf: temporary buffer to read in header
+ * @rc: temporary fcall for reading current frame
+ * @wpos: write position for current frame
+ * @wsize: amount of data to write for current frame
+ * @wbuf: current write buffer
+ * @poll_pending_link: pending links to be polled per conn
+ * @poll_wait: array of wait_q's for various worker threads
+ * @pt: poll state
+ * @rq: current read work
+ * @wq: current write work
+ * @wsched: ????
+ *
+ */
+
+struct p9_conn {
+	struct list_head mux_list;
+	struct p9_client *client;
+	int err;
+	struct list_head req_list;
+	struct list_head unsent_req_list;
+	struct p9_req_t *req;
+	char tmp_buf[7];
+	struct p9_fcall rc;
+	int wpos;
+	int wsize;
+	char *wbuf;
+	struct list_head poll_pending_link;
+	struct p9_poll_wait poll_wait[MAXPOLLWADDR];
+	poll_table pt;
+	struct work_struct rq;
+	struct work_struct wq;
+	unsigned long wsched;
+};
+
+/**
+ * struct p9_trans_fd - transport state
+ * @rd: reference to file to read from
+ * @wr: reference of file to write to
+ * @conn: connection state reference
+ *
+ */
+
+struct p9_trans_fd {
+	struct file *rd;
+	struct file *wr;
+	struct p9_conn conn;
+};
+
+#endif
diff --git a/net/9p/client.c b/net/9p/client.c
index 2ec0edc6104f..ddfb63672a63 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -40,6 +40,7 @@
 #include <linux/seq_file.h>
 #include <net/9p/client.h>
 #include <net/9p/transport.h>
+#include <net/9p/trans_fd.h>
 #include "protocol.h"
 
 #define CREATE_TRACE_POINTS
@@ -55,7 +56,7 @@ enum {
 	Opt_trans,
 	Opt_legacy,
 	Opt_version,
-	Opt_err,
+	Opt_error,
 };
 
 static const match_table_t tokens = {
@@ -63,7 +64,7 @@ static const match_table_t tokens = {
 	{Opt_legacy, "noextend"},
 	{Opt_trans, "trans=%s"},
 	{Opt_version, "version=%s"},
-	{Opt_err, NULL},
+	{Opt_error, NULL},
 };
 
 inline int p9_is_proto_dotl(struct p9_client *clnt)
@@ -329,12 +330,15 @@ EXPORT_SYMBOL(p9_tag_lookup);
 static void p9_free_req(struct p9_client *c, struct p9_req_t *r)
 {
 	unsigned long flags;
+	struct p9_trans_fd *ts = c->trans;
 	u16 tag = r->tc->tag;
 
 	p9_debug(P9_DEBUG_MUX, "clnt %p req %p tag: %d\n", c, r, tag);
 	spin_lock_irqsave(&c->lock, flags);
 	idr_remove(&c->reqs, tag);
 	spin_unlock_irqrestore(&c->lock, flags);
+	flush_work(&ts->conn.wq);
+	flush_work(&ts->conn.rq);
 	kfree(r->tc);
 	kfree(r->rc);
 	kmem_cache_free(p9_req_cache, r);
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index e2ef3c782c53..63b3c7ef0d90 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -45,122 +45,15 @@
 #include <net/9p/9p.h>
 #include <net/9p/client.h>
 #include <net/9p/transport.h>
+#include <net/9p/trans_fd.h>
 
 #include <linux/syscalls.h> /* killme */
 
-#define P9_PORT 564
-#define MAX_SOCK_BUF (64*1024)
-#define MAXPOLLWADDR	2
+static void p9_poll_workfn(struct work_struct *work);
 
 static struct p9_trans_module p9_tcp_trans;
 static struct p9_trans_module p9_fd_trans;
 
-/**
- * struct p9_fd_opts - per-transport options
- * @rfd: file descriptor for reading (trans=fd)
- * @wfd: file descriptor for writing (trans=fd)
- * @port: port to connect to (trans=tcp)
- *
- */
-
-struct p9_fd_opts {
-	int rfd;
-	int wfd;
-	u16 port;
-	bool privport;
-};
-
-/*
-  * Option Parsing (code inspired by NFS code)
-  *  - a little lazy - parse all fd-transport options
-  */
-
-enum {
-	/* Options that take integer arguments */
-	Opt_port, Opt_rfdno, Opt_wfdno, Opt_err,
-	/* Options that take no arguments */
-	Opt_privport,
-};
-
-static const match_table_t tokens = {
-	{Opt_port, "port=%u"},
-	{Opt_rfdno, "rfdno=%u"},
-	{Opt_wfdno, "wfdno=%u"},
-	{Opt_privport, "privport"},
-	{Opt_err, NULL},
-};
-
-enum {
-	Rworksched = 1,		/* read work scheduled or running */
-	Rpending = 2,		/* can read */
-	Wworksched = 4,		/* write work scheduled or running */
-	Wpending = 8,		/* can write */
-};
-
-struct p9_poll_wait {
-	struct p9_conn *conn;
-	wait_queue_entry_t wait;
-	wait_queue_head_t *wait_addr;
-};
-
-/**
- * struct p9_conn - fd mux connection state information
- * @mux_list: list link for mux to manage multiple connections (?)
- * @client: reference to client instance for this connection
- * @err: error state
- * @req_list: accounting for requests which have been sent
- * @unsent_req_list: accounting for requests that haven't been sent
- * @req: current request being processed (if any)
- * @tmp_buf: temporary buffer to read in header
- * @rc: temporary fcall for reading current frame
- * @wpos: write position for current frame
- * @wsize: amount of data to write for current frame
- * @wbuf: current write buffer
- * @poll_pending_link: pending links to be polled per conn
- * @poll_wait: array of wait_q's for various worker threads
- * @pt: poll state
- * @rq: current read work
- * @wq: current write work
- * @wsched: ????
- *
- */
-
-struct p9_conn {
-	struct list_head mux_list;
-	struct p9_client *client;
-	int err;
-	struct list_head req_list;
-	struct list_head unsent_req_list;
-	struct p9_req_t *req;
-	char tmp_buf[7];
-	struct p9_fcall rc;
-	int wpos;
-	int wsize;
-	char *wbuf;
-	struct list_head poll_pending_link;
-	struct p9_poll_wait poll_wait[MAXPOLLWADDR];
-	poll_table pt;
-	struct work_struct rq;
-	struct work_struct wq;
-	unsigned long wsched;
-};
-
-/**
- * struct p9_trans_fd - transport state
- * @rd: reference to file to read from
- * @wr: reference of file to write to
- * @conn: connection state reference
- *
- */
-
-struct p9_trans_fd {
-	struct file *rd;
-	struct file *wr;
-	struct p9_conn conn;
-};
-
-static void p9_poll_workfn(struct work_struct *work);
-
 static DEFINE_SPINLOCK(p9_poll_lock);
 static LIST_HEAD(p9_poll_pending_list);
 static DECLARE_WORK(p9_poll_work, p9_poll_workfn);
@@ -765,7 +658,7 @@ static int parse_opts(char *params, struct p9_fd_opts *opts)
 		int r;
 		if (!*p)
 			continue;
-		token = match_token(p, tokens, args);
+		token = match_token(p, trans_tokens, args);
 		if ((token != Opt_err) && (token != Opt_privport)) {
 			r = match_int(&args[0], &option);
 			if (r < 0) {
-- 
2.11.0

Powered by blists - more mailing lists