| 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: <88893809-ed13-dbb9-2446-8fd680f57693@huawei.com>
Date: Wed, 10 Sep 2025 22:57:11 +0800
From: Hanjun Guo <guohanjun@...wei.com>
To: Jiaqi Yan <jiaqiyan@...gle.com>, <tony.luck@...el.com>,
<rafael@...nel.org>
CC: <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 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>
Thanks
Hanjun
Powered by blists - more mailing lists