* Re: [PATCH v2] PCI/DOE: Fix double free of a duplicate feature's sysfs name
2026-10-03 10:22 [PATCH v2] PCI/DOE: Fix double free of a duplicate feature's sysfs name Donggeun Yoo
@ 2026-10-04 16:14 ` Jonathan Cameron
2026-10-05 16:20 ` Alistair
2026-10-05 23:21 ` Bjorn Helgaas
2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-10-04 16:14 UTC (permalink / raw)
To: Donggeun Yoo; +Cc: bhelgaas, alistair, linux-pci, linux-kernel, stable
On Sat, 3 Oct 2026 19:22:21 +0900
Donggeun Yoo <donggeunyoo.kernel@gmail.com> 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
>
> Clear the pointer after freeing the name.
>
> Fixes: 2311ab1820fe ("PCI/DOE: Expose DOE features via sysfs")
> Cc: stable@vger.kernel.org
> Suggested-by: Alistair Francis <alistair@alistair23.me>
> Assisted-by: LLM
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
> Changes in v2:
> - Keep freeing the duplicate's name in pci_doe_sysfs_feature_populate()
> and clear the pointer, instead of leaving it to
> pci_doe_sysfs_feature_remove() (Alistair Francis)
>
> v1: https://lore.kernel.org/all/20260928035035.252394-1-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
This one and related are valid cases that real hardware might well do.
> 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
So this one is a hardware bug I think. I guess maybe the DOE spec
may not strictly say you can't do this (I'm on wrong computer
so haven't checked) but it would be very odd.
Overall I'm never keen on code that relies on an error path to smooth
over a valid condition. We can't deduplicate in the xarrays for the
multimailbox case unless we do a combine of the xarrays and use
that instead for the registration.
That route would be a rather heavy weight fix though so I guess I don't mind
this one that much.
So argued myself around to this approach.
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
>
> The doe_features listing is the same before and after in every case.
>
> drivers/pci/doe.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index ac95b1d2d9997..6b5d9ba8e9697 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -208,8 +208,9 @@ 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);
> + }
> + kfree(attrs[i].attr.name);
> + attrs[i].attr.name = NULL;
> }
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] PCI/DOE: Fix double free of a duplicate feature's sysfs name
2026-10-03 10:22 [PATCH v2] PCI/DOE: Fix double free of a duplicate feature's sysfs name Donggeun Yoo
2026-10-04 16:14 ` Jonathan Cameron
@ 2026-10-05 16:20 ` Alistair
2026-10-05 23:21 ` Bjorn Helgaas
2 siblings, 0 replies; 4+ messages in thread
From: Alistair @ 2026-10-05 16:20 UTC (permalink / raw)
To: Donggeun Yoo, bhelgaas; +Cc: jic23, linux-pci, linux-kernel, stable
On Sat, 2026-10-03 at 19:22 +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
>
> Clear the pointer after freeing the name.
>
> Fixes: 2311ab1820fe ("PCI/DOE: Expose DOE features via sysfs")
> Cc: stable@vger.kernel.org
> Suggested-by: Alistair Francis <alistair@alistair23.me>
> Assisted-by: LLM
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
Alistair
> ---
> Changes in v2:
> - Keep freeing the duplicate's name in
> pci_doe_sysfs_feature_populate()
> and clear the pointer, instead of leaving it to
> pci_doe_sysfs_feature_remove() (Alistair Francis)
>
> v1:
> https://lore.kernel.org/all/20260928035035.252394-1-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 | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index ac95b1d2d9997..6b5d9ba8e9697 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -208,8 +208,9 @@ 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);
> + }
> + kfree(attrs[i].attr.name);
> + attrs[i].attr.name = NULL;
> }
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] PCI/DOE: Fix double free of a duplicate feature's sysfs name
2026-10-03 10:22 [PATCH v2] PCI/DOE: Fix double free of a duplicate feature's sysfs name Donggeun Yoo
2026-10-04 16:14 ` Jonathan Cameron
2026-10-05 16:20 ` Alistair
@ 2026-10-05 23:21 ` Bjorn Helgaas
2 siblings, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2026-10-05 23:21 UTC (permalink / raw)
To: Donggeun Yoo; +Cc: bhelgaas, alistair, jic23, linux-pci, linux-kernel, stable
On Sat, Oct 03, 2026 at 07:22:21PM +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
>
> Clear the pointer after freeing the name.
>
> Fixes: 2311ab1820fe ("PCI/DOE: Expose DOE features via sysfs")
> Cc: stable@vger.kernel.org
> Suggested-by: Alistair Francis <alistair@alistair23.me>
> Assisted-by: LLM
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Applied to pci/doe for v7.4, thanks!
> ---
> Changes in v2:
> - Keep freeing the duplicate's name in pci_doe_sysfs_feature_populate()
> and clear the pointer, instead of leaving it to
> pci_doe_sysfs_feature_remove() (Alistair Francis)
>
> v1: https://lore.kernel.org/all/20260928035035.252394-1-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 | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index ac95b1d2d9997..6b5d9ba8e9697 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -208,8 +208,9 @@ 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);
> + }
> + kfree(attrs[i].attr.name);
> + attrs[i].attr.name = NULL;
> }
> }
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread