Re: [PATCH rdma-next 06/13] net/smc: Look up the pnetid ib device within the net namespace

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing TOCTOU and Use-After-Free race where `smc_pnet_find_ib()` and `smc_pnet_find_smcd()` return un-refcounted device pointers.
- [High] Pre-existing Use-After-Free during SMC connection setup due to un-refcounted `ib_dev` pointer in `smc_init_info`.
--

--- Patch [6]: [PATCH rdma-next 06/13] net/smc: Look up the pnetid ib device within the net namespace ---
commit d209b8afd09f2213c9477f5a717fdf012a7c2077
Author: Jiri Pirko <jiri@resnulli.us>

net/smc: Look up the pnetid ib device within the net namespace

This commit scopes smc_pnet_find_ib() to the caller's net namespace. It
ensures that pnetid setup cannot bind to a same-named RDMA device from
another namespace.

> diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
> index 63e286e2dfaa8..ff9c9c35cc2f5 100644
> --- a/net/smc/smc_pnet.c
> +++ b/net/smc/smc_pnet.c
> @@ -304,13 +304,18 @@ static bool smc_pnetid_valid(const char *pnet_name, char *pnetid)
>  	return true;
>  }
>  
> -/* Find an infiniband device by a given name. The device might not exist. */
> -static struct smc_ib_device *smc_pnet_find_ib(char *ib_name)
> +/*
> + * Find an infiniband device by a given name, restricted to the devices
> + * accessible from @net. The device might not exist.
> + */
> +static struct smc_ib_device *smc_pnet_find_ib(struct net *net, char *ib_name)
>  {
>  	struct smc_ib_device *ibdev;
>  
>  	mutex_lock(&smc_ib_devices.mutex);
>  	list_for_each_entry(ibdev, &smc_ib_devices.list, list) {
> +		if (!rdma_dev_access_netns(ibdev->ibdev, net))
> +			continue;

[ ... ]

> @@ -419,7 +424,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
>  	bool new_ibdev;
>  
>  	/* try to apply the pnetid to active devices */
> -	ib_dev = smc_pnet_find_ib(ib_name);
> +	ib_dev = smc_pnet_find_ib(net, ib_name);
>  	if (ib_dev) {
>  		ibdev_applied = smc_pnet_apply_ib(ib_dev, ib_port, pnet_name);
>  		if (ibdev_applied)

[Severity: High]
This isn't a bug introduced by this patch, but smc_pnet_find_ib() in
net/smc/smc_pnet.c appears to return an un-refcounted device pointer.

If a user configures a PNETID for an IB or SMCD device via netlink and calls
smc_pnet_add_ib(), it locates the device in the global list while holding
smc_ib_devices.mutex. However, smc_pnet_find_ib() returns the raw pointer
after dropping the mutex.

If the device is concurrently hot-unplugged, smc_ib_remove_dev() or
smcd_unregister_dev() will acquire the mutex, remove the device from the
list, and kfree() the device structure.

Could the original netlink thread then continue execution and dereference the
now-freed pointer inside smc_pnet_apply_ib() or when printing
ib_dev->ibdev->name?

[Severity: High]
This is another pre-existing issue in net/smc/smc_pnet.c related to
un-refcounted pointers, but it happens during connection setup.

When a user calls connect() on an AF_SMC socket,
_smc_pnet_find_roce_by_pnetid() locates an active IB device while holding
smc_ib_devices.mutex, and stores its pointer in the smc_init_info structure
(ini->ib_dev) before dropping the mutex.

Connection establishment then continues across multiple function calls:
smc_find_proposal_devices() -> smc_connect_rdma() -> smc_conn_create()
using this un-refcounted pointer.

If the IB device is concurrently hot-unplugged, smc_ib_remove_dev() will
kfree() the device. Might the connection setup path then dereference the
freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709095532.855647-1-jiri@resnulli.us?part=6