dmaengine: idxd: fix problems on initialization error path.

[PATCH] dmaengine: idxd: fix problems on initialization error path.

Posted by Steve Wahl 4 days, 8 hours ago

Some error paths within idxd_pci_probe_alloc and functions it calls
did not keep proper track of what has already been allocated or freed,
resulting in calling destroy_workqueue with a null pointer, and once
that was fixed, attempting to free structures more than once.  These
conditions were hit 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.  And in idxd_free and
idxd_alloc, do not attempt to free allocations that
idxd_conf_device_release, called through put_device, will already have
freed.

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

Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
---
 drivers/dma/idxd/init.c  | 10 ++++++----
 drivers/dma/idxd/sysfs.c |  3 ++-
 2 files changed, 8 insertions(+), 5 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:
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.43.0

Re: [PATCH] dmaengine: idxd: fix problems on initialization error path.

Posted by Vinicius Costa Gomes 4 days, 2 hours ago

Hi Steve,

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

> Some error paths within idxd_pci_probe_alloc and functions it calls
> did not keep proper track of what has already been allocated or freed,
> resulting in calling destroy_workqueue with a null pointer, and once
> that was fixed, attempting to free structures more than once.  These
> conditions were hit 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.  And in idxd_free and
> idxd_alloc, do not attempt to free allocations that
> idxd_conf_device_release, called through put_device, will already have
> freed.
>
> Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
>
> Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> ---
>  drivers/dma/idxd/init.c  | 10 ++++++----
>  drivers/dma/idxd/sysfs.c |  3 ++-
>  2 files changed, 8 insertions(+), 5 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:

I think that this first part should be a separate patch.

> 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);

And this another.


Cheers,
-- 
Vinicius

Re: [PATCH] dmaengine: idxd: fix problems on initialization error path.

Posted by Steve Wahl 3 days, 6 hours ago

On Wed, May 20, 2026 at 01:10:03PM -0700, Vinicius Costa Gomes wrote:
> Hi Steve,
> 
> Steve Wahl <steve.wahl@hpe.com> writes:
> 
> > Some error paths within idxd_pci_probe_alloc and functions it calls
> > did not keep proper track of what has already been allocated or freed,
> > resulting in calling destroy_workqueue with a null pointer, and once
> > that was fixed, attempting to free structures more than once.  These
> > conditions were hit 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.  And in idxd_free and
> > idxd_alloc, do not attempt to free allocations that
> > idxd_conf_device_release, called through put_device, will already have
> > freed.
> >
> > Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
> >
> > Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> > ---
> >  drivers/dma/idxd/init.c  | 10 ++++++----
> >  drivers/dma/idxd/sysfs.c |  3 ++-
> >  2 files changed, 8 insertions(+), 5 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:
> 
> I think that this first part should be a separate patch.
> 
> > 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);
> 
> And this another.

I can split it as you desire.

--> Steve

-- 
Steve Wahl, Hewlett Packard Enterprise