[PATCH v2 for-rc] RDMA/srpt: Fix UAF when srpt_add_one() failed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Currently cancel_work_sync() is not called when srpt_refresh_port()
failed in srpt_add_one(). There is a probability that sdev has been
freed while the previously initiated sport->work is still running,
leading to a UAF as the log below:

[  T880] ib_srpt MAD registration failed for hns_1-1.
[  T880] ib_srpt srpt_add_one(hns_1) failed.
[  T376] Unable to handle kernel paging request at virtual address 0000000000010008
...
[  T376] Workqueue: events srpt_refresh_port_work [ib_srpt]
...
[  T376] Call trace:
[  T376]  srpt_refresh_port+0x94/0x264 [ib_srpt]
[  T376]  srpt_refresh_port_work+0x1c/0x2c [ib_srpt]
[  T376]  process_one_work+0x1d8/0x4cc
[  T376]  worker_thread+0x158/0x410
[  T376]  kthread+0x108/0x13c
[  T376]  ret_from_fork+0x10/0x18

Add cancel_work_sync() to the exception branch to fix this UAF.
Besides, exchange the order of INIT_WORK() and srpt_refresh_port()
in srpt_add_one(), so that when srpt_refresh_port() failed, there
is no need to cancel the work in this iteration.

Fixes: a42d985bd5b2 ("ib_srpt: Initial SRP Target merge for v3.3-rc1")
Signed-off-by: Junxian Huang <huangjunxian6@xxxxxxxxxxxxx>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 9632afbd727b..7def231da21a 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -648,6 +648,7 @@ static void srpt_unregister_mad_agent(struct srpt_device *sdev, int port_cnt)
 			ib_unregister_mad_agent(sport->mad_agent);
 			sport->mad_agent = NULL;
 		}
+		cancel_work_sync(&sport->work);
 	}
 }
 
@@ -3220,7 +3221,6 @@ static int srpt_add_one(struct ib_device *device)
 		sport->port_attrib.srp_max_rsp_size = DEFAULT_MAX_RSP_SIZE;
 		sport->port_attrib.srp_sq_size = DEF_SRPT_SQ_SIZE;
 		sport->port_attrib.use_srq = false;
-		INIT_WORK(&sport->work, srpt_refresh_port_work);
 
 		ret = srpt_refresh_port(sport);
 		if (ret) {
@@ -3229,6 +3229,8 @@ static int srpt_add_one(struct ib_device *device)
 			i--;
 			goto err_port;
 		}
+
+		INIT_WORK(&sport->work, srpt_refresh_port_work);
 	}
 
 	ib_register_event_handler(&sdev->event_handler);
@@ -3264,13 +3266,9 @@ static void srpt_remove_one(struct ib_device *device, void *client_data)
 	struct srpt_device *sdev = client_data;
 	int i;
 
-	srpt_unregister_mad_agent(sdev, sdev->device->phys_port_cnt);
-
 	ib_unregister_event_handler(&sdev->event_handler);
 
-	/* Cancel any work queued by the just unregistered IB event handler. */
-	for (i = 0; i < sdev->device->phys_port_cnt; i++)
-		cancel_work_sync(&sdev->port[i].work);
+	srpt_unregister_mad_agent(sdev, sdev->device->phys_port_cnt);
 
 	if (sdev->cm_id)
 		ib_destroy_cm_id(sdev->cm_id);
-- 
2.33.0





[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux