[PATCH] PCI/DOE: Fix double free of a duplicate feature's sysfs name

[PATCH] PCI/DOE: Fix double free of a duplicate feature's sysfs name

[Donggeun Yoo]

When a DOE feature is reported twice, sysfs_add_file_to_group() fails
with -EEXIST for the second copy and pci_doe_sysfs_feature_populate()
frees that attribute's name, but leaves the pointer in place.
pci_doe_sysfs_feature_remove() frees every name again when the device
is removed, or when a later feature fails to register, so the name is
freed twice:

  BUG: KASAN: double-free in pci_doe_sysfs_feature_remove+0x117/0x1b0
  Call Trace:
   kfree+0x11a/0x420
   pci_doe_sysfs_feature_remove+0x117/0x1b0
   pci_doe_sysfs_teardown+0x90/0xf0
   pci_remove_bus_device+0x128/0x2e0
   pci_stop_and_remove_bus_device_locked+0x1d/0x30
   remove_store+0xd2/0xf0

Leave the name to pci_doe_sysfs_feature_remove(), which already frees
it for every entry.

Fixes: 2311ab1820fe ("PCI/DOE: Expose DOE features via sysfs")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
Tested under QEMU (q35, cxl-type3, KASAN) on 72d3fcf802c4. The device
reports its CDAT feature once, so a test-only change either registers
its DOE capability as a second mailbox or inserts the feature N extra
times into one:

  case                                   before           after
  no duplicate, remove                   no report        no report
  no duplicate, a name fails at probe    no report        no report
  two mailboxes, remove                  double free      no report
  two mailboxes, 3 remove/rescan cycles  3 double frees   no report
  3 duplicates in one mailbox, remove    3 double frees   no report
  2 duplicates, a later name fails,      2 double frees   no report
    probed twice

The doe_features listing is the same before and after in every case.

 drivers/pci/doe.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
index ac95b1d2d9997..56605386c4c16 100644
--- a/drivers/pci/doe.c
+++ b/drivers/pci/doe.c
@@ -208,8 +208,7 @@ static int pci_doe_sysfs_feature_populate(struct pci_dev *pdev,
 				pci_warn(pdev, "Failed adding %s to sysfs group\n",
 					 attrs[i].attr.name);
 				goto fail;
-			} else
-				kfree(attrs[i].attr.name);
+			}
 		}
 	}
 
-- 
2.53.0

Re: [PATCH] PCI/DOE: Fix double free of a duplicate feature's sysfs name

[Alistair]

On Mon, 2026-09-28 at 12:50 +0900, Donggeun Yoo wrote:
> When a DOE feature is reported twice, sysfs_add_file_to_group() fails
> with -EEXIST for the second copy and pci_doe_sysfs_feature_populate()
> frees that attribute's name, but leaves the pointer in place.
> pci_doe_sysfs_feature_remove() frees every name again when the device
> is removed, or when a later feature fails to register, so the name is
> freed twice:
> 
>   BUG: KASAN: double-free in pci_doe_sysfs_feature_remove+0x117/0x1b0
>   Call Trace:
>    kfree+0x11a/0x420
>    pci_doe_sysfs_feature_remove+0x117/0x1b0
>    pci_doe_sysfs_teardown+0x90/0xf0
>    pci_remove_bus_device+0x128/0x2e0
>    pci_stop_and_remove_bus_device_locked+0x1d/0x30
>    remove_store+0xd2/0xf0
> 
> Leave the name to pci_doe_sysfs_feature_remove(), which already frees
> it for every entry.

What about adding a NULL check in pci_doe_sysfs_feature_remove()
instead? Seems silly to keep duplicate but unused names allocated until
teardown.

Alistair

> 
> Fixes: 2311ab1820fe ("PCI/DOE: Expose DOE features via sysfs")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
> Tested under QEMU (q35, cxl-type3, KASAN) on 72d3fcf802c4. The device
> reports its CDAT feature once, so a test-only change either registers
> its DOE capability as a second mailbox or inserts the feature N extra
> times into one:
> 
>   case                                   before           after
>   no duplicate, remove                   no report        no report
>   no duplicate, a name fails at probe    no report        no report
>   two mailboxes, remove                  double free      no report
>   two mailboxes, 3 remove/rescan cycles  3 double frees   no report
>   3 duplicates in one mailbox, remove    3 double frees   no report
>   2 duplicates, a later name fails,      2 double frees   no report
>     probed twice
> 
> The doe_features listing is the same before and after in every case.
> 
>  drivers/pci/doe.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index ac95b1d2d9997..56605386c4c16 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -208,8 +208,7 @@ static int pci_doe_sysfs_feature_populate(struct
> pci_dev *pdev,
>  				pci_warn(pdev, "Failed adding %s to
> sysfs group\n",
>  					 attrs[i].attr.name);
>  				goto fail;
> -			} else
> -				kfree(attrs[i].attr.name);
> +			}
>  		}
>  	}
>  

Re: [PATCH] PCI/DOE: Fix double free of a duplicate feature's sysfs name

[Donggeun Yoo]

On Mon, 28 Sep 2026 15:19:32 +1000, Alistair wrote:
> On Mon, 2026-09-28 at 12:50 +0900, Donggeun Yoo wrote:
> > Leave the name to pci_doe_sysfs_feature_remove(), which already frees
> > it for every entry.
>
> What about adding a NULL check in pci_doe_sysfs_feature_remove()
> instead? Seems silly to keep duplicate but unused names allocated until
> teardown.

That works too, and I considered it. I went with dropping the kfree()
because freeing them in one place seemed reasonable, and I thought the
cost was small.

If there are no other views in a day or so, I'll send a v2 your way.

Thanks for the review!

Donggeun