From: 'Simon Horman' <[email protected]>

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?

Reply via email to