Re: [PATCH v5 5/9] mm/memory-failure: efi: record hardware-poisoned frames into the poisoned-memory table

[Breno Leitao <leitao@debian.org>]

On Tue, Sep 15, 2026 at 01:36:29PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Critical] efi: memblock_add() with PAGE_ALIGN_DOWN forces adjacent MMIO into system RAM
> - [High] efi: ABI breakage in linux_efi_poisoned_memory due to native word size
> - [Medium] mm: hard memory failures on soft-offlined pages are omitted from EFI table
> --
> 
> commit e69689c81050c18119d69206dc9a7cb1e2fa4eeb
> Author: Breno Leitao <leitao@debian.org>
> 
> mm/memory-failure: efi: record hardware-poisoned frames into the poisoned-memory table
> 
> This commit hooks memory_failure() hard offline reports into the EFI
> poisoned memory table so that they persist across kexec. Soft-offlined
> pages are deliberately excluded to prevent predictive offlining from
> becoming a permanent loss.
> 
> > diff --git a/drivers/firmware/efi/poison.c b/drivers/firmware/efi/poison.c
> > index 3f12db3dc9b84..847592862f01d 100644
> > --- a/drivers/firmware/efi/poison.c
> > +++ b/drivers/firmware/efi/poison.c
> > @@ -91,3 +91,30 @@ void __init efi_poisoned_memory_reserve(void)
> >  	memblock_add(start, end - start);
> >  	memblock_reserve(start, end - start);
> 
> [Severity: Critical]
> Does this forcefully override the firmware's memory map for adjacent 4KB
> chunks on larger page architectures? 
> 
> On architectures like ARM64 where the kernel page size can be 16KB or
> 64KB, EFI memory descriptors remain 4KB. If the EFI poisoned memory table
> occupies a 4KB region within a 64KB page, rounding down to the kernel
> page boundary expands the range.
> 
> This could force adjacent 4KB EFI regions containing firmware MMIO or
> reserved non-RAM regions into system RAM via memblock_add(). If the
> kernel later maps these regions with Write-Back caching attributes in the
> direct map, could this cause fatal Machine Check Exceptions or SError
> panics during speculative CPU accesses?

arm64 already rounds every EFI descriptor outward via
memrange_efi_to_native(), so it adds no new class of over-mapping, and
reserve_regions() adds and reserves this WB ACPI-reclaim table anyway
moments later.

So, it doesn't seem relevant?!

> > diff --git a/include/linux/efi.h b/include/linux/efi.h index
> > dd3263456dd4a..56402fdccd114 100644 --- a/include/linux/efi.h +++
> > b/include/linux/efi.h @@ -1288,8 +1288,10 @@ struct
> > linux_efi_poisoned_memory {
> 
> [Severity: High] Does the linux_efi_poisoned_memory ABI structure
> break on architectures where the native word size differs from the
> firmware? 
> 
> The struct uses an unsigned long flexible array for the bitmap, which
> alters the element size depending on the kernel bitness (4 bytes on
> 32-bit and 8 bytes on 64-bit). efi_poison_geometry_valid() rejects
> tables where the size is not a multiple of sizeof(unsigned long). 
> 
> If a 32-bit EFI firmware generates this table, it will use 4-byte
> alignment and a 32-bit word array. Because CONFIG_EFI_POISONED_MEMORY
> is 64-bit only, the 64-bit kernel will evaluate sizeof(unsigned long)
> as 8 and reject perfectly valid 32-bit firmware tables as malformed. 
> 
> Since bit array layouts also differ across word sizes on big-endian
> architectures, should this firmware-to-kernel ABI boundary use
> fixed-size types like u32 or u64 instead?

That is not the case, because this feature depends on 64-bits and
!BIG_ENDIAN.

In fact, CONFIG_EFI is depends on OF && !CPU_BIG_ENDIAN
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c index
> > a2ca8df501cae..d9b8be696aac3 100644 --- a/mm/memory-failure.c +++
> > b/mm/memory-failure.c
> [ ... ]
> > @@ -1326,6 +1327,8 @@ static int action_result(unsigned long pfn,
> > enum mf_action_page_type type, if (type != MF_MSG_ALREADY_POISONED
> > && type != MF_MSG_PFN_MAP) { num_poisoned_pages_inc(pfn);
> > update_per_node_mf_stats(pfn, result); +		/* Only hard
> > offlines are carried over to the next kernel. */
> > +		efi_hwpoison_record_pfn(pfn); }
> 
> [Severity: Medium] Does placing efi_hwpoison_record_pfn() inside this
> conditional block unintentionally skip recording hard errors on
> previously soft-offlined pages? 
> 
> Soft-offline marks a page with PageHWPoison but intentionally does not
> record it in the EFI table to avoid permanent loss for predictive
> errors. 
> 
> If a genuine hard, uncorrectable error later occurs on that same page,
> memory_failure() will see the existing PageHWPoison flag and pass
> MF_MSG_ALREADY_POISONED to action_result(). 
> 
> By coupling the EFI recording to the meminfo stat increments, genuine
> hard errors on soft-offlined pages are omitted from the EFI table.
> Could this allow the next kernel to map and crash on physically broken
> RAM?

Yes, this one is real, thanks.

soft_offline_page() reaches num_poisoned_pages_inc() through
page_handle_poison() and never calls action_result(), so the hook does
not need the MF_MSG_ALREADY_POISONED guard to keep soft offlines out of
the table. That guard is there for the stats, and coupling to it drops a
hard error on a page that was soft-offlined earlier.

The frame is not lost for good -- the next kernel hands it out, the error
comes back and is recorded then -- but that is one machine check per
kexec that this series exists to avoid.

I will move the call out of the stats guard in v6, keeping MF_MSG_PFN_MAP
excluded since that path is not System RAM