Re: [PATCH v1] s390/iomem: register initrd as an iomem resource when retain_initrd is set

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] When 'retain_initrd' is specified but no initrd is actually loaded, a spurious 1-byte resource is incorrectly registered at physical address 0.
- [Medium] Divergence between s390 `strstr()` boot parameter parsing and the core kernel `__setup()` parsing leads to freed initrd memory being falsely exposed as busy.
--

--- Patch [1]: [PATCH v1] s390/iomem: register initrd as an iomem resource when retain_initrd is set ---
commit 0c196150a4a4c949fd3f9d1707d0bc631a2fd7ca
Author: Joseph Cathcart <[email protected]>

s390/iomem: register initrd as an iomem resource when retain_initrd is set

Currently, /proc/iomem contains no reference to initrd memory, even
when initrd is retained and still held in RAM. This makes the memory
ranges invisible from userspace, and unusable to programs such as kexec.

> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
> index b60284328fe3d..a3c57ff40bf81 100644
> --- a/arch/s390/kernel/setup.c
> +++ b/arch/s390/kernel/setup.c

[ ... ]

> @@ -501,6 +508,17 @@ static void __init setup_resources(void)
>  	bss_resource.start = __pa_symbol(__bss_start);
>  	bss_resource.end = __pa_symbol(__bss_stop) - 1;
>  
> +	if (strstr(boot_command_line, "retain_initrd"))
> +		retain_initrd = true;

[Severity: Medium]
Could this strstr() match substrings like "retain_initrd=0" or
"no_retain_initrd"?

The core kernel parser in init/initramfs.c (__setup("retain_initrd", ...))
enforces strict matching and rejects parameters like "=0". If the core kernel
leaves its do_retain_initrd flag false and frees the initrd memory, wouldn't
this substring match cause s390 to incorrectly mark the freed initrd memory
as an IORESOURCE_BUSY resource?

> +	if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && retain_initrd) {
> +		unsigned long initrd_addr, initrd_size;
> +	
> +		if (get_physmem_reserved(RR_INITRD, &initrd_addr, &initrd_size)) {
> +			initrd_resource.start = initrd_addr;
> +			initrd_resource.end = initrd_resource.start + initrd_size - 1;
> +		}
> +	}
> +

[ ... ]

> @@ -517,6 +535,8 @@ static void __init setup_resources(void)
>  
>  		for (j = 0; j < ARRAY_SIZE(standard_resources); j++) {
>  			std_res = standard_resources[j];
> +			if (std_res == &initrd_resource && !retain_initrd)
> +				continue;

[Severity: Medium]
If the retain_initrd parameter is provided but no initrd is actually loaded
(e.g. get_physmem_reserved() returns 0), retain_initrd remains true but
initrd_resource bounds are never set.

Does this result in an uninitialized initrd_resource being erroneously
registered as a 1-byte resource at physical address 0?

>  			if (std_res->start < res->start ||
>  			    std_res->start > res->end)
>  				continue;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1