Re: [PATCH net-next v6 02/19] psp: base PSP device support

[Paolo Abeni <pabeni@redhat.com>]

On 8/14/25 5:14 PM, Daniel Zahka wrote:
> On 8/14/25 10:21 AM, Paolo Abeni wrote:
>> On 8/12/25 2:29 AM, Daniel Zahka wrote:
>>> +/**
>>> + * psp_dev_unregister() - unregister PSP device
>>> + * @psd:	PSP device structure
>>> + */
>>> +void psp_dev_unregister(struct psp_dev *psd)
>>> +{
>>> +	mutex_lock(&psp_devs_lock);
>>> +	mutex_lock(&psd->lock);
>>> +
>>> +	psp_nl_notify_dev(psd, PSP_CMD_DEV_DEL_NTF);
>>> +	xa_store(&psp_devs, psd->id, NULL, GFP_KERNEL);
>> It's not 110% obvious to me that the above is equivalent to xa_clear(),
>> given the XA_FLAGS_ALLOC1 init flag. If you have to re-submit, please
>> consider using xa_clear() instead.
> 
> This was actually a deliberate decision to use xa_store() with NULL in 
> psp_dev_unregister(), and then call xa_erase() after from 
> psp_dev_destroy(). psp_dev_unregister() is called synchronously by 
> drivers to uniniatialize psp, whereas psp_dev_destroy() is called once 
> the refcount of a psp_dev goes to 0. A system could have multiple psp 
> NICs, in which case policy checks at the socket layer need to compare 
> the pair of (spi, psp dev id), as opposed to just the spi.
> 
> What we were going for with this decision was to try and prevent an 
> attacker from trying to quickly trigger or wait for 
> psp_dev_unregister(), and then try to bring up a new psp device with the 
> same psp_dev id, while a socket may still be holding a reference to the 
> old psp device. So we delay calling xa_erase() until after all 
> references to the old psp_dev are gone to release the id (xa_array slot).
> 
> Perhaps I can add a comment, because I can see how that would trip up 
> readers.

Indeed the above was completely unobvious to me, and definitely a
comment could help long term maintenance.

Thanks,

Paolo