From: sashiko-bot@kernel.org
To: "Ahmed Tiba" <ahmed.tiba@arm.com>
Cc: robh@kernel.org, linux-cxl@vger.kernel.org, conor+dt@kernel.org,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v7 03/10] ACPI: APEI: GHES: move GHESv2 ack and alloc helpers
Date: Wed, 08 Jul 2026 14:18:26 +0000 [thread overview]
Message-ID: <20260708141827.BFDF61F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260708-topics-ahmtib01-ras_ffh_arm_internal_review-v7-3-8b3a85216cef@arm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The EDAC driver initializes and exits by iterating over the `ghes_devs` linked list using `list_for_each_entry_safe()`, but fails to acquire the protective `ghes_devs_mutex`. Concurrent modification of this list by `ghes_probe()` or `ghes_remove()` (e.g., triggered via sysfs bind/unbind) will cause list corruption and use-after-free. This problem wasn't introduced by this patch, but pre-existed in the codebase.
--
--- Patch [3]: [PATCH v7 03/10] ACPI: APEI: GHES: move GHESv2 ack and alloc helpers ---
commit bf19da8163cfc99677668eb5a9ba4a2f8cc72b57
Author: Ahmed Tiba <ahmed.tiba@arm.com>
ACPI: APEI: GHES: move GHESv2 ack and alloc helpers
This commit moves the GHESv2 acknowledgment and error-source allocation
helpers from ghes.c into ghes_cper.c. The logic is kept unchanged while
making the helpers reusable.
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index f74fd5de34cc5..bda535224422d 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the GHES
codebase, I noticed a potential locking omission involving the ghes_devs
list.
Does the EDAC driver need to hold ghes_devs_mutex during initialization
and teardown?
Looking at drivers/edac/ghes_edac.c, ghes_edac_init() and ghes_edac_exit()
traverse the ghes_devs list locklessly:
drivers/edac/ghes_edac.c:ghes_edac_init() {
...
list_for_each_entry_safe(g, g_tmp, ghes_devs, elist) {
ghes_edac_register(g->dev);
}
...
}
However, in drivers/acpi/apei/ghes.c, ghes_remove() deletes entries from
this list under the mutex:
drivers/acpi/apei/ghes.c:ghes_remove() {
...
mutex_lock(&ghes_devs_mutex);
list_del(&ghes->elist);
mutex_unlock(&ghes_devs_mutex);
...
}
Could concurrent execution of module loading/unloading (e.g., modprobe
ghes_edac) and GHES platform device bind/unbind via sysfs cause linked
list corruption or a use-after-free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-topics-ahmtib01-ras_ffh_arm_internal_review-v7-0-8b3a85216cef@arm.com?part=3
next prev parent reply other threads:[~2026-07-08 14:18 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-08 13:58 [PATCH v7 00/10] ACPI: APEI: share GHES CPER helpers and add DT FFH provider Ahmed Tiba
2026-07-08 13:59 ` [PATCH v7 01/10] ACPI: APEI: GHES: share macros via a private header Ahmed Tiba
2026-07-08 14:18 ` sashiko-bot
2026-07-08 13:59 ` [PATCH v7 02/10] ACPI: APEI: GHES: move CPER read helpers Ahmed Tiba
2026-07-08 14:20 ` sashiko-bot
2026-07-08 13:59 ` [PATCH v7 03/10] ACPI: APEI: GHES: move GHESv2 ack and alloc helpers Ahmed Tiba
2026-07-08 14:18 ` sashiko-bot [this message]
2026-07-08 13:59 ` [PATCH v7 04/10] ACPI: APEI: GHES: move estatus cache helpers Ahmed Tiba
2026-07-08 14:19 ` sashiko-bot
2026-07-08 13:59 ` [PATCH v7 05/10] ACPI: APEI: GHES: move vendor record helpers Ahmed Tiba
2026-07-08 14:20 ` sashiko-bot
2026-07-08 13:59 ` [PATCH v7 06/10] ACPI: APEI: GHES: move CXL CPER helpers Ahmed Tiba
2026-07-08 14:19 ` sashiko-bot
2026-07-14 2:45 ` Alison Schofield
2026-07-14 9:27 ` Ahmed Tiba
2026-07-14 17:33 ` Alison Schofield
2026-07-08 13:59 ` [PATCH v7 07/10] ACPI: APEI: introduce GHES helper Ahmed Tiba
2026-07-08 14:21 ` sashiko-bot
2026-07-08 13:59 ` [PATCH v7 08/10] ACPI: APEI: share GHES CPER helpers Ahmed Tiba
2026-07-08 14:44 ` sashiko-bot
2026-07-08 13:59 ` [PATCH v7 09/10] dt-bindings: firmware: add arm,ras-cper Ahmed Tiba
2026-07-08 14:20 ` sashiko-bot
2026-07-08 13:59 ` [PATCH v7 10/10] RAS: add firmware-first CPER provider Ahmed Tiba
2026-07-08 14:35 ` sashiko-bot
2026-07-12 22:21 ` Uwe Kleine-König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260708141827.BFDF61F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=ahmed.tiba@arm.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.