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

Zhu Yanjun
2024-08-02T09:19:08Z

在 2024/8/2 10:28, Junxian Huang 写道:
>
>
> On 2024/8/2 5:55, Zhu Yanjun wrote:
>> 在 2024/8/1 15:44, Junxian Huang 写道:
>>> 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.
>>
>> Can you share the method to reproduce this problem?
>> I am interested in this problem.
>>
>> Thanks,
>> Zhu Yanjun
>>
>
> I was testing bonding in 5.10 kernel, doing
> ifenslave bond0 eth0 eth1; ifenslave -d bond0 eth0 eth1
> and
> ethtool --reset eth0 dedicated; ethtool --reset eth1 dedicated
> concurrently and infinitely.
>
> But I think this problem has been fixed in latest mainline probably.
> Please look into my reply to Bart in v2.

Thanks a lot. I am working on srq. Because ib_srpt also supports srq, I
just want to confirm if srq of ib_srpt can also work well on RoCEv2 and
IB with this commit.

Zhu Yanjun

>
> Junxian
>
>>>
>>> Fixes: a42d985bd5b2 ("ib_srpt: Initial SRP Target merge for v3.3-rc1")
>>> Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
>>> ---
>>>   drivers/infiniband/ulp/srpt/ib_srpt.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
>>> index 9632afbd727b..244e5c115bf7 100644
>>> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
>>> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
>>> @@ -3148,8 +3148,8 @@ static int srpt_add_one(struct ib_device *device)
>>>   {
>>>       struct srpt_device *sdev;
>>>       struct srpt_port *sport;
>>> +    u32 i, j;
>>>       int ret;
>>> -    u32 i;
>>>         pr_debug("device = %p\n", device);
>>>   @@ -3226,7 +3226,6 @@ static int srpt_add_one(struct ib_device *device)
>>>           if (ret) {
>>>               pr_err("MAD registration failed for %s-%d.\n",
>>>                      dev_name(&sdev->device->dev), i);
>>> -            i--;
>>>               goto err_port;
>>>           }
>>>       }
>>> @@ -3241,6 +3240,8 @@ static int srpt_add_one(struct ib_device *device)
>>>       return 0;
>>>     err_port:
>>> +    for (j = i, i--; j > 0; j--)
>>> +        cancel_work_sync(&sdev->port[j - 1].work);
>>>       srpt_unregister_mad_agent(sdev, i);
>>>   err_cm:
>>>       if (sdev->cm_id)
>>