net/9p/trans_fd.c: fix race by holding the lock
Subsystems (3)
PATCH 1/1 net/9p/trans_fd.c: fix race by holding the lock
It may be possible to run p9_fd_cancel() with a deleted req->req_list and incur in a double del. To fix hold the client->lock while changing the status, so the other threads will be synchronized.
Signed-off-by Tomas Bortoli <[email protected]>
Reported-by [email protected]
--- net/9p/trans_fd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
net/9p/trans_fd.c +5 -5
@@ -197,15 +197,14 @@ static void p9_mux_poll_stop(struct p9_conn *m)static void p9_conn_cancel (struct p9_conn *m,int err) {struct p9_req_t *req, *rtmp; -unsigned long flags;LIST_HEAD (cancel_list);p9_debug (P9_DEBUG_ERROR ,"mux %p err %d\n" , m, err); -spin_lock_irqsave (&m->client->lock, flags); +spin_lock (&m->client->lock);if (m->err) { -spin_unlock_irqrestore (&m->client->lock, flags); +spin_unlock (&m->client->lock);return ; }
@@ -217,7 +216,6 @@ static void p9_conn_cancel(struct p9_conn *m, int err)list_for_each_entry_safe (req, rtmp, &m->unsent_req_list, req_list) {list_move (&req->req_list, &cancel_list); } -spin_unlock_irqrestore (&m->client->lock, flags);list_for_each_entry_safe (req, rtmp, &cancel_list, req_list) {p9_debug (P9_DEBUG_ERROR ,"call back req %p\n" , req);
@@ -226,6 +224,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err) req->t_err = err;p9_client_cb (m->client, req,REQ_STATUS_ERROR ); } +spin_unlock (&m->client->lock); } static __poll_t
@@ -373,8 +372,9 @@ static void p9_read_work(struct work_struct *work)if (m->req->status !=REQ_STATUS_ERROR ) status =REQ_STATUS_RCVD ;list_del (&m->req->req_list); -spin_unlock (&m->client->lock); +/* update req->status while holding client->lock */ p9_client_cb (m->client, m->req, status); +spin_unlock (&m->client->lock); m->rc.sdata =NULL ; m->rc.offset =0 ; m->rc.capacity =0 ;
--
2.11.0
Dominique Martinet
maintainerTomas Bortoli wrote on Mon, Jul 23, 2018:
Quoted 6 lines
It may be possible to run
p9_fd_cancel()with a deletedreq->req_listand incur in a double del. To fix hold theclient->lockwhile changing the status, so the other threads will be synchronized.Signed-off-by Tomas Bortoli <[email protected]>Reported-by [email protected]
OK, taking this one.
Quoted 53 lines
--- net/9p/trans_fd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)net/9p/trans_fd.c +5
-5@@ -197,15 +197,14 @@ static void p9_mux_poll_stop(struct p9_conn *m)static void p9_conn_cancel (struct p9_conn *m,int err) {struct p9_req_t *req, *rtmp; -unsigned long flags;LIST_HEAD (cancel_list);p9_debug (P9_DEBUG_ERROR ,"mux %p err %d\n" , m, err); -spin_lock_irqsave (&m->client->lock, flags); +spin_lock (&m->client->lock);if (m->err) { -spin_unlock_irqrestore (&m->client->lock, flags); +spin_unlock (&m->client->lock);return ; }@@ -217,7 +216,6 @@ static void p9_conn_cancel(struct p9_conn *m, int err)list_for_each_entry_safe (req, rtmp, &m->unsent_req_list, req_list) {list_move (&req->req_list, &cancel_list); } -spin_unlock_irqrestore (&m->client->lock, flags);list_for_each_entry_safe (req, rtmp, &cancel_list, req_list) {p9_debug (P9_DEBUG_ERROR ,"call back req %p\n" , req);@@ -226,6 +224,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err) req->t_err = err;p9_client_cb (m->client, req,REQ_STATUS_ERROR ); } +spin_unlock (&m->client->lock); } static __poll_t@@ -373,8 +372,9 @@ static void p9_read_work(struct work_struct *work)if (m->req->status !=REQ_STATUS_ERROR ) status =REQ_STATUS_RCVD ;list_del (&m->req->req_list); -spin_unlock (&m->client->lock); +/* update req->status while holding client->lock */ p9_client_cb (m->client, m->req, status); +spin_unlock (&m->client->lock); m->rc.sdata =NULL ; m->rc.offset =0 ; m->rc.capacity =0 ;