Re: [PATCH] mtd: phram: reject non-RAM ranges for module parameters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mtd: phram: requiring System RAM in register_device() breaks phram use cases
- [High] mtd: phram: incomplete RAM range check in register_device() allows kernel oops
--

commit f4da2010e9e5aceb9b55f0b4d654c5eceb1299d4
Author: Rahul Kumar <rk0006818@gmail.com>

mtd: phram: reject non-RAM ranges for module parameters

This commit updates register_device() in the phram driver to require that
module-parameter physical address ranges intersect IORESOURCE_SYSTEM_RAM and
defaults those mappings to cached memremap().

> diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> index b42cadcd76b11..9747e7e58e6fc 100644
> --- a/drivers/mtd/devices/phram.c
> +++ b/drivers/mtd/devices/phram.c
[ ... ]
> @@ -126,10 +127,16 @@ static int register_device(struct platform_device *pdev, const char *name,
>  			   phys_addr_t start, size_t len, uint32_t erasesize)
>  {
>  	struct device_node *np = pdev ? pdev->dev.of_node : NULL;
> -	bool cached = np ? !of_property_read_bool(np, "no-map") : false;
> +	bool cached = np ? !of_property_read_bool(np, "no-map") : true;
>  	struct phram_mtd_list *new;
>  	int ret = -ENOMEM;
>  
> +	if (!np && region_intersects(start, len, IORESOURCE_SYSTEM_RAM,
> +				     IORES_DESC_NONE) != REGION_INTERSECTS) {

[Severity: High]
Won't requiring IORESOURCE_SYSTEM_RAM and defaulting to cached mappings in
register_device() introduce a regression that breaks the documented
module-parameter use cases for phram while exposing active kernel memory?

In drivers/mtd/devices/Kconfig, MTD_PHRAM is documented as:

	config MTD_PHRAM
		tristate "Physical system RAM"
		help
		  This is a re-implementation of the slram driver above.

		  Use this driver to access physical memory that the kernel proper
		  doesn't have access to, memory beyond the mem=xxx limit, nvram,
		  memory on the video card, etc...

Physical memory beyond the mem=xxx limit is removed from the memory map during
early boot before iomem_resource is populated, for example in
arch/x86/kernel/e820.c:parse_memopt():

	e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM);

(and similarly via arm64_memblock_init() -> memblock_mem_limit_remove_map() on
arm64, early_mem() on arm, and setup_bootmem() ->
memblock_enforce_memory_limit() on riscv). Likewise, ranges reserved via
memmap=nn$ss or memmap=nn!ss are registered as IORESOURCE_MEM rather than
IORESOURCE_SYSTEM_RAM in arch/x86/kernel/e820.c:e820_type_to_iomem_type():

	switch (entry->type) {
	case E820_TYPE_RAM:		return IORESOURCE_SYSTEM_RAM;
	...
	case E820_TYPE_PRAM:		/* Fall-through: */
	case E820_TYPE_PMEM:		/* Fall-through: */
	case E820_TYPE_RESERVED:	/* Fall-through: */
	case E820_TYPE_SOFT_RESERVED:	/* Fall-through: */
	default:			return IORESOURCE_MEM;
	}

NVRAM and video card memory are also not registered as IORESOURCE_SYSTEM_RAM.
When phram_setup() -> register_device() is called for any of these ranges via
the module parameter (where np is NULL), region_intersects() returns
REGION_DISJOINT and rejects them with -EINVAL.

Conversely, ranges that do match IORESOURCE_SYSTEM_RAM with IORES_DESC_NONE in
iomem_resource are active System RAM managed by the page allocator or holding
kernel text, data, and slab caches. Previously, cached was false when !np, so
phram_map() called ioremap(), which refused to map active unreserved System
RAM. With cached set to true and IORESOURCE_SYSTEM_RAM required,
phram_map() -> memremap(start, len, MEMREMAP_WB) -> try_ram_remap() in
kernel/iomem.c runs:

	if (pfn_valid(pfn) && !PageHighMem(pfn_to_page(pfn)) &&
	    arch_memremap_can_ram_remap(offset, size, flags))
		return __va(offset);

Doesn't this return the kernel linear mapping __va(start) without checking
PageReserved(), allowing reads, writes, or erases on the resulting MTD device
to corrupt or disclose live kernel memory?

> +		pr_err("start/len is not System RAM\n");
> +		return -EINVAL;
> +	}

[Severity: High]
Even if the goal is to restrict module-parameter devices to System RAM, can a
range that partially overlaps an unmapped physical memory hole (or wraps
around) still pass this region_intersects() check and trigger a kernel oops or
ioremap warning?

As described in the comment above region_intersects() in kernel/resource.c:

	 * Return REGION_DISJOINT if the region does not overlap @flags/@desc,
	 * return REGION_MIXED if the region overlaps @flags/@desc and another
	 * resource, and return REGION_INTERSECTS if the region overlaps @flags/@desc
	 * and no other defined resource. Note that REGION_INTERSECTS is also
	 * returned in the case when the specified region overlaps RAM and undefined
	 * memory holes.

In kernel/resource.c:__region_intersects(), REGION_INTERSECTS is returned
whenever the range overlaps at least one matching IORESOURCE_SYSTEM_RAM
resource and no other defined resource in iomem_resource. Undefined physical
memory holes have no resource entry, and DEFINE_RES(start, size, 0) computes
start + size - 1 without overflow checking.

When phram_map() then calls memremap(start, len, MEMREMAP_WB),
kernel/iomem.c:try_ram_remap() only validates the starting page via
PHYS_PFN(offset) and ignores size:

1. If start is inside System RAM and start + len extends past the end of
System RAM into an unmapped hole (or start + len - 1 wraps around inside the
same System RAM resource), region_intersects() returns REGION_INTERSECTS and
try_ram_remap() returns __va(start). When mtd_device_register() scans the
device or userspace accesses it, won't phram_read(), phram_write(), or
phram_erase() call memcpy() or memset() past the end of the linear direct map
and fault?

2. If start lies in an undefined hole before System RAM and start + len
extends into System RAM, region_intersects() still returns REGION_INTERSECTS,
while try_ram_remap() sees !pfn_valid(PHYS_PFN(start)) and falls back to
arch_memremap_wb() -> ioremap_cache(start, len). Won't that still trigger the
WARN_ONCE() in __ioremap_caller() on x86 when checking the overlapping RAM
tail, or map the non-RAM hole on arm64 and fault in memcpy() when FTL scans
the device?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021115348.799538-1-rk0006818@gmail.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/