| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0g_f3Ut_8nf1NM5nJb3OSS83ccS65sT=DZOu0LWuCWZ1Q@mail.gmail.com>
Date: Mon, 15 Sep 2025 21:33:43 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Hanjun Guo <guohanjun@...wei.com>, Jiaqi Yan <jiaqiyan@...gle.com>
Cc: tony.luck@...el.com, rafael@...nel.org, dan.j.williams@...el.com,
bp@...en8.de, mchehab@...nel.org, xueshuai@...ux.alibaba.com,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] ACPI: APEI: EINJ: Allow more types of addresses except MMIO
On Wed, Sep 10, 2025 at 4:57 PM Hanjun Guo <guohanjun@...wei.com> wrote:
>
> On 2025/9/10 12:45, Jiaqi Yan wrote:
> > EINJ driver today only allows injection request to go through for two
> > kinds of IORESOURCE_MEM: IORES_DESC_PERSISTENT_MEMORY and
> > IORES_DESC_SOFT_RESERVED. This check prevents user of EINJ to test
> > memory corrupted in many interesting areas:
> >
> > - Legacy persistent memory
> > - Memory claimed to be used by ACPI tables or NV storage
> > - Kernel crash memory and others
> >
> > There is need to test how kernel behaves when something consumes memory
> > errors in these memory regions. For example, if certain ACPI table is
> > corrupted, does kernel crash gracefully to prevent "silent data
> > corruption". For another example, legacy persistent memory, when managed
> > by Device DAX, does support recovering from Machine Check Exception
> > raised by memory failure, hence worth to be tested.
> >
> > However, attempt to inject memory error via EINJ to legacy persistent
> > memory or ACPI owned memory fails with -EINVAL.
> >
> > Allow EINJ to inject at address except it is MMIO. Leave it to the BIOS
> > or firmware to decide what is a legitimate injection target.
> >
> > In addition to the test done in [1], on a machine having the following
> > iomem resources:
> >
> > ...
> > 01000000-08ffffff : Crash kernel
> > 768f0098-768f00a7 : APEI EINJ
> > ...
> > 768f4000-77323fff : ACPI Non-volatile Storage
> > 77324000-777fefff : ACPI Tables
> > 777ff000-777fffff : System RAM
> > 77800000-7fffffff : Reserved
> > 80000000-8fffffff : PCI MMCONFIG 0000 [bus 00-ff]
> > 90040000-957fffff : PCI Bus 0000:00
> > ...
> > 300000000-3ffffffff : Persistent Memory (legacy)
> > ...
> >
> > I commented __einj_error_inject during the test and just tested when
> > injecting a memory error at each start address shown above:
> > - 0x80000000 and 0x90040000 both failed with EINVAL
> > - request passed through for all other addresses
> >
> > Signed-off-by: Jiaqi Yan <jiaqiyan@...gle.com>
> > ---
> >
> > Changelog
> >
> > v2 [2] -> v3:
> > - Remove unnecessary IORES_DESC_CXL per comment from Hanjun [3].
> > - Minor update to code comment.
> >
> > v1 [1] -> v2:
> > - In addition to allow IORES_DESC_PERSISTENT_MEMORY_LEGACY, open the
> > door wider and only exclude MMIO per suggestion from Tony [4].
> > - Rebased to commit 11e7861d680c ("Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm").
> >
> > [1] https://lore.kernel.org/linux-acpi/20250825223348.3780279-1-jiaqiyan@google.com
> > [2] https://lore.kernel.org/linux-acpi/20250830030226.918555-1-jiaqiyan@google.com
> > [3] https://lore.kernel.org/linux-acpi/bc8ad4b8-c000-0298-efd1-4a332c4c7820@huawei.com
> > [4] https://lore.kernel.org/linux-acpi/SJ1PR11MB60835824926BEE57F094DE6FFC39A@SJ1PR11MB6083.namprd11.prod.outlook.com
> >
> > drivers/acpi/apei/einj-core.c | 51 ++++++++++++++++++++++++++++-------
> > 1 file changed, 42 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> > index 2561b045acc7b..3c87953dbd197 100644
> > --- a/drivers/acpi/apei/einj-core.c
> > +++ b/drivers/acpi/apei/einj-core.c
> > @@ -656,6 +656,43 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> > return rc;
> > }
> >
> > +/* Allow almost all types of address except MMIO. */
> > +static bool is_allowed_range(u64 base_addr, u64 size)
> > +{
> > + int i;
> > + /*
> > + * MMIO region is usually claimed with IORESOURCE_MEM + IORES_DESC_NONE.
> > + * However, IORES_DESC_NONE is treated like a wildcard when we check if
> > + * region intersects with known resource. So do an allow list check for
> > + * IORES_DESCs that definitely or most likely not MMIO.
> > + */
> > + int non_mmio_desc[] = {
> > + IORES_DESC_CRASH_KERNEL,
> > + IORES_DESC_ACPI_TABLES,
> > + IORES_DESC_ACPI_NV_STORAGE,
> > + IORES_DESC_PERSISTENT_MEMORY,
> > + IORES_DESC_PERSISTENT_MEMORY_LEGACY,
> > + /* Treat IORES_DESC_DEVICE_PRIVATE_MEMORY as MMIO. */
> > + IORES_DESC_RESERVED,
> > + IORES_DESC_SOFT_RESERVED,
> > + };
> > +
> > + if (region_intersects(base_addr, size, IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE)
> > + == REGION_INTERSECTS)
> > + return true;
> > +
> > + for (i = 0; i < ARRAY_SIZE(non_mmio_desc); ++i) {
> > + if (region_intersects(base_addr, size, IORESOURCE_MEM, non_mmio_desc[i])
> > + == REGION_INTERSECTS)
> > + return true;
> > + }
> > +
> > + if (arch_is_platform_page(base_addr))
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > /* Inject the specified hardware error */
> > int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> > u64 param4)
> > @@ -702,19 +739,15 @@ int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> > * Disallow crazy address masks that give BIOS leeway to pick
> > * injection address almost anywhere. Insist on page or
> > * better granularity and that target address is normal RAM or
> > - * NVDIMM.
> > + * as long as is not MMIO.
>
> Thanks for updating this as well.
>
> > */
> > base_addr = param1 & param2;
> > size = ~param2 + 1;
> >
> > - if (((param2 & PAGE_MASK) != PAGE_MASK) ||
> > - ((region_intersects(base_addr, size, IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE)
> > - != REGION_INTERSECTS) &&
> > - (region_intersects(base_addr, size, IORESOURCE_MEM, IORES_DESC_PERSISTENT_MEMORY)
> > - != REGION_INTERSECTS) &&
> > - (region_intersects(base_addr, size, IORESOURCE_MEM, IORES_DESC_SOFT_RESERVED)
> > - != REGION_INTERSECTS) &&
> > - !arch_is_platform_page(base_addr)))
> > + if ((param2 & PAGE_MASK) != PAGE_MASK)
> > + return -EINVAL;
> > +
> > + if (!is_allowed_range(base_addr, size))
> > return -EINVAL;
> >
> > if (is_zero_pfn(base_addr >> PAGE_SHIFT))
>
> Reviewed-by: Hanjun Guo <guohanjun@...wei.com>
Applied as 6.18 material, thanks!
Powered by blists - more mailing lists