Re: [PATCH net 3/3] netdevsim: psp: rcu protect psp_dev reference

[Willem de Bruijn]

Daniel Zahka wrote:
> There are two issues with the way psp_dev is used in nsim_do_psp():
> 
> 1. There is no check for IS_ERR() on the peers psp_dev, before
>    dereferencing.
> 2. The refcount on this psp_dev can be dropped by
>    nsim_psp_rereg_write()
> 
> To fix this, we can make netdevsim's reference to its psp_dev an rcu
> reference, and then nsim_do_psp() can read the fields it needs from an
> rcu critical section.
> 
> Fixes: f857478d6206 ("netdevsim: a basic test PSP implementation")
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Daniel Zahka <daniel.zahka@xxxxxxxxx>

>  static ssize_t
> @@ -228,16 +237,23 @@ nsim_psp_rereg_write(struct file *file, const char __user *data, size_t count,
>  		     loff_t *ppos)
>  {
>  	struct netdevsim *ns = file->private_data;
> -	int err;
> +	struct psp_dev *psd;
> +	ssize_t ret;
>  
>  	mutex_lock(&ns->psp.rereg_lock);
> -	__nsim_psp_uninit(ns);
> +	__nsim_psp_uninit(ns, false);
> +
> +	psd = psp_dev_create(ns->netdev, &nsim_psp_ops, &nsim_psp_caps, ns);
> +	if (IS_ERR(psd)) {
> +		ret = PTR_ERR(psd);
> +		goto out;
> +	}

Do you want to create the new device first and only delete the old
state if that succeeds? To avoid a netdevsim in state without dev.

>  
> -	ns->psp.dev = psp_dev_create(ns->netdev, &nsim_psp_ops,
> -				     &nsim_psp_caps, ns);
> -	err = PTR_ERR_OR_ZERO(ns->psp.dev);
> +	rcu_assign_pointer(ns->psp.dev, psd);
> +	ret = count;
> +out:
>  	mutex_unlock(&ns->psp.rereg_lock);
> -	return err ?: count;
> +	return ret;
>  }
>