Re: [Intel-wired-lan] [PATCH v2] dpf: fix UAF and double free in idpf_plug_vport_aux_dev() error path

[Jacob Keller]

On 4/13/2026 4:20 AM, Guangshuo Li wrote:
> If auxiliary_device_add() fails, idpf_plug_vport_aux_dev() calls
> auxiliary_device_uninit(adev), whose release callback
> idpf_vport_adev_release() frees the containing
> struct iidc_rdma_vport_auxiliary_dev.
> 
> The current error path then accesses adev->id and later frees iadev
> again, which may lead to a use-after-free and double free.
> 
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
> 
> Fix it by storing the allocated auxiliary device id in a local
> variable and avoiding direct freeing of iadev after
> auxiliary_device_uninit().
> 
> Fixes: be91128c579c ("idpf: implement RDMA vport auxiliary dev create, init, and destroy")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---

This doesn't look right. The commit message analysis seems to match this
fix from Greg KH:

https://lore.kernel.org/intel-wired-lan/2026041432-tapestry-condition-22ff@gregkh/

But the changes do not make any sense to me. It looks like a poorly done
AI-generated "fix" which is not correct. Greg's version does look like
it properly resolves this.

> v2:
>   - note that the issue was identified by my static analysis tool
>   - and confirmed by manual review
> 

What even is this change log?? I see that version was sent and everyone
else was sane enough to just silently reject or ignore the v1...

>  drivers/net/ethernet/intel/idpf/idpf_idc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_idc.c b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> index 6dad0593f7f2..2a18907643fc 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_idc.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> @@ -59,6 +59,7 @@ static int idpf_plug_vport_aux_dev(struct iidc_rdma_core_dev_info *cdev_info,
>  	char name[IDPF_IDC_MAX_ADEV_NAME_LEN];
>  	struct auxiliary_device *adev;
>  	int ret;
> +	int adev_id;
>  

You create a local variable here...

>  	iadev = kzalloc(sizeof(*iadev), GFP_KERNEL);
>  	if (!iadev)
> @@ -74,11 +75,14 @@ static int idpf_plug_vport_aux_dev(struct iidc_rdma_core_dev_info *cdev_info,
>  		goto err_ida_alloc;
>  	}
>  	adev->id = ret;
> +	adev->id = adev_id;

adev_is is never initialized, so you assign a random garbage
uninitialized value. This is obviously wrong and will lead to worse
errors than the failed cleanup.

I'm rejecting this patch in favor of the clearly appropriate fix from Greg.

>  	adev->dev.release = idpf_vport_adev_release;
>  	adev->dev.parent = &cdev_info->pdev->dev;
>  	sprintf(name, "%04x.rdma.vdev", cdev_info->pdev->vendor);
>  	adev->name = name;
>  
> +	/* iadev is owned by the auxiliary device */
> +	iadev = NULL;>  	ret = auxiliary_device_init(adev);
>  	if (ret)
>  		goto err_aux_dev_init;
> @@ -92,7 +96,7 @@ static int idpf_plug_vport_aux_dev(struct iidc_rdma_core_dev_info *cdev_info,
>  err_aux_dev_add:
>  	auxiliary_device_uninit(adev);
>  err_aux_dev_init:
> -	ida_free(&idpf_idc_ida, adev->id);
> +	ida_free(&idpf_idc_ida, adev_id);
>  err_ida_alloc:
>  	vdev_info->adev = NULL;
>  	kfree(iadev);