* [PATCH] iommu/riscv: Add dependency between iommu and devices
From: wang.yechao255 @ 2026-06-04 6:55 UTC (permalink / raw)
To: linux-acpi, linux-riscv, linux-kernel, iommu
Cc: sunilvl, rafael, lenb, pjw, palmer, aou, alex, tomasz.jeznach
From: Wang Yechao <wang.yechao255@zte.com.cn>
Commit 9156585280f1 ("ACPI: RIMT: Add dependency between iommu and
devices") adds the dependency between iommu and devices on ACPI
systems. On devicetree systems, the incorrect removal order also
occurs.
It can be reproduced on the QEMU RISC-V machine if the kernel enables
IOMMU_DMA:
[ 635.081530] e1000e: EEE TX LPI TIMER: 00000000
[ 656.100306] rcu: INFO: rcu_sched self-detected stall on CPU
[ 656.101374] rcu: 5-....: (5250 ticks this GP) idle=d774/1/0x4000000000000000 softirq=5173/5185 fqs=2625
[ 656.102237] rcu: (t=5251 jiffies g=36825 q=101 ncpus=16)
[ 656.103801] CPU: 5 UID: 0 PID: 1958 Comm: reboot Tainted: G W 7.1.0-rc5 #31 PREEMPTLAZY
[ 656.104127] Tainted: [W]=WARN
[ 656.104182] Hardware name: QEMU QEMU Virtual Machine, BIOS 2.7 02/02/2022
[ 656.104339] epc : riscv_iommu_cmd_sync.constprop.0+0xb8/0x148
[ 656.105352] ra : riscv_iommu_cmd_sync.constprop.0+0xa8/0x148
[ 656.105433] epc : ffffffff807ca980 ra : ffffffff807ca970 sp : ff60000085dbf960
[ 656.105475] gp : ffffffff81e0d798 tp : ff60000084b58e00 t0 : ffffffff80021048
[ 656.105514] t1 : ff60000081b18400 t2 : 45203a6530303031 s0 : ff60000085dbf9c0
[ 656.105554] s1 : 00000098c92a567c a0 : 00000098c03986f0 a1 : ff60000085dbf970
[ 656.105594] a2 : 000024bb5cac6aee a3 : ff200000004f1000 a4 : ff6000008140a040
[ 656.105632] a5 : 0000000000000669 a6 : 0000000000000000 a7 : 00000000ffffa000
[ 656.105669] s2 : 0000000000000000 s3 : 00000098c0398308 s4 : 000000000000066a
[ 656.105706] s5 : 0000000008f0d180 s6 : 000000a8d08b8de9 s7 : 0000000000001fff
[ 656.105743] s8 : ff6000008140a040 s9 : ff6000008484cb00 s10: ff200000005cc000
[ 656.105781] s11: ff600000814652a0 t3 : 000000f000000000 t4 : 0000000000000000
[ 656.105845] t5 : 0000000000000003 t6 : ff600000841666b0 ssp : 0000000000000000
[ 656.105883] status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
[ 656.106072] riscv_iommu_cmd_sync.constprop.0+0xb8/0x148
[ 656.106321] riscv_iommu_iotlb_inval+0x120/0x160
[ 656.106373] riscv_iommu_iotlb_sync+0x48/0x60
[ 656.106422] __iommu_dma_unmap+0xca/0xf8
[ 656.106470] iommu_dma_unmap_phys+0x58/0xc8
[ 656.106517] dma_unmap_phys+0x15c/0x248
[ 656.106564] dma_unmap_page_attrs+0x1e/0x30
[ 656.106915] e1000_clean_rx_ring+0x1d2/0x200 [e1000e]
[ 656.107668] e1000e_down+0x168/0x1c8 [e1000e]
[ 656.107995] e1000e_pm_freeze+0x94/0x128 [e1000e]
[ 656.108328] e1000_shutdown+0x28/0x48 [e1000e]
[ 656.108652] pci_device_shutdown+0x34/0x48
[ 656.108706] device_shutdown+0x104/0x1e8
[ 656.108752] kernel_restart+0x46/0xb8
[ 656.108797] __do_sys_reboot+0xc0/0x1c8
[ 656.108840] __riscv_sys_reboot+0x22/0x38
[ 656.108882] do_trap_ecall_u+0x236/0x3f8
[ 656.108947] handle_exception+0x15a/0x166
So move the device link into the iommu driver to fix both ACPI and
devicetree systems.
Fixes: 488ffbf18171 ("iommu/riscv: Paging domain support")
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
---
drivers/acpi/riscv/rimt.c | 7 -------
drivers/iommu/riscv/iommu.c | 7 +++++++
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/riscv/rimt.c b/drivers/acpi/riscv/rimt.c
index 906282b0e63c..229c4a0d47a3 100644
--- a/drivers/acpi/riscv/rimt.c
+++ b/drivers/acpi/riscv/rimt.c
@@ -263,13 +263,6 @@ static int rimt_iommu_xlate(struct device *dev, struct acpi_rimt_node *node, u32
if (!rimt_fwnode)
return -EPROBE_DEFER;
- /*
- * EPROBE_DEFER ensures IOMMU is probed before the devices that
- * depend on them. During shutdown, however, the IOMMU may be removed
- * first, leading to issues. To avoid this, a device link is added
- * which enforces the correct removal order.
- */
- device_link_add(dev, rimt_fwnode->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
return acpi_iommu_fwspec_init(dev, deviceid, rimt_fwnode);
}
diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
index a31f50bbad35..f9368e977346 100644
--- a/drivers/iommu/riscv/iommu.c
+++ b/drivers/iommu/riscv/iommu.c
@@ -1385,6 +1385,13 @@ static struct iommu_device *riscv_iommu_probe_device(struct device *dev)
dev_iommu_priv_set(dev, info);
+ /*
+ * During shutdown, however, the IOMMU may be removed first, leading
+ * to issues. To avoid this, a device link is added which enforces
+ * the correct removal order.
+ */
+ device_link_add(dev, fwspec->iommu_fwnode->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
+
return &iommu->iommu;
}
--
2.43.5
^ permalink raw reply related
* Re: [PATCH] ACPICA: Replace strncpy() with strscpy_pad() in acpi_ut_safe_strncpy()
From: Jiri Slaby @ 2026-06-04 6:32 UTC (permalink / raw)
To: Rafael J. Wysocki, Kees Cook
Cc: Robert Moore, Len Brown, linux-kernel, linux-acpi, acpica-devel,
linux-hardening
In-Reply-To: <79e9e913-0fb1-4110-804b-c3b5d0edafe4@kernel.org>
Ping?
On 29. 05. 26, 9:24, Jiri Slaby wrote:
> On 29. 05. 26, 9:20, Jiri Slaby wrote:
>> On 23. 03. 26, 18:31, Rafael J. Wysocki wrote:
>>> On Mon, Mar 23, 2026 at 6:24 PM Kees Cook <kees@kernel.org> wrote:
>>>>
>>>> Replace the deprecated[1] strncpy() with strscpy_pad() in
>>>> acpi_ut_safe_strncpy().
>>>>
>>>> The function is a "safe strncpy" wrapper that does
>>>> strncpy(dest, source, dest_size) followed by manual NUL-termination
>>>> at dest[dest_size - 1]. strscpy_pad() is a direct replacement: it
>>>> NUL-terminates, zero-pads the remainder, and the manual termination
>>>> is no longer needed.
>>>>
>>>> All callers pass NUL-terminated source strings (C string literals,
>>>> __FILE__ via ACPI_MODULE_NAME, or user-provided filenames that have
>>>> already been validated). The destinations are fixed-size char arrays
>>>> in ACPICA internal structures (allocation->module, aml_op_name,
>>>> acpi_gbl_db_debug_filename), all consumed as C strings.
>>>>
>>>> No behavioral change: strscpy_pad() produces identical output to
>>>> strncpy() + manual NUL-termination for NUL-terminated sources that
>>>> are shorter than dest_size. For sources longer than dest_size,
>>>> strncpy() wrote dest_size non-NUL bytes then the manual termination
>>>> overwrote the last byte with NUL; strscpy_pad() writes dest_size-1
>>>> bytes plus NUL: same result.
>>>>
>>>> Link: https://github.com/KSPP/linux/issues/90 [1]
>>>> Signed-off-by: Kees Cook <kees@kernel.org>
>>>> ---
>>>> This touches the ACPICA component shared with the upstream ACPICA
>>>> project (https://github.com/acpica/acpica), where the function
>>>> is named AcpiUtSafeStrncpy(). The upstream codebase uses its own
>>>> platform abstraction layer (acenv.h/acgcc.h) where I've mapped various
>>>> kernel APIs before like ACPI_FLEX_ARRAY and similar helpers. However,
>>>> acpi_ut_safe_strncpy() is an explicit function implementation rather
>>>> than a macro mapping, so the approach for upstreaming this change to
>>>> ACPICA is not clear. What's the best way to land this?
>>>
>>> I can apply this directly, it shouldn't be a major problem for porting
>>> patches from the upstream.
>>
>> As I reported in https://github.com/acpica/acpica/issues/1158 (but got
>> no reply), this patch breaks build of acpica against 7.1-rc*:
>> > ../../../../../drivers/acpi/acpica/utnonansi.c:171:9: error:
>> implicit declaration of function ‘strscpy_pad’ [-Wimplicit-function-
>> declaration]
>>
>> Is strscpy_pad() supposed to be emulated in acpica?
>
> No, I misread the log, it's the in-kernel acpidump failing to build:
>
> ~/linux/tools/power/acpi/tools/acpidump> make
> MKDIR include
> CP include
> CC tools/acpidump/apdump.o
> CC tools/acpidump/apfiles.o
> CC tools/acpidump/apmain.o
> CC tools/acpidump/osunixdir.o
> CC tools/acpidump/osunixmap.o
> CC tools/acpidump/osunixxf.o
> CC tools/acpidump/tbprint.o
> CC tools/acpidump/tbxfroot.o
> CC tools/acpidump/utascii.o
> CC tools/acpidump/utbuffer.o
> CC tools/acpidump/utcksum.o
> CC tools/acpidump/utdebug.o
> CC tools/acpidump/utexcep.o
> CC tools/acpidump/utglobal.o
> CC tools/acpidump/uthex.o
> CC tools/acpidump/utmath.o
> CC tools/acpidump/utnonansi.o
> ../../../../../drivers/acpi/acpica/utnonansi.c: In function
> ‘acpi_ut_safe_strncpy’:
> ../../../../../drivers/acpi/acpica/utnonansi.c:171:9: error: implicit
> declaration of function ‘strscpy_pad’ [-Wimplicit-function-declaration]
> 171 | strscpy_pad(dest, source, dest_size);
> | ^~~~~~~~~~~
> make: *** [../../Makefile.rules:25: /home/xslaby/linux/tools/power/acpi/
> tools/acpidump/utnonansi.o] Chyba 1
>
>>>> (This is one of the last users of strncpy in the kernel.)
>>>> ---
>>>> drivers/acpi/acpica/utnonansi.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/acpica/utnonansi.c b/drivers/acpi/acpica/
>>>> utnonansi.c
>>>> index ff0802ace19b..3a7952be6545 100644
>>>> --- a/drivers/acpi/acpica/utnonansi.c
>>>> +++ b/drivers/acpi/acpica/utnonansi.c
>>>> @@ -168,8 +168,7 @@ void acpi_ut_safe_strncpy(char *dest, char
>>>> *source, acpi_size dest_size)
>>>> {
>>>> /* Always terminate destination string */
>>>>
>>>> - strncpy(dest, source, dest_size);
>>>> - dest[dest_size - 1] = 0;
>>>> + strscpy_pad(dest, source, dest_size);
>>>> }
>>>>
>>>> #endif
>>>> --
>>>
>>
>> thanks,
>
--
js
suse labs
^ permalink raw reply
* Re: [PATCH v2 4/4] ACPI: NFIT: core: Fix possible deadlock and missing notifications
From: Dave Jiang @ 2026-06-03 23:10 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, nvdimm, Alison Schofield,
Xiang Chen
In-Reply-To: <3420096.aeNJFYEL58@rafael.j.wysocki>
On 6/3/26 10:58 AM, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> After commit 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before
> getting NFIT table"), ACPI NFIT driver removal may deadlock if an ACPI
> notify on the NFIT device is triggered concurrently. A similar deadlock
> may occur if an ACPI notify on the NFIT device is triggered during a
> failing driver probe.
>
> The deadlock is possible because acpi_dev_remove_notify_handler() calls
> acpi_os_wait_events_complete() after removing the notify handler and the
> driver core invokes it under the NFIT platform device lock which is also
> acquired by acpi_nfit_notify(). Thus acpi_os_wait_events_complete() may
> be waiting for acpi_nfit_notify() to complete, but the latter may not be
> able to acquire the device lock which is being held by the driver core
> while the former is being executed.
>
> Moreover, after commit 03667e146f81 ("ACPI: NFIT: core: Convert the
> driver to a platform one"), there are no sysfs notifications regarding
> NVDIMM devices because __acpi_nvdimm_notify() always bails out after
> checking the driver data pointer of the device's parent. That parent
> is the ACPI companion of the platform device used for driver binding,
> so its driver data pointer is always NULL after the commit in question
> which was overlooked by it.
>
> A remedy for the deadlock is to use a special separate lock for ACPI
> notify synchronization with driver probe and removal instead of the
> device lock of the NFIT device, while a remedy for the second issue
> is to populate the driver data pointer of the NFIT device's ACPI
> companion when the driver is ready to operate, so do both these things.
> However, since the new lock is not held across the entire teardown and
> acpi_nfit_notify() should do nothing when teardown is in progress, make
> it check the driver data pointer of the NFIT device's ACPI companion, in
> analogy with the existing check in __acpi_nvdimm_notify(), and bail out
> if that pointer is NULL.
>
> Fixes: 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before getting NFIT table")
> Fixes: 03667e146f81 ("ACPI: NFIT: core: Convert the driver to a platform one")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: All applicable <stable@vger.kernel.org>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/acpi/nfit/core.c | 63 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index aaa84ae7a20e..cb771d9cadb2 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -56,6 +56,8 @@ MODULE_PARM_DESC(force_labels, "Opt-in to labels despite missing methods");
> LIST_HEAD(acpi_descs);
> DEFINE_MUTEX(acpi_desc_lock);
>
> +DEFINE_MUTEX(acpi_notify_lock);
> +
> static struct workqueue_struct *nfit_wq;
>
> struct nfit_table_prev {
> @@ -1708,9 +1710,15 @@ static void acpi_nvdimm_notify(acpi_handle handle, u32 event, void *data)
> struct acpi_device *adev = data;
> struct device *dev = &adev->dev;
>
> - device_lock(dev->parent);
> + /*
> + * Locking is needed here for synchronization with driver probe and
> + * removal and the parent's driver data pointer is NULL when teardown
> + * is in progress (while the parent here is expected to be the ACPI
> + * companion of the platform device used for driver binding).
> + */
> + guard(mutex)(&acpi_notify_lock);
> +
> __acpi_nvdimm_notify(dev, event);
> - device_unlock(dev->parent);
> }
>
> static bool acpi_nvdimm_has_method(struct acpi_device *adev, char *method)
> @@ -3156,11 +3164,10 @@ EXPORT_SYMBOL_GPL(acpi_nfit_init);
> static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
> {
> struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
> - struct device *dev = acpi_desc->dev;
>
> - /* Bounce the device lock to flush acpi_nfit_add / acpi_nfit_notify */
> - device_lock(dev);
> - device_unlock(dev);
> + /* Bounce the notify lock to flush acpi_nfit_probe / acpi_nfit_notify */
> + mutex_lock(&acpi_notify_lock);
> + mutex_unlock(&acpi_notify_lock);
>
> /* Bounce the init_mutex to complete initial registration */
> mutex_lock(&acpi_desc->init_mutex);
> @@ -3292,10 +3299,17 @@ static void acpi_nfit_put_table(void *table)
> static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
> {
> struct device *dev = data;
> + struct acpi_device *adev = ACPI_COMPANION(dev);
>
> - device_lock(dev);
> - __acpi_nfit_notify(dev, handle, event);
> - device_unlock(dev);
> + /*
> + * Locking is needed here for synchronization with driver probe and
> + * removal and the ACPI companion's driver data pointer is NULL when
> + * teardown is in progress.
> + */
> + guard(mutex)(&acpi_notify_lock);
> +
> + if (dev_get_drvdata(&adev->dev))
> + __acpi_nfit_notify(dev, handle, event);
> }
>
> void acpi_nfit_shutdown(void *data)
> @@ -3337,11 +3351,18 @@ static int acpi_nfit_probe(struct platform_device *pdev)
> struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> struct acpi_nfit_desc *acpi_desc;
> struct device *dev = &pdev->dev;
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> struct acpi_table_header *tbl;
> acpi_status status = AE_OK;
> acpi_size sz;
> int rc = 0;
>
> + /*
> + * Prevent acpi_nfit_notify() from progressing until the probe is
> + * complete in case there is a concurrent event to process.
> + */
> + guard(mutex)(&acpi_notify_lock);
> +
> rc = devm_acpi_install_notify_handler(dev, ACPI_DEVICE_NOTIFY,
> acpi_nfit_notify, dev);
> if (rc)
> @@ -3357,6 +3378,11 @@ static int acpi_nfit_probe(struct platform_device *pdev)
> * data in the format of a series of NFIT Structures.
> */
> dev_dbg(dev, "failed to find NFIT at startup\n");
> + /*
> + * Let acpi_nfit_update_notify() run in case it will need to
> + * allocate the acpi_desc object.
> + */
> + dev_set_drvdata(&adev->dev, dev);
> return 0;
> }
>
> @@ -3374,7 +3400,7 @@ static int acpi_nfit_probe(struct platform_device *pdev)
> acpi_desc->acpi_header = *tbl;
>
> /* Evaluate _FIT and override with that if present */
> - status = acpi_evaluate_object(ACPI_HANDLE(dev), "_FIT", NULL, &buf);
> + status = acpi_evaluate_object(adev->handle, "_FIT", NULL, &buf);
> if (ACPI_SUCCESS(status) && buf.length > 0) {
> union acpi_object *obj = buf.pointer;
>
> @@ -3391,14 +3417,27 @@ static int acpi_nfit_probe(struct platform_device *pdev)
> + sizeof(struct acpi_table_nfit),
> sz - sizeof(struct acpi_table_nfit));
>
> - if (rc)
> + if (rc) {
> acpi_nfit_shutdown(acpi_desc);
> + return rc;
> + }
>
> - return rc;
> + /*
> + * Let notify handlers operate (the actual value of the ACPI companion's
> + * driver data pointer does not matter here so long as it is not NULL).
> + */
> + dev_set_drvdata(&adev->dev, dev);
> + return 0;
> }
>
> static void acpi_nfit_remove(struct platform_device *pdev)
> {
> + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
> +
> + guard(mutex)(&acpi_notify_lock);
> +
> + /* Make notify handlers bail out early going forward. */
> + dev_set_drvdata(&adev->dev, NULL);
> acpi_nfit_shutdown(platform_get_drvdata(pdev));
> }
>
^ permalink raw reply
* Re: [PATCH v2 3/4] ACPI: NFIT: core: Eliminate redundant local variable
From: Dave Jiang @ 2026-06-03 23:07 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, nvdimm, Alison Schofield,
Xiang Chen
In-Reply-To: <14028918.uLZWGnKmhe@rafael.j.wysocki>
On 6/3/26 10:57 AM, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Eliminate local variable acpi_desc from __acpi_nvdimm_notify() because it
> is redundant (its value is only checked against NULL once and the value
> assigned to it may be checked directly instead) and update the subsequent
> comment to reflect the code change.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/acpi/nfit/core.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 01c73be0bd00..aaa84ae7a20e 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1680,7 +1680,6 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc,
> void __acpi_nvdimm_notify(struct device *dev, u32 event)
> {
> struct nfit_mem *nfit_mem;
> - struct acpi_nfit_desc *acpi_desc;
>
> dev_dbg(dev->parent, "%s: event: %d\n", dev_name(dev),
> event);
> @@ -1691,12 +1690,11 @@ void __acpi_nvdimm_notify(struct device *dev, u32 event)
> return;
> }
>
> - acpi_desc = dev_get_drvdata(dev->parent);
> - if (!acpi_desc)
> + if (!dev_get_drvdata(dev->parent))
> return;
>
> /*
> - * If we successfully retrieved acpi_desc, then we know nfit_mem data
> + * If the parent's driver data pointer is not NULL, then nfit_mem data
> * is still valid.
> */
> nfit_mem = dev_get_drvdata(dev);
^ permalink raw reply
* Re: [PATCH v2 2/4] ACPI: NFIT: core: Fix acpi_nfit_init() error cleanup
From: Dave Jiang @ 2026-06-03 23:06 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, nvdimm, Alison Schofield,
Xiang Chen
In-Reply-To: <1963615.tdWV9SEqCh@rafael.j.wysocki>
On 6/3/26 10:57 AM, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> If acpi_nfit_init() fails after adding the acpi_desc object to the
> acpi_descs list, that object is never removed from that list because
> the acpi_nfit_shutdown() devm action is not added for the NFIT device
> in that case. Next, the acpi_nfit_init() failure causes
> acpi_nfit_probe() to fail, the acpi_desc object is freed, and a
> dangling pointer is left behind in the acpi_descs. Any subsequent
> ACPI Machine Check Exception will trigger nfit_handle_mce() which
> iterates over acpi_descs and so a use-after-free will occur.
>
> Moreover, if acpi_nfit_probe() returns 0 after installing a notify
> handler for the NFIT device and without allocating the acpi_desc
> object and setting the NFIT device's driver data pointer, the
> acpi_desc object will be allocated by acpi_nfit_update_notify()
> and acpi_nfit_init() will be called to initialize it. Regardless
> of whether or not acpi_nfit_init() fails in that case, the
> acpi_nfit_shutdown() devm action is not added for the NFIT device
> and acpi_desc is never removed from the acpi_descs list. If the
> acpi_desc object is freed subsequently on driver removal, any
> subsequent ACPI MCE will lead to a use-after-free like in the
> previous case.
>
> To address the first issue mentioned above, make acpi_nfit_probe()
> call acpi_nfit_shutdown() directly on acpi_nfit_init() failures and
> to address the other one, add a remove callback to the driver and
> make it call acpi_nfit_shutdown(). Also, since it is now possible to
> pass NULL to acpi_nfit_shutdown() or the acpi_desc object passed to it
> may not have been initialized, add checks against NULL for acpi_desc and
> its nvdimm_bus field to that function and make acpi_nfit_unregister()
> clear the latter after unregistering the NVDIMM bus.
>
> Fixes: a61fe6f7902e ("nfit, tools/testing/nvdimm: unify common init for acpi_nfit_desc")
> Fixes: fbabd829fe76 ("acpi, nfit: fix module unload vs workqueue shutdown race")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: All applicable <stable@vger.kernel.org>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/acpi/nfit/core.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 8024cd3cad14..01c73be0bd00 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -3069,6 +3069,8 @@ static void acpi_nfit_unregister(void *data)
> struct acpi_nfit_desc *acpi_desc = data;
>
> nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
> + /* The nvdimm_bus object may have been freed, so clear the pointer. */
> + acpi_desc->nvdimm_bus = NULL;
> }
>
> int acpi_nfit_init(struct acpi_nfit_desc *acpi_desc, void *data, acpi_size sz)
> @@ -3301,7 +3303,10 @@ static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
> void acpi_nfit_shutdown(void *data)
> {
> struct acpi_nfit_desc *acpi_desc = data;
> - struct device *bus_dev = to_nvdimm_bus_dev(acpi_desc->nvdimm_bus);
> + struct device *bus_dev;
> +
> + if (!acpi_desc || !acpi_desc->nvdimm_bus)
> + return;
>
> /*
> * Destruct under acpi_desc_lock so that nfit_handle_mce does not
> @@ -3316,6 +3321,7 @@ void acpi_nfit_shutdown(void *data)
> mutex_unlock(&acpi_desc->init_mutex);
> cancel_delayed_work_sync(&acpi_desc->dwork);
>
> + bus_dev = to_nvdimm_bus_dev(acpi_desc->nvdimm_bus);
> /*
> * Bounce the nvdimm bus lock to make sure any in-flight
> * acpi_nfit_ars_rescan() submissions have had a chance to
> @@ -3388,9 +3394,14 @@ static int acpi_nfit_probe(struct platform_device *pdev)
> sz - sizeof(struct acpi_table_nfit));
>
> if (rc)
> - return rc;
> + acpi_nfit_shutdown(acpi_desc);
>
> - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
> + return rc;
> +}
> +
> +static void acpi_nfit_remove(struct platform_device *pdev)
> +{
> + acpi_nfit_shutdown(platform_get_drvdata(pdev));
> }
>
> static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
> @@ -3474,6 +3485,7 @@ MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
>
> static struct platform_driver acpi_nfit_driver = {
> .probe = acpi_nfit_probe,
> + .remove = acpi_nfit_remove,
> .driver = {
> .name = "acpi-nfit",
> .acpi_match_table = acpi_nfit_ids,
^ permalink raw reply
* Re: [PATCH v2 1/4] ACPI: NFIT: core: Fix possible NULL pointer dereference
From: Dave Jiang @ 2026-06-03 22:33 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, nvdimm, Alison Schofield,
Xiang Chen
In-Reply-To: <2418508.ElGaqSPkdT@rafael.j.wysocki>
On 6/3/26 10:56 AM, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> After commit 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before
> getting NFIT table"), acpi_nfit_probe() installs an ACPI notify handler
> for the NFIT device before checking the presence of the NFIT table. If
> that table is not there, 0 is returned without allocating the acpi_desc
> object and setting the driver data pointer of the NFIT device. If the
> platform firmware triggers an NFIT_NOTIFY_UC_MEMORY_ERROR notification
> on the NFIT device at that point, acpi_nfit_uc_error_notify() will
> dereference a NULL pointer.
>
> Prevent that from occurring by adding an acpi_desc check against NULL
> to acpi_nfit_uc_error_notify().
>
> Fixes: 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before getting NFIT table")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: All applicable <stable@vger.kernel.org>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/acpi/nfit/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 5cab62f618c8..8024cd3cad14 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -3442,6 +3442,9 @@ static void acpi_nfit_uc_error_notify(struct device *dev, acpi_handle handle)
> {
> struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(dev);
>
> + if (!acpi_desc)
> + return;
> +
> if (acpi_desc->scrub_mode == HW_ERROR_SCRUB_ON)
> acpi_nfit_ars_rescan(acpi_desc, ARS_REQ_LONG);
> else
^ permalink raw reply
* [PATCH v1] ACPI: bus: Clean up devm_acpi_install_notify_handler()
From: Rafael J. Wysocki @ 2026-06-03 18:26 UTC (permalink / raw)
To: Linux ACPI; +Cc: LKML, Andy Shevchenko
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Add a pointer to the struct acpi_device used for installing the ACPI
notify handler to struct acpi_notify_handler_devres so it need not
be retrieved from the owner device via ACPI_COMPANION() in
devm_acpi_notify_handler_release().
While at it, drop the function name from one of the messages printed
by devm_acpi_install_notify_handler() for consistency and fix up white
space in its kerneldoc comment.
No intentional functional impact.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
Applies on top of linux-next.
---
drivers/acpi/bus.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -680,6 +680,7 @@ void acpi_dev_remove_notify_handler(stru
EXPORT_SYMBOL_GPL(acpi_dev_remove_notify_handler);
struct acpi_notify_handler_devres {
+ struct acpi_device *adev;
acpi_notify_handler handler;
u32 handler_type;
};
@@ -688,13 +689,12 @@ static void devm_acpi_notify_handler_rel
{
struct acpi_notify_handler_devres *dr = res;
- acpi_dev_remove_notify_handler(ACPI_COMPANION(dev), dr->handler_type,
- dr->handler);
+ acpi_dev_remove_notify_handler(dr->adev, dr->handler_type, dr->handler);
}
/**
* devm_acpi_install_notify_handler - Install an ACPI notify handler for a
- * managed device
+ * managed device
* @dev: Device to install a notify handler for
* @handler_type: Type of the notify handler
* @handler: Handler function to install
@@ -725,7 +725,7 @@ int devm_acpi_install_notify_handler(str
adev = ACPI_COMPANION(dev);
if (!adev)
- return dev_err_probe(dev, -ENODEV, "No ACPI companion in %s()\n", __func__);
+ return dev_err_probe(dev, -ENODEV, "No ACPI companion\n");
dr = devres_alloc(devm_acpi_notify_handler_release, sizeof(*dr), GFP_KERNEL);
if (!dr)
@@ -737,6 +737,7 @@ int devm_acpi_install_notify_handler(str
return dev_err_probe(dev, ret, "Failed to install an ACPI notify handler\n");
}
+ dr->adev = adev;
dr->handler = handler;
dr->handler_type = handler_type;
devres_add(dev, dr);
^ permalink raw reply
* [PATCH v2 0/4] ACPI: NFIT: core: Fix multiple issues related to concurrency and cleanup
From: Rafael J. Wysocki @ 2026-06-03 17:55 UTC (permalink / raw)
To: Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, Dave Jiang, nvdimm,
Alison Schofield, Xiang Chen
Hi All,
This series is a replacement for
https://lore.kernel.org/linux-acpi/6000262.DvuYhMxLoT@rafael.j.wysocki/
and it addresses some additional issues discovered during the review
of the patch above. Some of it is based on the sashiko.dev feedback:
https://sashiko.dev/#/patchset/6000262.DvuYhMxLoT%40rafael.j.wysocki
Patch [1/4] adds a NULL pointer check to prevent a possible NULL pointer
dereference from occurring.
Patch [2/4] fixes issues related to acpi_nfit_init() failures.
Patch [3/4] is a preparatory cleanup before the next patch.
Patch [4/4] addresses a possible deadlock during driver removal or
failing probe and missing NVDIMM device notifications from platform
firmware.
Thanks!
^ permalink raw reply
* [PATCH v2 1/4] ACPI: NFIT: core: Fix possible NULL pointer dereference
From: Rafael J. Wysocki @ 2026-06-03 17:56 UTC (permalink / raw)
To: Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, Dave Jiang, nvdimm,
Alison Schofield, Xiang Chen
In-Reply-To: <5110904.31r3eYUQgx@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
After commit 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before
getting NFIT table"), acpi_nfit_probe() installs an ACPI notify handler
for the NFIT device before checking the presence of the NFIT table. If
that table is not there, 0 is returned without allocating the acpi_desc
object and setting the driver data pointer of the NFIT device. If the
platform firmware triggers an NFIT_NOTIFY_UC_MEMORY_ERROR notification
on the NFIT device at that point, acpi_nfit_uc_error_notify() will
dereference a NULL pointer.
Prevent that from occurring by adding an acpi_desc check against NULL
to acpi_nfit_uc_error_notify().
Fixes: 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before getting NFIT table")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: All applicable <stable@vger.kernel.org>
---
drivers/acpi/nfit/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5cab62f618c8..8024cd3cad14 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3442,6 +3442,9 @@ static void acpi_nfit_uc_error_notify(struct device *dev, acpi_handle handle)
{
struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(dev);
+ if (!acpi_desc)
+ return;
+
if (acpi_desc->scrub_mode == HW_ERROR_SCRUB_ON)
acpi_nfit_ars_rescan(acpi_desc, ARS_REQ_LONG);
else
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/4] ACPI: NFIT: core: Fix acpi_nfit_init() error cleanup
From: Rafael J. Wysocki @ 2026-06-03 17:57 UTC (permalink / raw)
To: Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, Dave Jiang, nvdimm,
Alison Schofield, Xiang Chen
In-Reply-To: <5110904.31r3eYUQgx@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
If acpi_nfit_init() fails after adding the acpi_desc object to the
acpi_descs list, that object is never removed from that list because
the acpi_nfit_shutdown() devm action is not added for the NFIT device
in that case. Next, the acpi_nfit_init() failure causes
acpi_nfit_probe() to fail, the acpi_desc object is freed, and a
dangling pointer is left behind in the acpi_descs. Any subsequent
ACPI Machine Check Exception will trigger nfit_handle_mce() which
iterates over acpi_descs and so a use-after-free will occur.
Moreover, if acpi_nfit_probe() returns 0 after installing a notify
handler for the NFIT device and without allocating the acpi_desc
object and setting the NFIT device's driver data pointer, the
acpi_desc object will be allocated by acpi_nfit_update_notify()
and acpi_nfit_init() will be called to initialize it. Regardless
of whether or not acpi_nfit_init() fails in that case, the
acpi_nfit_shutdown() devm action is not added for the NFIT device
and acpi_desc is never removed from the acpi_descs list. If the
acpi_desc object is freed subsequently on driver removal, any
subsequent ACPI MCE will lead to a use-after-free like in the
previous case.
To address the first issue mentioned above, make acpi_nfit_probe()
call acpi_nfit_shutdown() directly on acpi_nfit_init() failures and
to address the other one, add a remove callback to the driver and
make it call acpi_nfit_shutdown(). Also, since it is now possible to
pass NULL to acpi_nfit_shutdown() or the acpi_desc object passed to it
may not have been initialized, add checks against NULL for acpi_desc and
its nvdimm_bus field to that function and make acpi_nfit_unregister()
clear the latter after unregistering the NVDIMM bus.
Fixes: a61fe6f7902e ("nfit, tools/testing/nvdimm: unify common init for acpi_nfit_desc")
Fixes: fbabd829fe76 ("acpi, nfit: fix module unload vs workqueue shutdown race")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: All applicable <stable@vger.kernel.org>
---
drivers/acpi/nfit/core.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 8024cd3cad14..01c73be0bd00 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3069,6 +3069,8 @@ static void acpi_nfit_unregister(void *data)
struct acpi_nfit_desc *acpi_desc = data;
nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
+ /* The nvdimm_bus object may have been freed, so clear the pointer. */
+ acpi_desc->nvdimm_bus = NULL;
}
int acpi_nfit_init(struct acpi_nfit_desc *acpi_desc, void *data, acpi_size sz)
@@ -3301,7 +3303,10 @@ static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
void acpi_nfit_shutdown(void *data)
{
struct acpi_nfit_desc *acpi_desc = data;
- struct device *bus_dev = to_nvdimm_bus_dev(acpi_desc->nvdimm_bus);
+ struct device *bus_dev;
+
+ if (!acpi_desc || !acpi_desc->nvdimm_bus)
+ return;
/*
* Destruct under acpi_desc_lock so that nfit_handle_mce does not
@@ -3316,6 +3321,7 @@ void acpi_nfit_shutdown(void *data)
mutex_unlock(&acpi_desc->init_mutex);
cancel_delayed_work_sync(&acpi_desc->dwork);
+ bus_dev = to_nvdimm_bus_dev(acpi_desc->nvdimm_bus);
/*
* Bounce the nvdimm bus lock to make sure any in-flight
* acpi_nfit_ars_rescan() submissions have had a chance to
@@ -3388,9 +3394,14 @@ static int acpi_nfit_probe(struct platform_device *pdev)
sz - sizeof(struct acpi_table_nfit));
if (rc)
- return rc;
+ acpi_nfit_shutdown(acpi_desc);
- return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+ return rc;
+}
+
+static void acpi_nfit_remove(struct platform_device *pdev)
+{
+ acpi_nfit_shutdown(platform_get_drvdata(pdev));
}
static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
@@ -3474,6 +3485,7 @@ MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
static struct platform_driver acpi_nfit_driver = {
.probe = acpi_nfit_probe,
+ .remove = acpi_nfit_remove,
.driver = {
.name = "acpi-nfit",
.acpi_match_table = acpi_nfit_ids,
--
2.51.0
^ permalink raw reply related
* [PATCH v2 3/4] ACPI: NFIT: core: Eliminate redundant local variable
From: Rafael J. Wysocki @ 2026-06-03 17:57 UTC (permalink / raw)
To: Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, Dave Jiang, nvdimm,
Alison Schofield, Xiang Chen
In-Reply-To: <5110904.31r3eYUQgx@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Eliminate local variable acpi_desc from __acpi_nvdimm_notify() because it
is redundant (its value is only checked against NULL once and the value
assigned to it may be checked directly instead) and update the subsequent
comment to reflect the code change.
No functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/nfit/core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 01c73be0bd00..aaa84ae7a20e 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1680,7 +1680,6 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc,
void __acpi_nvdimm_notify(struct device *dev, u32 event)
{
struct nfit_mem *nfit_mem;
- struct acpi_nfit_desc *acpi_desc;
dev_dbg(dev->parent, "%s: event: %d\n", dev_name(dev),
event);
@@ -1691,12 +1690,11 @@ void __acpi_nvdimm_notify(struct device *dev, u32 event)
return;
}
- acpi_desc = dev_get_drvdata(dev->parent);
- if (!acpi_desc)
+ if (!dev_get_drvdata(dev->parent))
return;
/*
- * If we successfully retrieved acpi_desc, then we know nfit_mem data
+ * If the parent's driver data pointer is not NULL, then nfit_mem data
* is still valid.
*/
nfit_mem = dev_get_drvdata(dev);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 4/4] ACPI: NFIT: core: Fix possible deadlock and missing notifications
From: Rafael J. Wysocki @ 2026-06-03 17:58 UTC (permalink / raw)
To: Linux ACPI
Cc: Dan Williams, LKML, Vishal Verma, Dave Jiang, nvdimm,
Alison Schofield, Xiang Chen
In-Reply-To: <5110904.31r3eYUQgx@rafael.j.wysocki>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
After commit 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before
getting NFIT table"), ACPI NFIT driver removal may deadlock if an ACPI
notify on the NFIT device is triggered concurrently. A similar deadlock
may occur if an ACPI notify on the NFIT device is triggered during a
failing driver probe.
The deadlock is possible because acpi_dev_remove_notify_handler() calls
acpi_os_wait_events_complete() after removing the notify handler and the
driver core invokes it under the NFIT platform device lock which is also
acquired by acpi_nfit_notify(). Thus acpi_os_wait_events_complete() may
be waiting for acpi_nfit_notify() to complete, but the latter may not be
able to acquire the device lock which is being held by the driver core
while the former is being executed.
Moreover, after commit 03667e146f81 ("ACPI: NFIT: core: Convert the
driver to a platform one"), there are no sysfs notifications regarding
NVDIMM devices because __acpi_nvdimm_notify() always bails out after
checking the driver data pointer of the device's parent. That parent
is the ACPI companion of the platform device used for driver binding,
so its driver data pointer is always NULL after the commit in question
which was overlooked by it.
A remedy for the deadlock is to use a special separate lock for ACPI
notify synchronization with driver probe and removal instead of the
device lock of the NFIT device, while a remedy for the second issue
is to populate the driver data pointer of the NFIT device's ACPI
companion when the driver is ready to operate, so do both these things.
However, since the new lock is not held across the entire teardown and
acpi_nfit_notify() should do nothing when teardown is in progress, make
it check the driver data pointer of the NFIT device's ACPI companion, in
analogy with the existing check in __acpi_nvdimm_notify(), and bail out
if that pointer is NULL.
Fixes: 9b311b7313d6 ("ACPI: NFIT: Install Notify() handler before getting NFIT table")
Fixes: 03667e146f81 ("ACPI: NFIT: core: Convert the driver to a platform one")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: All applicable <stable@vger.kernel.org>
---
drivers/acpi/nfit/core.c | 63 ++++++++++++++++++++++++++++++++--------
1 file changed, 51 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index aaa84ae7a20e..cb771d9cadb2 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -56,6 +56,8 @@ MODULE_PARM_DESC(force_labels, "Opt-in to labels despite missing methods");
LIST_HEAD(acpi_descs);
DEFINE_MUTEX(acpi_desc_lock);
+DEFINE_MUTEX(acpi_notify_lock);
+
static struct workqueue_struct *nfit_wq;
struct nfit_table_prev {
@@ -1708,9 +1710,15 @@ static void acpi_nvdimm_notify(acpi_handle handle, u32 event, void *data)
struct acpi_device *adev = data;
struct device *dev = &adev->dev;
- device_lock(dev->parent);
+ /*
+ * Locking is needed here for synchronization with driver probe and
+ * removal and the parent's driver data pointer is NULL when teardown
+ * is in progress (while the parent here is expected to be the ACPI
+ * companion of the platform device used for driver binding).
+ */
+ guard(mutex)(&acpi_notify_lock);
+
__acpi_nvdimm_notify(dev, event);
- device_unlock(dev->parent);
}
static bool acpi_nvdimm_has_method(struct acpi_device *adev, char *method)
@@ -3156,11 +3164,10 @@ EXPORT_SYMBOL_GPL(acpi_nfit_init);
static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
- struct device *dev = acpi_desc->dev;
- /* Bounce the device lock to flush acpi_nfit_add / acpi_nfit_notify */
- device_lock(dev);
- device_unlock(dev);
+ /* Bounce the notify lock to flush acpi_nfit_probe / acpi_nfit_notify */
+ mutex_lock(&acpi_notify_lock);
+ mutex_unlock(&acpi_notify_lock);
/* Bounce the init_mutex to complete initial registration */
mutex_lock(&acpi_desc->init_mutex);
@@ -3292,10 +3299,17 @@ static void acpi_nfit_put_table(void *table)
static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
{
struct device *dev = data;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
- device_lock(dev);
- __acpi_nfit_notify(dev, handle, event);
- device_unlock(dev);
+ /*
+ * Locking is needed here for synchronization with driver probe and
+ * removal and the ACPI companion's driver data pointer is NULL when
+ * teardown is in progress.
+ */
+ guard(mutex)(&acpi_notify_lock);
+
+ if (dev_get_drvdata(&adev->dev))
+ __acpi_nfit_notify(dev, handle, event);
}
void acpi_nfit_shutdown(void *data)
@@ -3337,11 +3351,18 @@ static int acpi_nfit_probe(struct platform_device *pdev)
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_nfit_desc *acpi_desc;
struct device *dev = &pdev->dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct acpi_table_header *tbl;
acpi_status status = AE_OK;
acpi_size sz;
int rc = 0;
+ /*
+ * Prevent acpi_nfit_notify() from progressing until the probe is
+ * complete in case there is a concurrent event to process.
+ */
+ guard(mutex)(&acpi_notify_lock);
+
rc = devm_acpi_install_notify_handler(dev, ACPI_DEVICE_NOTIFY,
acpi_nfit_notify, dev);
if (rc)
@@ -3357,6 +3378,11 @@ static int acpi_nfit_probe(struct platform_device *pdev)
* data in the format of a series of NFIT Structures.
*/
dev_dbg(dev, "failed to find NFIT at startup\n");
+ /*
+ * Let acpi_nfit_update_notify() run in case it will need to
+ * allocate the acpi_desc object.
+ */
+ dev_set_drvdata(&adev->dev, dev);
return 0;
}
@@ -3374,7 +3400,7 @@ static int acpi_nfit_probe(struct platform_device *pdev)
acpi_desc->acpi_header = *tbl;
/* Evaluate _FIT and override with that if present */
- status = acpi_evaluate_object(ACPI_HANDLE(dev), "_FIT", NULL, &buf);
+ status = acpi_evaluate_object(adev->handle, "_FIT", NULL, &buf);
if (ACPI_SUCCESS(status) && buf.length > 0) {
union acpi_object *obj = buf.pointer;
@@ -3391,14 +3417,27 @@ static int acpi_nfit_probe(struct platform_device *pdev)
+ sizeof(struct acpi_table_nfit),
sz - sizeof(struct acpi_table_nfit));
- if (rc)
+ if (rc) {
acpi_nfit_shutdown(acpi_desc);
+ return rc;
+ }
- return rc;
+ /*
+ * Let notify handlers operate (the actual value of the ACPI companion's
+ * driver data pointer does not matter here so long as it is not NULL).
+ */
+ dev_set_drvdata(&adev->dev, dev);
+ return 0;
}
static void acpi_nfit_remove(struct platform_device *pdev)
{
+ struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+
+ guard(mutex)(&acpi_notify_lock);
+
+ /* Make notify handlers bail out early going forward. */
+ dev_set_drvdata(&adev->dev, NULL);
acpi_nfit_shutdown(platform_get_drvdata(pdev));
}
--
2.51.0
^ permalink raw reply related
* [PATCH v1] ACPI: IPMI: Fix message kref handling on dead device
From: Yuho Choi @ 2026-06-03 16:31 UTC (permalink / raw)
To: Rafael J . Wysocki; +Cc: Len Brown, linux-acpi, linux-kernel, Yuho Choi
acpi_ipmi_space_handler() takes an extra reference on tx_msg before
checking whether the selected IPMI device is dead. The reference
belongs to the tx_msg_list entry and is normally dropped by
ipmi_cancel_tx_msg() or ipmi_flush_tx_msg() after the message is removed
from the list.
On the dead-device path, the message has not been queued yet, but the
error path still calls ipmi_msg_release() directly. That bypasses
kref_put() and frees tx_msg while the queued-message reference is still
recorded in the kref count.
Take the queued-message reference only after the dead-device check
succeeds, immediately before adding tx_msg to the list.
Fixes: 7b9844772237 ("ACPI / IPMI: Add reference counting for ACPI IPMI transfers")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
drivers/acpi/acpi_ipmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index 8f1aeae8b72e..79ce6e72bf29 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -550,7 +550,6 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address,
return AE_TYPE;
}
- acpi_ipmi_msg_get(tx_msg);
mutex_lock(&driver_data.ipmi_lock);
/* Do not add a tx_msg that can not be flushed. */
if (ipmi_device->dead) {
@@ -558,6 +557,7 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address,
ipmi_msg_release(tx_msg);
return AE_NOT_EXIST;
}
+ acpi_ipmi_msg_get(tx_msg);
spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags);
list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list);
spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v02] mailbox: pcc: report errors for PCC clients
From: Adam Young @ 2026-06-03 15:15 UTC (permalink / raw)
To: Sudeep Holla, Adam Young
Cc: Jassi Brar, linux-kernel, linux-hwmon, Rafael J . Wysocki,
Len Brown, linux-acpi, Andi Shyti, Guenter Roeck, Huisong Li,
MyungJoo Ham, Kyungmin Park, Chanwoo Choi, linux-arm-kernel
In-Reply-To: <20260519-inquisitive-teal-yak-56abd1@sudeepholla>
On 5/19/26 09:23, Sudeep Holla wrote:
> On Mon, May 18, 2026 at 03:30:06PM -0400, Adam Young wrote:
>> The tx_done callback function has a return code (rc) parameter
>> that the tx_done callback can use to determine how to handle an error.
>> However the IRQ handler was not setting that value if there is an error.
>>
>> The following clients are affected:
>>
>> drivers/acpi/cppc_acpi.c
>> drivers/i2c/busses/i2c-xgene-slimpro.c
>> drivers/hwmon/xgene-hwmon.c
>> drivers/soc/hisilicon/kunpeng_hccs.c
>> drivers/devfreq/hisi_uncore_freq.c
>>
>> All of these only use the error code to report, so they
>> are expecting an error code to come thorugh, but they
>> do not modify behavior based on this code.
>>
>> In the case of an error code in the IRQ, the handler was returning
>> IRQ_NONE which is not correct: the IRQ handler was matched
>> to the IRQ. This mean that multiple error codes returned from
>> a PCC triggered interrupt would end up disabling the device.
>>
>> In addition, if the error code IRQ was coming from a Type4 Device that was
>> expecting an IRQ response, that device would then be hung.
>>
>> Fixes: c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC subspaces(type 3/4)")
>> Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
>>
>> ---
>> ---
>> drivers/mailbox/pcc.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
>> index 636879ae1db7..16b9ce087b9e 100644
>> --- a/drivers/mailbox/pcc.c
>> +++ b/drivers/mailbox/pcc.c
>> @@ -314,6 +314,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>> {
>> struct pcc_chan_info *pchan;
>> struct mbox_chan *chan = p;
>> + int rc;
>>
>> pchan = chan->con_priv;
>>
>> @@ -327,8 +328,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>> if (!pcc_mbox_cmd_complete_check(pchan))
>> return IRQ_NONE;
>>
>> - if (pcc_mbox_error_check_and_clear(pchan))
>> - return IRQ_NONE;
>> + rc = pcc_mbox_error_check_and_clear(pchan);
> I think we may have to skip the check inside pcc_mbox_error_check_and_clear()
> for Type 4 channel as the spec expects OSPM to ignore it. It is a separate
> fix, just noting that here.
I think that should be in this patch, for correctness. It is a small
enough change. I'll update.
>
>>
>> /*
>> * Clear this flag after updating interrupt ack register and just
>> @@ -337,8 +337,9 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
>> * required to avoid any possible race in updatation of this flag.
>> */
>> pchan->chan_in_use = false;
>> - mbox_chan_received_data(chan, NULL);
>> - mbox_chan_txdone(chan, 0);
>> + if (!rc)
>> + mbox_chan_received_data(chan, NULL);
> Not sure if making this conditional is good as some platforms may expect
> it to move the state machine, I am not sure 100% just thinking aloud here.
They don't do that now. If the RC is non-zero there is no call to
mbox_chan_received_data, it just short circuits above, so I do not see
how we could break something that is currently working by bypassing the
mbox_chan_received_data, whereas by not-skipping it, we will trigger the
call, and that will likely not handle the error. I think we need to skip it.
^ permalink raw reply
* Re: [RFC PATCH 0/3] ACPI, x86/cpu/amd: Parse S5_REST_STATUS from ACPI PHAT on supported AMD platforms
From: Rong Zhang @ 2026-06-03 14:37 UTC (permalink / raw)
To: K Prateek Nayak, Rafael J. Wysocki, Len Brown, Borislav Petkov,
Mario Limonciello, Yazen Ghannam, linux-kernel, linux-acpi,
acpica-devel, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86
Cc: Vilas Sridharan, H. Peter Anvin, Andrew Cooper, Mikhail Paulyshka,
Kai Huang
In-Reply-To: <20260603063317.13665-1-kprateek.nayak@amd.com>
Hi K Prateek,
On Wed, 2026-06-03 at 06:33 +0000, K Prateek Nayak wrote:
> x86/cpu/amd bits uses the FCH register to read the S5_RESET_STATUS which
> is known to be inaccurate on some configurations where the firmware can
> consume the status before the OS boots.
>
> Vilas and Yazen noted that the ACPI PHAT table has a read-only shadow
> copy of this state and is the preferred way for OS to consume the S5
> Reset Status.
>
> Introduce generic helpers to parse ACPI PHAT records and use the
> platform specific helpers to interpret the vendor specific reset reason
> records.
>
> Since these records are read-only, the status cannot be cleared once
> consumed and the last reset reason will persist across kxec until next
> reset which reintroduces the case prevented by commit e6416c2dfe23c
> ("x86/CPU/AMD: Prevent reset reasons from being retained across reboot")
> by clearing the FCH register once consumed.
It's not only about kexec.
The last reset reason register could remain unchanged when the reset is
caused by a voltage drop on a power rail, probably due to a broken power
supply or VRM. This wasted me a lot of frustrating time debugging
irrelevant reset reasons.
Eventually, I RMA'd the broken device and submitted the patch to save
other's time.
Also, some bits are not cleared by hardware as per [1]. If the firmware
doesn't clear them either they will persist anyway.
So I'd prefer clearing the register in any cases.
[1]: https://bugzilla.kernel.org/show_bug.cgi?id=206537#attach_303991
Thanks,
Rong
>
> This can be avoided with a boolean flags passed over as setup data
> during kexec but this scheme adds unnecessary complexity in
> implementation that has been left out for now.
>
> Patches are based on:
>
> git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi
>
> at commit 3109f9f38800 ("ACPI: button: Add missing device class clearing
> on probe failures") (tag: 'acpi-7.1-rc6')
>
> Series has been tested on both, a Zen4 platform that does not contain
> the PHAT record, and a Zen5 platform that contains the PHAT record.
> The Zen5 platform was also tested with CONFIG_ACPI_PHAT disabled to
> confirm that logic correctly falls back to reading the FCH register.
>
> x86 folks have been Cc'd only on the Cover letter and the x86 bits. ACPI
> folks, my fellow AMD colleagues, and the lists should have received the
> entire series. If you would like to be Cc'd on the full series for
> future posting, please let me know. Sorry in advance for any oversight!
> ---
> K Prateek Nayak (3):
> ACPICA: actbl2.h: ACPI 6.5: PHAT: Add more struct definitions
> ACPI: PHAT: Add generic helper to parse PHAT records
> x86/cpu/amd: Fetch S5_RESET_STATUS from PHAT when supported
>
> arch/x86/kernel/cpu/amd.c | 34 ++++++++++
> drivers/acpi/Kconfig | 11 ++++
> drivers/acpi/Makefile | 1 +
> drivers/acpi/acpi_phat.c | 133 ++++++++++++++++++++++++++++++++++++++
> include/acpi/actbl2.h | 14 ++++
> include/linux/acpi.h | 14 ++++
> 6 files changed, 207 insertions(+)
> create mode 100644 drivers/acpi/acpi_phat.c
>
>
> base-commit: 3109f9f38800841e46769e95e1ba11f1f8c7b230
^ permalink raw reply
* Re: [RFC PATCH 3/3] x86/cpu/amd: Fetch S5_RESET_STATUS from PHAT when supported
From: Mario Limonciello @ 2026-06-03 14:26 UTC (permalink / raw)
To: K Prateek Nayak, Rafael J. Wysocki, Len Brown, Borislav Petkov,
Yazen Ghannam, linux-kernel, linux-acpi, acpica-devel,
Thomas Gleixner, Ingo Molnar, Dave Hansen, x86
Cc: Vilas Sridharan, H. Peter Anvin, Andrew Cooper, Mikhail Paulyshka,
Kai Huang, Rong Zhang
In-Reply-To: <20260603063317.13665-4-kprateek.nayak@amd.com>
On 6/3/26 01:33, K Prateek Nayak wrote:
> Firmware can consume the S5_RESET_STATUS from the FCH register and the
> OS may see an incorrect value at the time of boot on certain platforms.
>
> Vilas, Yazen noted that S5_RESET_STATUS is also populated in ACPI PHAT
> on newer AMD platforms which is more reliable than the status from the
> FCH register.
>
> Use the S5_RESET_STATUS from the ACPI PHAT which is described in the
> "Platform Health Assessment Table (PHAT)" section of "AMD Family 1Ah
> Models 00h–0Fh and Models 10h–1Fh ACPI v6.5 Porting Guide" [1] on
> supported platform and fallback to the legacy FCH path if the PHAT
> record is not found.
>
> Unlike the FCH register, PHAT only provides a read-only value which
> cannot be cleared once consumed and will persist across kexec boots.
>
> Link: https://docs.amd.com/v/u/en-US/58088_0.90_PUB [1]
> Suggested-by: Vilas Sridharan <Vilas.Sridharan@amd.com>
> Suggested-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
> arch/x86/kernel/cpu/amd.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 2f8e8ff2d000..d3dee7fd4c51 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> #include <linux/export.h>
> +#include <linux/acpi.h>
> #include <linux/bitops.h>
> #include <linux/dmi.h>
> #include <linux/elf.h>
> @@ -11,6 +12,8 @@
> #include <linux/random.h>
> #include <linux/topology.h>
> #include <linux/platform_data/x86/amd-fch.h>
> +
> +#include <acpi/actbl2.h>
> #include <asm/processor.h>
> #include <asm/apic.h>
> #include <asm/cacheinfo.h>
> @@ -1342,8 +1345,21 @@ static const char * const s5_reset_reason_txt[] = {
> [31] = "a software sync flood event occurred",
> };
>
> +/*
> + * "AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh ACPI v6.5 Porting Guide"
> + * specifies the vendor-specific GUID of FCH::PM::S5_RESET_STATUS record as
> + * "1f425831-da46-4f65-9296-3c4d44c387ab" alongside the format of reset reason.
> + */
> +#define S5_RESET_STATUS_GUID GUID_INIT(0x1f425831, 0xda46, 0x4f65, 0x92, 0x96,\
> + 0x3c, 0x4d, 0x44, 0xc3, 0x87, 0xab)
> +
> static __init int print_s5_reset_status_mmio(void)
> {
> + struct reset_status_record {
> + struct acpi_phat_vendor_element header;
> + u32 s5_reset_status;
> + } __packed *record;
> + guid_t guid = S5_RESET_STATUS_GUID;
> void __iomem *addr;
> u32 value;
> int i;
> @@ -1351,6 +1367,23 @@ static __init int print_s5_reset_status_mmio(void)
> if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> return 0;
>
> + record = (void *)acpi_phat_get_vendor_reset_reason(&guid);
> + if (!IS_ERR(record)) {
> + bool record_valid = false;
> +
> + /* Sanity check before using the parsed record. */
> + if (record->header.length == sizeof(*record)) {
Why not do this sanity check in acpi_phat_get_vendor_reset_reason()
directly? Then we can always assume if something is returned it's valid
and it's simpler here (and any other potential callers in future).
> + pr_debug("Using ACPI PHAT record for S5_RESET_STATUS\n");
> + value = record->s5_reset_status;
> + record_valid = true;
> + }
> +
> + acpi_phat_put_vendor_reset_reason((void *)record);
> +
> + if (record_valid)
> + goto parse_status;
> + }
> +
> addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
> if (!addr)
> return 0;
> @@ -1374,6 +1407,7 @@ static __init int print_s5_reset_status_mmio(void)
> iowrite32(value, addr);
> iounmap(addr);
>
> +parse_status:
> for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
> if (!(value & BIT(i)))
> continue;
^ permalink raw reply
* Re: [RFC PATCH 2/3] ACPI: PHAT: Add generic helper to parse PHAT records
From: Mario Limonciello @ 2026-06-03 14:25 UTC (permalink / raw)
To: K Prateek Nayak, Rafael J. Wysocki, Len Brown, Borislav Petkov,
Yazen Ghannam, linux-kernel, linux-acpi, acpica-devel
Cc: Vilas Sridharan
In-Reply-To: <20260603063317.13665-3-kprateek.nayak@amd.com>
On 6/3/26 01:33, K Prateek Nayak wrote:
> The ACPI Specification v6.5 [1] under Sec. 5.2.31 "Platform Health
> Assessment Table (PHAT)" provides a well defined data format for reset
> reason records, including the ones that are vendor specific.
>
> Provide a generic helper to locate the vendor specific records and parse
> them into a buffer that can be consumed by the users.
>
> The first user for the interface is added in the subsequent commit with
> AMD processors using it to parse the s5_RESET_STATUS from PHAT records.
>
> Make the Kconfig depend on ACPI && CPU_SUP_AMD to prevent the need for
> unnecessarily compiling in ACPI PHAT support when not needed.
>
> Link: https://uefi.org/sites/default/files/resources/ACPI_Spec_6_5_Aug29.pdf [1]
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
> drivers/acpi/Kconfig | 11 ++++
> drivers/acpi/Makefile | 1 +
> drivers/acpi/acpi_phat.c | 133 +++++++++++++++++++++++++++++++++++++++
> include/linux/acpi.h | 14 +++++
> 4 files changed, 159 insertions(+)
> create mode 100644 drivers/acpi/acpi_phat.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index f165d14cf61a..494c757a62d7 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -607,6 +607,17 @@ config ACPI_PRMT
> substantially increase computational overhead related to the
> initialization of some server systems.
>
> +config ACPI_PHAT
> + bool "ACPI PHAT Record parsing support"
> + depends on X86 && ACPI && CPU_SUP_AMD
> + default y
> + help
> + The Platform Health Assessment Table (PHAT) allows platforms to
> + expose read-only data for diagnostics and health assessments.
> +
> + Enable this feature to access generic helpers for parsing PHAT
> + data records.
> +
> endif # ACPI
>
> config X86_PM_TIMER
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index d1b0affb844f..d6cf252ca4ea 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -108,6 +108,7 @@ obj-$(CONFIG_ACPI_SPCR_TABLE) += spcr.o
> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
> obj-$(CONFIG_ACPI_PPTT) += pptt.o
> obj-$(CONFIG_ACPI_PFRUT) += pfr_update.o pfr_telemetry.o
> +obj-$(CONFIG_ACPI_PHAT) += acpi_phat.o
>
> # processor has its own "processor." module_param namespace
> processor-y := processor_driver.o processor_thermal.o
> diff --git a/drivers/acpi/acpi_phat.c b/drivers/acpi/acpi_phat.c
> new file mode 100644
> index 000000000000..de612ae018e0
> --- /dev/null
> +++ b/drivers/acpi/acpi_phat.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Helpers for parsing Platform Health Assessment Table (PHAT) records.
> + */
> +#include <linux/kernel.h>
> +#include <linux/acpi.h>
> +
> +#include <acpi/actbl2.h>
> +
> +/*
> + * __phat_get_firmware_health_data() - Internal helper to locate the
> + * "Reset Reason Health Record" within the Platform Health Assessment
> + * Table (PHAT).
> + *
> + * @table: Pointer to the beginning of PHAT.
> + *
> + * Return: A pointer within the @table if "Reset Reason Health Record"
> + * is found; NULL otherwise.
> + */
> +static struct acpi_phat_health_data *
> +__phat_get_firmware_health_data(struct acpi_table_phat *table)
> +{
> + unsigned int length = table->header.length;
> + void *header = table;
> +
> + /* No records. */
> + if (length <= sizeof(struct acpi_table_header))
> + return NULL;
> +
> + /*
> + * Advance to the end of the table header
> + * where the first record starts.
> + */
> + header += sizeof(struct acpi_table_header);
> + length -= sizeof(struct acpi_table_header);
> +
> + /*
> + * Search for PHAT firmware health data record header
> + * with type == ACPI_PHAT_TYPE_FW_HEALTH_DATA.
> + */
> + while (length) {
> + struct acpi_phat_health_data *data = header;
> +
> + if (data->header.type == ACPI_PHAT_TYPE_FW_HEALTH_DATA)
> + return header;
> +
> + /* Move to the next header */
> + header += data->header.length;
> + length -= data->header.length;
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + * acpi_phat_get_vendor_reset_reason - Find a "Vendor Specific Reset Reason
> + * Entry" with the matching @guid from the "Firmware Health Data Record". If
> + * successfully located, the function will allocate an object of the size
> + * "acpi_phat_vendor_element.length" and return a pointer populated with the
> + * content of the record.
> + *
> + * @guid: The "Vendor Data ID" of the reset reason record.
> + *
> + * Return: A valid pointer to an allocated "acpi_phat_vendor_element" populated
> + * with the data from the record with matching @guid; an ERR_PTR() otherwise if
> + * no matching records were found, or if the element could not be allocated.
> + * If a valid pointer was returned, the user must call
> + * acpi_phat_put_vendor_reset_reason() for the object once done to reclaim the
> + * allocated memory.
> + */
> +struct acpi_phat_vendor_element *acpi_phat_get_vendor_reset_reason(guid_t *guid)
> +{
> + struct acpi_table_header *phat_tbl __free(acpi_put_table) = NULL;
> + struct acpi_phat_health_data *fw_health_data;
> + struct acpi_phat_device_data *dev_data;
> + acpi_status status;
> + void *data;
> + int i;
> +
> + status = acpi_get_table(ACPI_SIG_PHAT, 0, &phat_tbl);
> + if (ACPI_FAILURE(status))
> + return ERR_PTR(-ENODEV);
For some further sanity checking, should you look at the PHAT revision
here matches 2 as well? Table 55 in the linked spec.
> +
> + fw_health_data =
> + __phat_get_firmware_health_data((struct acpi_table_phat *)phat_tbl);
> + if (!fw_health_data)
> + return ERR_PTR(-ENODEV);
> +
> + /* Check if Device-specific data record is present. */
> + if (!fw_health_data->device_specific_offset)
> + return ERR_PTR(-ENODEV);
> +
> + dev_data = (void *)fw_health_data + fw_health_data->device_specific_offset;
> + if (!dev_data->vendor_count)
> + return ERR_PTR(-ENODEV);
> +
> + /* Vendor data starts after Device-specific data */
> + data = (void *)dev_data + sizeof(*dev_data);
> +
> + for (i = 0; i <= dev_data->vendor_count; ++i) {
> + struct acpi_phat_vendor_element *vendor_data = data;
> + int length = vendor_data->length;
> +
> + /*
> + * Move to the next Vendor specific entry if
> + * the GUID of entry doesn't match.
> + */
> + if (!guid_equal(guid, (guid_t *)vendor_data->vendor_guid)) {
> + data += vendor_data->length;
> + continue;
> + }
> +
> + vendor_data = kmalloc(length, GFP_KERNEL);
> + if (!vendor_data)
> + return ERR_PTR(-ENOMEM);
> +
> + memcpy(vendor_data, data, length);
> + return vendor_data;
> + }
> +
> + return ERR_PTR(-ENODEV);
> +}
> +
> +/**
> + * acpi_phat_put_vendor_reset_reason - Reclaim the object allocated by
> + * acpi_phat_get_vendor_reset_reason().
> + *
> + * @reason: A valid pointer returned by acpi_phat_get_vendor_reset_reason()
> + */
> +void acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason)
> +{
> + kfree(reason);
> +}
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 67effb91fa98..daed1d66ca43 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1627,6 +1627,20 @@ extern int acpi_ffh_address_space_arch_handler(acpi_integer *value,
> static inline void acpi_init_ffh(void) { }
> #endif
>
> +#ifdef CONFIG_ACPI_PHAT
> +struct acpi_phat_vendor_element *acpi_phat_get_vendor_reset_reason(guid_t *guid);
> +void acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason);
> +#else
> +static inline struct acpi_phat_vendor_element *
> +acpi_phat_get_vendor_reset_reason(guid_t *guid)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> +static inline void
> +acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason) { }
> +#endif
> +
> #ifdef CONFIG_ACPI
> extern void acpi_device_notify(struct device *dev);
> extern void acpi_device_notify_remove(struct device *dev);
^ permalink raw reply
* Re: [PATCH v1 06/17] ACPI: HED: Switch over to devres-based resource management
From: Andy Shevchenko @ 2026-06-03 11:12 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux ACPI, LKML, Hans de Goede, Armin Wolf
In-Reply-To: <CAJZ5v0hX_UCzmEuGYT6mhTf-L=vzN7dgoG_9FZw-Ccr=_Fo+5w@mail.gmail.com>
On Wed, Jun 03, 2026 at 12:51:20PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 3, 2026 at 12:10 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 21, 2026 at 04:04:03PM +0200, Rafael J. Wysocki wrote:
...
> > > static void acpi_hed_remove(struct platform_device *pdev)
> > > {
> > > - struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> > > -
> > > hed_present = false;
> > > - acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > - acpi_hed_notify);
> > > }
> >
> > devm will be cleaned after this, right?
>
> Yes.
>
> > So, here is a window that if one tries
> > to unbind device from the driver in one thread and do the opposite in another
> > they will see the flag false while resources are still allocated for the old
> > one. It seems that hed_present should be also devm:ed?
>
> If this is the same device, both binding and unbinding (including
> devm) take place under its device lock which will serialize all of
> that.
>
> If they are different devices with different ACPI companions, there's
> no resource conflict. If they are both valid HEDs (which would be a
> stretch already) and they both trigger a notification exactly at the
> "wrong" time (more stretch), worst-case stuff will be called twice in
> a row via blocking_notifier_call_chain().
Thanks for elaborating. We are good then.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 06/17] ACPI: HED: Switch over to devres-based resource management
From: Rafael J. Wysocki @ 2026-06-03 10:51 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rafael J. Wysocki, Linux ACPI, LKML, Hans de Goede, Armin Wolf
In-Reply-To: <ah9UyXwsLAjIJEff@ashevche-desk.local>
On Wed, Jun 3, 2026 at 12:10 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, May 21, 2026 at 04:04:03PM +0200, Rafael J. Wysocki wrote:
>
> > Use the newly introduced devm_acpi_install_notify_handler() for
> > installing an ACPI notify handler and since that function checks the
> > ACPI companion of the owner device against NULL internally, remove the
> > the explicit ACPI companion check from acpi_hed_probe().
> >
> > No intentional functional impact.
>
> ...
>
> > static void acpi_hed_remove(struct platform_device *pdev)
> > {
> > - struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> > -
> > hed_present = false;
> > - acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > - acpi_hed_notify);
> > }
>
> devm will be cleaned after this, right?
Yes.
> So, here is a window that if one tries
> to unbind device from the driver in one thread and do the opposite in another
> they will see the flag false while resources are still allocated for the old
> one. It seems that hed_present should be also devm:ed?
If this is the same device, both binding and unbinding (including
devm) take place under its device lock which will serialize all of
that.
If they are different devices with different ACPI companions, there's
no resource conflict. If they are both valid HEDs (which would be a
stretch already) and they both trigger a notification exactly at the
"wrong" time (more stretch), worst-case stuff will be called twice in
a row via blocking_notifier_call_chain().
^ permalink raw reply
* Re: [PATCH 5/8] dt-bindings: arm: ras: Introduce bindings for ARM AEST
From: Umang Chheda @ 2026-06-03 10:27 UTC (permalink / raw)
To: Rob Herring
Cc: Ruidong Tian, Tony Luck, Borislav Petkov, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, catalin.marinas,
will, lpieralisi, rafael, mark.rutland, Sudeep Holla,
linux-arm-msm, linux-acpi, linux-arm-kernel, linux-edac,
linux-kernel, devicetree
In-Reply-To: <48e298d0-f4cb-4afb-8ba1-a6b39f3285cb@oss.qualcomm.com>
Hello Rob,
On 5/20/2026 11:43 PM, Umang Chheda wrote:
> Hello Rob,
>
> Thanks for helping reviewing the code!
>
> On 5/13/2026 11:28 PM, Rob Herring wrote:
>> On Tue, May 05, 2026 at 05:53:49PM +0530, Umang Chheda wrote:
>>> The Arm Error Source Table (AEST) specification describes how firmware
>>> exposes RAS error source topology to the operating system. On ACPI
>>> systems this information is provided via the AEST ACPI table.
>>>
>>> Introduce Device Tree bindings that provide an equivalent description
>>> of AEST error sources for DT-based platforms.
>>>
>>> Signed-off-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
>>> ---
>>> .../devicetree/bindings/arm/arm,aest.yaml | 406 +++++++++++++++++++++
>>> include/dt-bindings/arm/aest.h | 43 +++
>>> 2 files changed, 449 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/arm,aest.yaml b/Documentation/devicetree/bindings/arm/arm,aest.yaml
>>> new file mode 100644
>>> index 000000000000..7809a0d38270
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/arm,aest.yaml
>>> @@ -0,0 +1,406 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/arm/arm,aest.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Arm Error Source Table (AEST)
>>> +
>>> +maintainers:
>>> + - Umang Chheda <umang.chheda@oss.qualcomm.com>
>>> +
>>> +description:
>>> + The Arm Error Source Table (AEST) describes RAS error sources and their
>>> + register interfaces. Each error source exposes one or more error records
>>> + through either system registers or a memory-mapped register window, and
>>> + may signal errors via interrupts. The top-level node acts as a container
>>> + for one or more child nodes, each describing a single AEST error source.
>>> + Refer to the Arm AEST specification (DEN0085 / DDI 0587B) for details.
>>> + Flag bit constants for use in DT source files are defined in
>>> + <dt-bindings/arm/aest.h>.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: arm,aest
>>> +
>>> + "#address-cells":
>>> + const: 2
>>> +
>>> + "#size-cells":
>>> + const: 2
>>> +
>>> + ranges: true
>>> +
>>> +required:
>>> + - compatible
>>> +
>>> +additionalProperties: false
>>> +
>>> +patternProperties:
>>> + "^aest-[a-z0-9-]+(@[0-9a-f]+)?$":
>>> + type: object
>>> + description:
>>> + An AEST error source node describing one error source defined by
>>> + the Arm AEST specification.
>>> +
>>> + properties:
>>> + compatible:
>>> + description:
>>> + Identifies the type of AEST error source. Each value corresponds to
>>> + a distinct error source class defined by the Arm AEST specification.
>>> + arm,aest-proxy represents a proxy error source that forwards errors
>>> + from another error source.
>>> + enum:
>>> + - arm,aest-processor
>>> + - arm,aest-memory
>>> + - arm,aest-smmu
>>> + - arm,aest-gic
>>> + - arm,aest-pcie
>>> + - arm,aest-vendor
>>> + - arm,aest-proxy
>>
>> This is a fundamental difference how DT and ACPI get structured. ACPI
>> defines new table for some feature and puts everything in that table.
>> For DT, these all belong in the node for the corresponding h/w. For
>> example, if the GIC supports AEST, then that belongs in the GIC node.
>
> Thanks for the feedback. To clarify your suggestion — should the AEST
> RAS properties be added directly as properties of the hardware node
> (e.g. arm,ras-num-records inside the cpu@0 node itself), or as a child
> node under the hardware node (e.g. a ras-error-source {} child under cpu@0)?
Can you please help with this query ?
>
>
>>
>>> +
>>> + reg:
>>> + description:
>>> + Register ranges for the error source. Absence of reg implies
>>> + system-register access (interface type 0). A single range implies
>>> + memory-mapped access (interface type 1). Two ranges imply
>>> + single-record memory-mapped access (interface type 2).
>>> + minItems: 1
>>> + maxItems: 4
>>> +
>>> + reg-names:
>>> + description:
>>> + Names for the register ranges. The base error-record window is
>>> + unnamed (or first entry). Optional named ranges provide access to
>>> + the fault-injection, error-group, and interrupt-config register
>>> + windows defined by the AEST specification.
>>> + minItems: 1
>>> + maxItems: 4
>>> + items:
>>> + enum:
>>> + - fault-inject
>>> + - err-group
>>> + - irq-config
>>> +
>>> + interrupts:
>>> + description: Interrupts associated with the error source.
>>> + minItems: 1
>>> + maxItems: 2
>>> +
>>> + interrupt-names:
>>> + description: Names of the interrupts associated with the error source.
>>> + minItems: 1
>>> + maxItems: 2
>>> + items:
>>> + enum:
>>> + - fhi
>>> + - eri
>>> +
>>> + arm,fhi-flags:
>>> + description:
>>> + Bitmask of flags for the fault-handling interrupt (FHI), as defined
>>> + in the AEST node interrupt structure flags field. Constants are
>>> + defined in <dt-bindings/arm/aest.h> - AEST_IRQ_MODE_LEVEL (0),
>>> + AEST_IRQ_MODE_EDGE (1).
>>
>> DT already has a way to define interrupt flags. Why invent something
>> new?
>
> Ack, this flag is not needed for DT based systems, will remove this.
>
>>
>> Rob
>
> Thanks,
> Umang
>
>
Thanks,
Umang
^ permalink raw reply
* Re: [PATCH v2 2/2] device property: fix infinite loop in fwnode_for_each_child_node()
From: Andy Shevchenko @ 2026-06-03 9:51 UTC (permalink / raw)
To: Xu Yang, Bartosz Golaszewski
Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Mauro Carvalho Chehab,
Laurent Pinchart, linux-acpi, driver-core, linux-kernel, Xu Yang,
stable
In-Reply-To: <20260603-fixes_fwnode_iteration-v2-2-0ae381f8b7b9@nxp.com>
On Wed, Jun 03, 2026 at 04:44:32PM +0800, Xu Yang wrote:
> When iterate over children of a fwnode that has a secondary fwnode,
> fwnode_get_next_child_node() can enter an infinite loop if the secondary
> fwnode has more than one child.
>
> Parent Child
> (Primary fwnode) FWa: {FWa1, FWa2, FWa3}
> (Secondary fwnode) FWb: {FWb1, FWb2}
>
> In this case:
>
> ┌─> fwnode_get_next_child_node(FWa, FWa1)
> │ - fwnode_call_ptr_op(FWa, get_next_child_node, FWa1) returns FWa2
> │
> │ ...
> │
> │ fwnode_get_next_child_node(FWa, FWa3)
> │ - fwnode_call_ptr_op(FWa, get_next_child_node, FWa3) returns NULL
> │ - fwnode_call_ptr_op(FWb, get_next_child_node, FWa3) returns FWb1
> │
> │ fwnode_get_next_child_node(FWa, FWb1)
> │ - fwnode_call_ptr_op(FWa, get_next_child_node, FWb1) returns FWa1
> └────┘
>
> This cause fwnode_for_each_child_node() to loop indefinitely, reapeatedly
> output {FWa1, FWa2, FWa3, FWb1, FWa1, ...}.
>
> The root cause is that when the current child (FWb1) belongs to the
> secondary fwnode, calling get_next_child_node() on the parimary fwnode
> incorrectly returns the first child (FWa1) again instead of NULL.
>
> Fix this by dynamically checking the parent fwnode of the current child
> before calling get_next_child_node(). This approach follows the pattern
> established in commit b5b41ab6b0c1 ("device property: Check
> fwnode->secondary in fwnode_graph_get_next_endpoint()").
...
TBH, this code becomes twisted and complicated. Can we add some test cases to
show the problem? Also we need to add other possible combinations (somewhat
about ~5-6) of the different types of fwnode in a relationship.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 0/2] device property: fix child iteration issues with secondary fwnodes
From: Andy Shevchenko @ 2026-06-03 9:43 UTC (permalink / raw)
To: Xu Yang, Bartosz Golaszewski
Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Mauro Carvalho Chehab,
Laurent Pinchart, linux-acpi, driver-core, linux-kernel, Xu Yang,
stable
In-Reply-To: <20260603-fixes_fwnode_iteration-v2-0-0ae381f8b7b9@nxp.com>
On Wed, Jun 03, 2026 at 04:44:30PM +0800, Xu Yang wrote:
> This series fixes two issues in the fwnode child iteration logic when
> a secondary fwnode is present.
>
> The first patch addresses a refcount imbalance in
> software_node_get_next_child(). When a software node is used as a
> secondary fwnode, the iteration code may incorrectly decrement the
> refcount of child nodes that do not belong to the software node
> hierarchy. This results in refcount underflow and possible use-after-free.
>
> The second patch fixes an infinite loop in
> fwnode_for_each_child_node(), caused by improper handling of iteration
> state across primary and secondary fwnodes. When iterating over children
> from both primary and secondary fwnodes, the code may incorrectly
> resume iteration from the primary fwnode even when the current child
> belongs to the secondary, leading to repeated traversal and a loop.
>
> Both issues are triggered when mixing different fwnode types through the
> secondary mechanism, and stem from incorrect assumptions about ownership
> and traversal context of child nodes.
Please, Cc Bart who is heavily working on software nodes these days.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 1/2] software node: fix refcount leak in software_node_get_next_child()
From: Andy Shevchenko @ 2026-06-03 9:40 UTC (permalink / raw)
To: Xu Yang
Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Mauro Carvalho Chehab,
Laurent Pinchart, linux-acpi, driver-core, linux-kernel, Xu Yang,
stable
In-Reply-To: <20260603-fixes_fwnode_iteration-v2-1-0ae381f8b7b9@nxp.com>
On Wed, Jun 03, 2026 at 04:44:31PM +0800, Xu Yang wrote:
> When a swnode acts as a secondary fwnode and is participates in child
> iteration, a refcount leak occurs for the last child of the primary
> fwnode's children.
>
> Parent Child
> (Primary fwnode) FW: {FW1, FW2, FW3}
> (Secondary fwnode) SW: {}
>
> In this case, FW3's refcount is decremented twice during iteration:
>
> fwnode_get_next_child_node(FW, FW3)
> 1. fwnode_call_ptr_op(FW, get_next_child_node, FW3) returns NULL and
> decrements FW3's refcount
> 2. fwnode_call_ptr_op(SW, get_next_child_node, FW3) returns NULL and
> decrements FW3's refcount again
>
> The same double-decrement issue occurs when SW has children.
>
> The kernel dump as below:
>
> [ 25.435805] OF: ERROR: of_node_release() detected bad of_node_put() on /soc/usb@4c010010/usb@4c100000
> [ 25.445072] CPU: 0 UID: 0 PID: 617 Comm: sh Not tainted 7.1.0-rc4-next-20260522-00011-g7376b330abca #210 PREEMPT
> [ 25.445080] Hardware name: NXP i.MX95 19X19 board (DT)
> [ 25.445083] Call trace:
> [ 25.445086] show_stack+0x18/0x30 (C)
> [ 25.445101] dump_stack_lvl+0x60/0x80
> [ 25.445108] dump_stack+0x18/0x24
> [ 25.445113] of_node_release+0x158/0x194
> [ 25.445122] kobject_put+0xa0/0x120
> [ 25.445129] of_node_put+0x18/0x28
> [ 25.445134] of_fwnode_put+0x38/0x58
> [ 25.445141] software_node_get_next_child+0x54/0x15c
> [ 25.445150] fwnode_get_next_child_node+0x70/0x94
> [ 25.445156] fwnode_get_next_available_child_node+0x34/0x88
> [ 25.445162] device_links_driver_bound+0x2f4/0x334
> [ 25.445168] driver_bound+0x68/0xb0
> ...
> [ 25.445258] OF: ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'
>
> Fix this by ensuring software_node_get_next_child() does not decrement
> the child's refcount when:
> - The parent has no children, OR
> - The parent has children but the input child is not a swnode
>
> This prevents the refcount from being incorrectly decremented for
> fwnodes that don't belong to the software node hierarchy.
...
> struct swnode *p = to_swnode(fwnode);
> struct swnode *c = to_swnode(child);
>
> - if (!p || list_empty(&p->children) ||
> - (c && list_is_last(&c->entry, &p->children))) {
> - fwnode_handle_put(child);
Wouldn't be better to use swnode_get() / swnode_put() instead?
*Yes, we might need to add some NULL checks there.
> + if (!p || list_empty(&p->children))
> return NULL;
> - }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2 2/2] device property: fix infinite loop in fwnode_for_each_child_node()
From: Xu Yang @ 2026-06-03 8:44 UTC (permalink / raw)
To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Mauro Carvalho Chehab, Laurent Pinchart
Cc: linux-acpi, driver-core, linux-kernel, Xu Yang, stable
In-Reply-To: <20260603-fixes_fwnode_iteration-v2-0-0ae381f8b7b9@nxp.com>
From: Xu Yang <xu.yang_2@nxp.com>
When iterate over children of a fwnode that has a secondary fwnode,
fwnode_get_next_child_node() can enter an infinite loop if the secondary
fwnode has more than one child.
Parent Child
(Primary fwnode) FWa: {FWa1, FWa2, FWa3}
(Secondary fwnode) FWb: {FWb1, FWb2}
In this case:
┌─> fwnode_get_next_child_node(FWa, FWa1)
│ - fwnode_call_ptr_op(FWa, get_next_child_node, FWa1) returns FWa2
│
│ ...
│
│ fwnode_get_next_child_node(FWa, FWa3)
│ - fwnode_call_ptr_op(FWa, get_next_child_node, FWa3) returns NULL
│ - fwnode_call_ptr_op(FWb, get_next_child_node, FWa3) returns FWb1
│
│ fwnode_get_next_child_node(FWa, FWb1)
│ - fwnode_call_ptr_op(FWa, get_next_child_node, FWb1) returns FWa1
└────┘
This cause fwnode_for_each_child_node() to loop indefinitely, reapeatedly
output {FWa1, FWa2, FWa3, FWb1, FWa1, ...}.
The root cause is that when the current child (FWb1) belongs to the
secondary fwnode, calling get_next_child_node() on the parimary fwnode
incorrectly returns the first child (FWa1) again instead of NULL.
Fix this by dynamically checking the parent fwnode of the current child
before calling get_next_child_node(). This approach follows the pattern
established in commit b5b41ab6b0c1 ("device property: Check
fwnode->secondary in fwnode_graph_get_next_endpoint()").
Fixes: 2692c614f8f0 ("device property: Allow secondary lookup in fwnode_get_next_child_node()")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- use __free() to put parent fwnode
---
drivers/base/property.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index e08eadd66f4f..f51087065bf6 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -808,17 +808,29 @@ fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
struct fwnode_handle *child)
{
struct fwnode_handle *next;
+ const struct fwnode_handle *parent;
+ struct fwnode_handle *child_parent __free(fwnode_handle) = NULL;
if (IS_ERR_OR_NULL(fwnode))
return NULL;
+ /*
+ * If this function is in a loop and the previous iteration returned
+ * an child from fwnode->secondary, then we need to use the secondary
+ * as parent rather than @fwnode.
+ */
+ if (child) {
+ child_parent = fwnode_get_parent(child);
+ parent = child_parent;
+ } else {
+ parent = fwnode;
+ }
- /* Try to find a child in primary fwnode */
- next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);
+ next = fwnode_call_ptr_op(parent, get_next_child_node, child);
if (next)
return next;
/* When no more children in primary, continue with secondary */
- return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
+ return fwnode_call_ptr_op(parent->secondary, get_next_child_node, NULL);
}
EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] software node: fix refcount leak in software_node_get_next_child()
From: Xu Yang @ 2026-06-03 8:44 UTC (permalink / raw)
To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Mauro Carvalho Chehab, Laurent Pinchart
Cc: linux-acpi, driver-core, linux-kernel, Xu Yang, stable
In-Reply-To: <20260603-fixes_fwnode_iteration-v2-0-0ae381f8b7b9@nxp.com>
From: Xu Yang <xu.yang_2@nxp.com>
When a swnode acts as a secondary fwnode and is participates in child
iteration, a refcount leak occurs for the last child of the primary
fwnode's children.
Parent Child
(Primary fwnode) FW: {FW1, FW2, FW3}
(Secondary fwnode) SW: {}
In this case, FW3's refcount is decremented twice during iteration:
fwnode_get_next_child_node(FW, FW3)
1. fwnode_call_ptr_op(FW, get_next_child_node, FW3) returns NULL and
decrements FW3's refcount
2. fwnode_call_ptr_op(SW, get_next_child_node, FW3) returns NULL and
decrements FW3's refcount again
The same double-decrement issue occurs when SW has children.
The kernel dump as below:
[ 25.435805] OF: ERROR: of_node_release() detected bad of_node_put() on /soc/usb@4c010010/usb@4c100000
[ 25.445072] CPU: 0 UID: 0 PID: 617 Comm: sh Not tainted 7.1.0-rc4-next-20260522-00011-g7376b330abca #210 PREEMPT
[ 25.445080] Hardware name: NXP i.MX95 19X19 board (DT)
[ 25.445083] Call trace:
[ 25.445086] show_stack+0x18/0x30 (C)
[ 25.445101] dump_stack_lvl+0x60/0x80
[ 25.445108] dump_stack+0x18/0x24
[ 25.445113] of_node_release+0x158/0x194
[ 25.445122] kobject_put+0xa0/0x120
[ 25.445129] of_node_put+0x18/0x28
[ 25.445134] of_fwnode_put+0x38/0x58
[ 25.445141] software_node_get_next_child+0x54/0x15c
[ 25.445150] fwnode_get_next_child_node+0x70/0x94
[ 25.445156] fwnode_get_next_available_child_node+0x34/0x88
[ 25.445162] device_links_driver_bound+0x2f4/0x334
[ 25.445168] driver_bound+0x68/0xb0
...
[ 25.445258] OF: ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'
Fix this by ensuring software_node_get_next_child() does not decrement
the child's refcount when:
- The parent has no children, OR
- The parent has children but the input child is not a swnode
This prevents the refcount from being incorrectly decremented for
fwnodes that don't belong to the software node hierarchy.
Fixes: fb5ec981adf0 ("media: software_node: Fix refcounts in software_node_get_next_child()")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- no changes
---
drivers/base/swnode.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 869228a65cb3..507de464d387 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -474,18 +474,18 @@ software_node_get_next_child(const struct fwnode_handle *fwnode,
struct swnode *p = to_swnode(fwnode);
struct swnode *c = to_swnode(child);
- if (!p || list_empty(&p->children) ||
- (c && list_is_last(&c->entry, &p->children))) {
- fwnode_handle_put(child);
+ if (!p || list_empty(&p->children))
return NULL;
- }
- if (c)
+ if (c) {
+ fwnode_handle_put(child);
+ if (list_is_last(&c->entry, &p->children))
+ return NULL;
c = list_next_entry(c, entry);
- else
+ } else {
c = list_first_entry(&p->children, struct swnode, entry);
+ }
- fwnode_handle_put(child);
return fwnode_handle_get(&c->fwnode);
}
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox