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
