public inbox for intel-wired-lan@osuosl.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Paul Menzel <pmenzel@molgen.mpg.de>
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	stable <stable@kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH net] idpf: fix double free and use-after-free in aux device error paths
Date: Tue, 14 Apr 2026 10:00:06 +0200	[thread overview]
Message-ID: <2026041432-tapestry-condition-22ff@gregkh> (raw)
In-Reply-To: <afefe8b5-5bd9-4019-9d12-5ee2a7f577a2@molgen.mpg.de>

On Tue, Apr 14, 2026 at 08:54:55AM +0200, Paul Menzel wrote:
> Dear Greg,
> 
> 
> Thank you for the patch.
> 
> Am 11.04.26 um 12:12 schrieb Greg Kroah-Hartman:
> > 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 65637c3a1811 ("idpf: fix UAF in RDMA core aux dev
> 
> The commit hash is pasted twice.

Argh, when I cut/paste from my terminal that happened, my fault.

> > 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 <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>
> > ---
> > Note, these cleanup paths are messy, but I couldn't see a simpler way
> > without a lot more rework, so I choose the simple way :)
> > 
> >   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;
> >   err_aux_dev_add:
> > +	ida_free(&idpf_idc_ida, adev->id);
> > +	vdev_info->adev = NULL;
> >   	auxiliary_device_uninit(adev);
> > +	return ret;
> >   err_aux_dev_init:
> >   	ida_free(&idpf_idc_ida, adev->id);
> >   err_ida_alloc:
> > @@ -228,7 +231,10 @@ static int idpf_plug_core_aux_dev(struct iidc_rdma_core_dev_info *cdev_info)
> >   	return 0;
> >   err_aux_dev_add:
> > +	ida_free(&idpf_idc_ida, adev->id);
> > +	cdev_info->adev = NULL;
> >   	auxiliary_device_uninit(adev);
> > +	return ret;
> >   err_aux_dev_init:
> >   	ida_free(&idpf_idc_ida, adev->id);
> >   err_ida_alloc:
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> gemini/gemini-3.1-pro-preview has two comments [1]. Maybe the driver
> developers could judge their relevance.

These "pre-existing" reports are getting annoying.  While they are nice
to see for driver authors, it makes developers sending bug fixes in feel
like they are forced to do "more".  I think they are trying to tune this
to be a bit more sane...

thanks,

greg k-h

      reply	other threads:[~2026-04-14  8:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-11 10:12 [Intel-wired-lan] [PATCH net] idpf: fix double free and use-after-free in aux device error paths Greg Kroah-Hartman
2026-04-13 11:06 ` Loktionov, Aleksandr
2026-04-14  0:46 ` Jacob Keller
2026-04-14  6:54 ` Paul Menzel
2026-04-14  8:00   ` Greg Kroah-Hartman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026041432-tapestry-condition-22ff@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pmenzel@molgen.mpg.de \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox