Re: [PATCH net 08/13] idpf: fix double free and use-after-free in aux device error paths

[Jacob Keller <jacob.e.keller@intel.com>]

On 5/4/2026 10:14 PM, Jacob Keller wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> When auxiliary_device_add() fails in idpf_plug_vport_aux_dev() or
> idpf_plug_core_aux_dev(), the err_aux_dev_add label calls
> auxiliary_device_uninit() and falls through to err_aux_dev_init.  The
> uninit call will trigger put_device(), which invokes the release
> callback (idpf_vport_adev_release / idpf_core_adev_release) that frees
> iadev.  The fall-through then reads adev->id from the freed iadev for
> ida_free() and double-frees iadev with kfree().
> 
> Free the IDA slot and clear the back-pointer before uninit, while adev
> is still valid, then return immediately.
> 
> Commit 65637c3a1811 ("idpf: fix UAF in RDMA core aux dev deinitialization")
> fixed the same use-after-free in the matching unplug path in this file but
> missed both probe error paths.
> 
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: stable@kernel.org
> Fixes: be91128c579c ("idpf: implement RDMA vport auxiliary dev create, init, and destroy")
> Fixes: f4312e6bfa2a ("idpf: implement core RDMA auxiliary dev create, init, and destroy")
> Assisted-by: gregkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/idpf/idpf_idc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_idc.c b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> index 7e4f4ac92653..b7d6b08fc89e 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_idc.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> @@ -90,7 +90,10 @@ static int idpf_plug_vport_aux_dev(struct iidc_rdma_core_dev_info *cdev_info,
>  	return 0;
>  

Sashiko points out some possible flaws in the idpf_plug_vport_aux_dev
function that are unrelated to this change:

> This isn't an issue introduced by this patch, but does earlier code prematurely
> publish vdev_info->adev before initialization is complete?
> vdev_info->adev is set near the beginning of idpf_plug_vport_aux_dev():
> 	adev = &iadev->adev;
> 	vdev_info->adev = &iadev->adev;
> 	iadev->vdev_info = vdev_info;
> 	...
> 	ret = auxiliary_device_init(adev);
> If a concurrent MTU event triggers idpf_idc_vdev_mtu_event(), it will read
> the pointer locklessly:
> void idpf_idc_vdev_mtu_event(...)
> {
> 	...
> 	device_lock(&vdev_info->adev->dev);
> 	adev = vdev_info->adev;
> 	if (!adev || !adev->dev.driver)
> 		goto unlock;
> If device_initialize() hasn't executed yet, the device's mutex is uninitialized
> and attempting to lock it could result in a panic.
> Furthermore, if auxiliary_device_add() fails and the error path sets the
> pointer to NULL and frees the device, could a concurrent reader that fetched
> the pointer just before it was cleared suffer a use-after-free when attempting
> to lock the freed device?
> Should the assignment to vdev_info->adev be deferred until after the device has
> been successfully initialized and added?
> Also, in idpf_idc_vdev_mtu_event(), is it possible for vdev_info->adev to be
> NULL when device_lock() is called?
> 	device_lock(&vdev_info->adev->dev);
> 	adev = vdev_info->adev;
> 	if (!adev || !adev->dev.driver)
> 		goto unlock;
> If vdev_info->adev is NULL, evaluating &vdev_info->adev->dev evaluates to
> NULL, which is then passed to mutex_lock() inside device_lock(). The subsequent
> check for (!adev) indicates this NULL state is anticipated. Should the
> NULL check happen before attempting to acquire the lock?

I do not believe these should block this fix, and we'll need to have
someone from the idpf team review this code and determine if this is
valid and find a fix for it.

Thanks,
Jake