Re: [PATCH] RDMA/rxe: rework per-net tunnel socket lifetime to fix refcount underflow

[Serhat Kumral <serhatkumral1@gmail.com>]

Hi Yanjun,

Thanks for the review.

> core problem is to serialize the dellink operation.
>
> This commit replaces sk->sk_refcnt with the per-network-namespace
> variables nr_sk4 and nr_sk6. However, this change does not actually
> resolve the underlying issue, because it does not serialize dellink. As
> a result, the race condition can still occur.

You are right that v1 does not serialize dellink itself: if
RDMA_NLDEV_CMD_DELLINK and the NETDEV_UNREGISTER notifier run
concurrently for the same device, rxe_net_del() can be entered twice,
and that device would then drop two references instead of one.

What the per-net mutex does guarantee is that the socket pointer is
cleared under the lock and udp_tunnel_sock_release() is called at most
once per socket, so the refcount underflow / use-after-free from the
syzbot report can no longer occur. The remaining effect of the
unserialized dellink is that the shared socket could be released early
while another device in the namespace still uses it.

> One possible solution is to use rtnl_lock(). However, this is not an
> ideal approach, since there is ongoing work in the kernel community to
> reduce and eventually eliminate unnecessary uses of rtnl_lock().
>
> Another option is to introduce a static mutex specifically for dellink
> serialization. While this would likely solve the race, it is not an
> elegant solution and adds another global lock solely to work around this
> issue.

For that remaining window, instead of rtnl_lock() or a global mutex,
v2 makes rxe_net_del() idempotent per device with a test_and_set_bit()
in struct rxe_dev, so only the first invocation drops the pernet
socket references. This keeps the serialization at device granularity
without adding any global locking. If you would rather serialize
dellink itself, I am happy to respin that way.

I will send v2 shortly.

Thanks,
Serhat