[PATCH v2 1/2] dmaengine: idxd: Do not call destroy_workqueue with null idxd->wq

[PATCH v2 1/2] dmaengine: idxd: Do not call destroy_workqueue with null idxd->wq

[Steve Wahl]

Error paths within idxd_pci_probe_alloc and related functions end up
calling destroy_workqueue with a null pointer, from
idxd_conf_device_release via put_device, because that allocation has
not yet occurred when the error is hit.

This was encountered running in a kexec'd kdump kernel with reduced
resources, causing the "Device is HALTED!" branch in
idxd_device_init_reset to be taken.

In idxd_conf_device_release, check that the workqueue has been
allocated before trying to destroy it.

Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")

Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
---
v2: split into two patches as requested by Vinicius Costa

 drivers/dma/idxd/sysfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 6d251095c350..d5ffc641c856 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1836,7 +1836,8 @@ static void idxd_conf_device_release(struct device *dev)
 {
 	struct idxd_device *idxd = confdev_to_idxd(dev);
 
-	destroy_workqueue(idxd->wq);
+	if (idxd->wq)
+		destroy_workqueue(idxd->wq);
 	kfree(idxd->groups);
 	bitmap_free(idxd->wq_enable_map);
 	kfree(idxd->wqs);
-- 
2.51.0

[PATCH v2 2/2] dmaengine: idxd: fix duplicate memory frees on initialization error path.

[Steve Wahl]

Error paths within idxd_pci_probe_alloc and related functions end up
attempting to free memory already freed from idxd_conf_device_release
via put_device.

This was encountered running in a kexec'd kdump kernel with reduced
resources, causing the "Device is HALTED!" branch in
idxd_device_init_reset to be taken.

In idxd_free and idxd_alloc, do not attempt to free allocations that
will already have been freed.

Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
---
v2: split into two patches as requested by Vinicius Costa

 drivers/dma/idxd/init.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index f1cfc7790d95..227e323cc5a0 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -607,9 +607,6 @@ static void idxd_free(struct idxd_device *idxd)
 		return;
 
 	put_device(idxd_confdev(idxd));
-	bitmap_free(idxd->opcap_bmap);
-	ida_free(&idxd_ida, idxd->id);
-	kfree(idxd);
 }
 
 static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
@@ -649,8 +646,13 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
 	return idxd;
 
 err_name:
+	/*
+	 * once device_initialize(conf_dev) is called,
+	 * put_device(conf_dev) will end up calling
+	 * idxd_conf_device_release() which will free the rest.
+	 */
 	put_device(conf_dev);
-	bitmap_free(idxd->opcap_bmap);
+	return NULL;
 err_opcap:
 	ida_free(&idxd_ida, idxd->id);
 err_ida:
-- 
2.51.0

Re: [PATCH v2 1/2] dmaengine: idxd: Do not call destroy_workqueue with null idxd->wq

[Vinicius Costa Gomes]

Steve Wahl <steve.wahl@hpe.com> writes:

> Error paths within idxd_pci_probe_alloc and related functions end up
> calling destroy_workqueue with a null pointer, from
> idxd_conf_device_release via put_device, because that allocation has
> not yet occurred when the error is hit.
>
> This was encountered running in a kexec'd kdump kernel with reduced
> resources, causing the "Device is HALTED!" branch in
> idxd_device_init_reset to be taken.
>
> In idxd_conf_device_release, check that the workqueue has been
> allocated before trying to destroy it.
>
> Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
>
> Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> ---



> v2: split into two patches as requested by Vinicius Costa
>
>  drivers/dma/idxd/sysfs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 6d251095c350..d5ffc641c856 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1836,7 +1836,8 @@ static void idxd_conf_device_release(struct device *dev)
>  {
>  	struct idxd_device *idxd = confdev_to_idxd(dev);
>  
> -	destroy_workqueue(idxd->wq);
> +	if (idxd->wq)
> +		destroy_workqueue(idxd->wq);
>  	kfree(idxd->groups);
>  	bitmap_free(idxd->wq_enable_map);
>  	kfree(idxd->wqs);
> -- 
> 2.51.0
>

-- 
Vinicius

Re: [PATCH v2 1/2] dmaengine: idxd: Do not call destroy_workqueue with null idxd->wq

[Vinicius Costa Gomes]

Steve Wahl <steve.wahl@hpe.com> writes:

> Error paths within idxd_pci_probe_alloc and related functions end up
> calling destroy_workqueue with a null pointer, from
> idxd_conf_device_release via put_device, because that allocation has
> not yet occurred when the error is hit.
>
> This was encountered running in a kexec'd kdump kernel with reduced
> resources, causing the "Device is HALTED!" branch in
> idxd_device_init_reset to be taken.
>
> In idxd_conf_device_release, check that the workqueue has been
> allocated before trying to destroy it.
>
> Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
>
> Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> ---

(for the earlier email, I meant to add this)

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
-- 
Vinicius

Re: [PATCH v2 2/2] dmaengine: idxd: fix duplicate memory frees on initialization error path.

[Vinicius Costa Gomes]

Steve Wahl <steve.wahl@hpe.com> writes:

> Error paths within idxd_pci_probe_alloc and related functions end up
> attempting to free memory already freed from idxd_conf_device_release
> via put_device.
>
> This was encountered running in a kexec'd kdump kernel with reduced
> resources, causing the "Device is HALTED!" branch in
> idxd_device_init_reset to be taken.
>
> In idxd_free and idxd_alloc, do not attempt to free allocations that
> will already have been freed.
>
> Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> ---

Bogdan Codres series (but submitted a bit later), tries to fix this
issue, has a better splat and a reproducer, I think it makes sense to
add the splat and reproducer here, and add Bogdan as Reported-by (if
agreed, of course).

I am wondering about adding a third patch for the dangling ->wq pointer, as
reported by Sashiko would make sense.

Something like this might do the job (totally untested):

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index f1cfc7790d95..0a74018f31a8 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -415,6 +415,7 @@ static void idxd_cleanup_internals(struct idxd_device *idxd)
 	idxd_clean_engines(idxd);
 	idxd_clean_wqs(idxd);
 	destroy_workqueue(idxd->wq);
+	idxd->wq = NULL;
 }

How does it sound?

> v2: split into two patches as requested by Vinicius Costa
>
>  drivers/dma/idxd/init.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index f1cfc7790d95..227e323cc5a0 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -607,9 +607,6 @@ static void idxd_free(struct idxd_device *idxd)
>  		return;
>  
>  	put_device(idxd_confdev(idxd));
> -	bitmap_free(idxd->opcap_bmap);
> -	ida_free(&idxd_ida, idxd->id);
> -	kfree(idxd);
>  }
>  
>  static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
> @@ -649,8 +646,13 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
>  	return idxd;
>  
>  err_name:
> +	/*
> +	 * once device_initialize(conf_dev) is called,
> +	 * put_device(conf_dev) will end up calling
> +	 * idxd_conf_device_release() which will free the rest.
> +	 */
>  	put_device(conf_dev);
> -	bitmap_free(idxd->opcap_bmap);
> +	return NULL;
>  err_opcap:
>  	ida_free(&idxd_ida, idxd->id);
>  err_ida:
> -- 
> 2.51.0
>

-- 
Vinicius

Re: [PATCH v2 2/2] dmaengine: idxd: fix duplicate memory frees on initialization error path.

[Steve Wahl]

On Mon, Jun 29, 2026 at 11:44:36AM +0000, Codres, Bogdan wrote:
> Hi Vinicius, Steve,
> 
> Thanks for the heads-up. I'm totally fine with being added as:
> 
> Reported-by: Bogdan Codres <bogdan.codres@windriver.com>
> 
> on Steve's patch — his fix addresses the same issue I was seeing.
> You can use my splat as the reproducer.
> 
> Regarding the third patch for the dangling ->wq pointer — the proposed change looks reasonable to me.
> 
> Best regards,
> Bogdan
> 
> Ph.D. eng. Bogdan Codres
> Member of Technical Staff in Cloud Platform Engineering, Wind River
> 

Hello, everyone,

As there's no code changes (and I even find it remarkable how similar
the two were, I think only the comments differed), I am completely
open to Vinicius just modifying the commit message to include the
parts he wants to see.  It would take less time than another round of
posting a patch.  But if you need more from me, let me know.

Thanks,

--> Steve

> ________________________________
> From: Codres, Bogdan <Bogdan.Codres@windriver.com>
> Sent: Monday, June 29, 2026 14:32
> To: Vinicius Costa Gomes <vinicius.gomes@intel.com>; Steve Wahl <steve.wahl@hpe.com>; Steve Wahl <steve.wahl@hpe.com>; Dave Jiang <dave.jiang@intel.com>; Vinod Koul <vkoul@kernel.org>; Frank Li <Frank.Li@kernel.org>; dmaengine@vger.kernel.org <dmaengine@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> Cc: Russ Anderson <rja@hpe.com>; Dimitri Sivanich <sivanich@hpe.com>
> Subject: Re: [PATCH v2 2/2] dmaengine: idxd: fix duplicate memory frees on initialization error path.
> 
> Hi Vinicius, Steve,
> Thanks for the heads-up. I'm totally fine with being added as
> Reported-by: Bogdan Codres <bogdan.codres@windriver.com>
> on Steve's patch — his fix addresses the same issue I was seeing.
> You can use my splat ad the reproducer.
> Regarding the third patch for the dangling ->wq pointer — the idxd->wq = NULL after destroy_workqueue() looks reasonable to me
> Best regards, Bogdan
> 
> 
> Best Regards,
> 
> Ph.D. eng. Bogdan Codres
> 
> Member of Technical Staff in Cloud Platform Engineering, Wind River
> 
> ________________________________
> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Sent: Saturday, June 27, 2026 3:57
> To: Steve Wahl <steve.wahl@hpe.com>; Steve Wahl <steve.wahl@hpe.com>; Dave Jiang <dave.jiang@intel.com>; Vinod Koul <vkoul@kernel.org>; Frank Li <Frank.Li@kernel.org>; dmaengine@vger.kernel.org <dmaengine@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> Cc: Russ Anderson <rja@hpe.com>; Dimitri Sivanich <sivanich@hpe.com>; Codres, Bogdan <Bogdan.Codres@windriver.com>
> Subject: Re: [PATCH v2 2/2] dmaengine: idxd: fix duplicate memory frees on initialization error path.
> 
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> Steve Wahl <steve.wahl@hpe.com> writes:
> 
> > Error paths within idxd_pci_probe_alloc and related functions end up
> > attempting to free memory already freed from idxd_conf_device_release
> > via put_device.
> >
> > This was encountered running in a kexec'd kdump kernel with reduced
> > resources, causing the "Device is HALTED!" branch in
> > idxd_device_init_reset to be taken.
> >
> > In idxd_free and idxd_alloc, do not attempt to free allocations that
> > will already have been freed.
> >
> > Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> > ---
> 
> Bogdan Codres series (but submitted a bit later), tries to fix this
> issue, has a better splat and a reproducer, I think it makes sense to
> add the splat and reproducer here, and add Bogdan as Reported-by (if
> agreed, of course).
> 
> I am wondering about adding a third patch for the dangling ->wq pointer, as
> reported by Sashiko would make sense.
> 
> Something like this might do the job (totally untested):
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index f1cfc7790d95..0a74018f31a8 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -415,6 +415,7 @@ static void idxd_cleanup_internals(struct idxd_device *idxd)
>         idxd_clean_engines(idxd);
>         idxd_clean_wqs(idxd);
>         destroy_workqueue(idxd->wq);
> +       idxd->wq = NULL;
>  }
> 
> How does it sound?
> 
> > v2: split into two patches as requested by Vinicius Costa
> >
> >  drivers/dma/idxd/init.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> > index f1cfc7790d95..227e323cc5a0 100644
> > --- a/drivers/dma/idxd/init.c
> > +++ b/drivers/dma/idxd/init.c
> > @@ -607,9 +607,6 @@ static void idxd_free(struct idxd_device *idxd)
> >               return;
> >
> >       put_device(idxd_confdev(idxd));
> > -     bitmap_free(idxd->opcap_bmap);
> > -     ida_free(&idxd_ida, idxd->id);
> > -     kfree(idxd);
> >  }
> >
> >  static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
> > @@ -649,8 +646,13 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
> >       return idxd;
> >
> >  err_name:
> > +     /*
> > +      * once device_initialize(conf_dev) is called,
> > +      * put_device(conf_dev) will end up calling
> > +      * idxd_conf_device_release() which will free the rest.
> > +      */
> >       put_device(conf_dev);
> > -     bitmap_free(idxd->opcap_bmap);
> > +     return NULL;
> >  err_opcap:
> >       ida_free(&idxd_ida, idxd->id);
> >  err_ida:
> > --
> > 2.51.0
> >
> 
> --
> Vinicius

-- 
Steve Wahl, Hewlett Packard Enterprise