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

[Alistair <alistair@alistair23.me>]

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