netdev - Re: RE: [PATCHv2 0/6] Fix the problem that rxe can not work in net

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 13 Nov 2022 18:25:47 +0800
From:   Yanjun Zhu <yanjun.zhu@...ux.dev>
To:     Parav Pandit <parav@...dia.com>, Yanjun Zhu <yanjun.zhu@...ux.dev>,
        "jgg@...pe.ca" <jgg@...pe.ca>, "leon@...nel.org" <leon@...nel.org>,
        "zyjzyj2000@...il.com" <zyjzyj2000@...il.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
Cc:     Zhu Yanjun <yanjun.zhu@...el.com>
Subject: Re: RE: [PATCHv2 0/6] Fix the problem that rxe can not work in net

在 2022/11/13 12:58, Parav Pandit 写道:
> Hi Yanjun,
> 
>> From: Yanjun Zhu <yanjun.zhu@...ux.dev>
>> Sent: Thursday, November 10, 2022 10:38 PM
>>
>>
>> 在 2022/11/11 11:35, Parav Pandit 写道:
>>>> From: Yanjun Zhu <yanjun.zhu@...ux.dev>
>>>> Sent: Thursday, November 10, 2022 9:37 PM
>>>
>>>> Can you help to review these patches?
>>> I will try to review it before 13th.
> 
> I did a brief review of patch set.
> I didn’t go line by line for each patch; hence I give lumped comments here for overall series.
> 
> 1. Add example and test results in below test flow in exclusive mode in cover letter.
>     # ip netns exec net1 rdma link add rxe1 type rxe netdev eno3
>     # ip netns del net0
>     Verify that rdma device rxe1 is deleted.

Hi, Parav

Thanks a lot. I will add this example to the cover letter.
And I confirm that rdma device rxe1 is deleted after the command "ip 
netns del net0".

I will delve into the following comments.

Thanks and Regards,
Zhu Yanjun

> 
> 2. Usage of dev_net() in rxe_setup_udp_tunnel() is unsafe.
>     This is because when rxe_setup_udp_tunnel() is executed, net ns of netdev can change.
>     This needs to be synchronized with per net notifier register_pernet_subsys() of exit or exit_batch.
>     This notifiers callback should be added to rxe module.
> 
> 3. You need to set bind_ifindex of udp config to the netdev given in newlink in rxe_setup_udp_tunnel.
>     Should be a separate pre-patch to ensure that close and right relation to udp socket with netdev in a given netns.
> 
> 4. Rearrange series to implement delete link as separate series from net ns securing series.
> They are unrelated. Current delink series may have use after free accesses. Those needs to be guarded in likely larger series.
> 
> 5. udp tunnel must shutdown synchronously when rdma link del is done.
>     This means any new packet arriving after this point, will be dropped.
>     Any existing packet handling present is flushed.
>     From your cover letter description, it appears that sock deletion is refcount based and above semantics is not ensured.
> 
> 6. In patch 5, rxe_get_dev_from_net() can return NULL, hence l_sk6 check can be unsafe. Please add check for rdev null before rdev->l_sk6 check.
> 
> 7. In patch 5, I didn't fully inspect, but seems like call to rxe_find_route4() is not rcu safe.
> Hence, extension of dev_net() in rxe_find_route4() doesn't look secure.
> Accessing sock_net() is more accurate, because at this layer, it is processing packets at socket layer.