Re: [PATCH net v2] idpf: handle NULL adev in idpf_idc_vdev_mtu_event

[Simon Horman <horms@kernel.org>]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
idpf: handle NULL adev in idpf_idc_vdev_mtu_event

This commit attempts to fix a NULL pointer dereference in the MTU event
handler by caching the auxiliary device pointer using READ_ONCE(). It avoids
dereferencing a NULL pointer if the device is cleared concurrently by
idpf_idc_vport_dev_ctrl().

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_idc.c b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> index b7d6b08fc89e8..9f764135507c2 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_idc.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> @@ -162,9 +162,12 @@ void idpf_idc_vdev_mtu_event(struct iidc_rdma_vport_dev_info *vdev_info,
>  
>  	set_bit(event_type, event.type);
>  
> -	device_lock(&vdev_info->adev->dev);
> -	adev = vdev_info->adev;
> -	if (!adev || !adev->dev.driver)
> +	adev = READ_ONCE(vdev_info->adev);
> +	if (!adev)
> +		return;
> +
> +	device_lock(&adev->dev);

The commit message indicates that idpf_idc_vport_dev_ctrl() and MTU changes
can run concurrently. Does caching the adev pointer with READ_ONCE() provide
enough protection against the device memory being freed?

If idpf_idc_vport_dev_ctrl() unregisters and frees the auxiliary device
immediately after READ_ONCE() reads a non-NULL pointer, could
device_lock(&adev->dev) then operate on a freed device?

Would it be necessary to increment the device's reference count, perhaps
using get_device() or holding a broader lock, before accessing adev->dev to
prevent a use-after-free regression?