Re: [PATCH rdma-next v3 06/14] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] `smc_pnet_find_ib()` and `smc_pnet_find_smcd()` return unreferenced pointers leading to a Use-After-Free during concurrent device removal.
--

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

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

Scope smc_pnet_find_ib() to the caller's net namespace so pnetid setup
cannot bind to a same-named RDMA device from another namespace once names
become per-netns.

> 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

[ ... ]

> @@ -408,8 +413,8 @@ static int smc_pnet_add_eth(struct smc_pnettable *pnettable, struct net *net,
>  	return rc;
>  }
>  
> -static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
> -			   u8 ib_port, char *pnet_name)
> +static int smc_pnet_add_ib(struct smc_pnettable *pnettable, struct net *net,
> +			   char *ib_name, u8 ib_port, char *pnet_name)
>  {
>  	struct smc_pnetentry *tmp_pe, *new_pe;
>  	struct smc_ib_device *ib_dev;
> @@ -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);

[Severity: Critical]
This isn't a bug introduced by this patch, but does returning an unreferenced
pointer from smc_pnet_find_ib() create a use-after-free window?

Looking at smc_pnet_find_ib(), the function holds smc_ib_devices.mutex while
finding the device, but then drops the lock and returns the raw ibdev pointer:

net/smc/smc_pnet.c:smc_pnet_find_ib() {
    ...
    out:
        mutex_unlock(&smc_ib_devices.mutex);
        return ibdev;
}

If a concurrent device removal path acquires the mutex, removes the device
from the list, and frees the struct smc_ib_device via kfree(), could the
netlink thread subsequently dereference the freed pointer?

>  	if (ib_dev) {
>  		ibdev_applied = smc_pnet_apply_ib(ib_dev, ib_port, pnet_name);
>  		if (ibdev_applied)

For example, smc_pnet_apply_ib() re-acquires the mutex and writes to
ib_dev->pnetid, and the caller then reads ib_dev->ibdev->name for logging.

Does this same unreferenced pointer pattern also affect smc_pnet_find_smcd()
when it is called immediately after?

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