Re: [PATCH 1/2] s390/ipl: Fix NULL deref in kdump without re-IPL parm block

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] NULL pointer dereference in `dump_reipl_run()` when the system is booted without an IPL parameter block.
- [High] Unbounded `memcpy()` in `reipl_type_init()` causes a heap buffer overflow in the dump kernel.
--

--- Patch [1]: [PATCH 1/2] s390/ipl: Fix NULL deref in kdump without re-IPL parm block ---
commit f223a6548d3bfb367ccf07d1f7597eab8c386045
Author: Vasily Gorbik <[email protected]>

s390/ipl: Fix NULL deref in kdump without re-IPL parm block

Some IPL types, like HMC FTP boot or QEMU direct kernel boot, might
not provide an IPL parameter block. In this case, reipl_type_init()
selects IPL_TYPE_UNKNOWN, and reipl_block_actual remains NULL.

> diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
> index 3c346b02ceb95..7024fc4137152 100644
> --- a/arch/s390/kernel/ipl.c
> +++ b/arch/s390/kernel/ipl.c
> @@ -1157,6 +1157,8 @@ static struct attribute_group reipl_nss_attr_group = {
>  
>  void set_os_info_reipl_block(void)
>  {
> +	if (!reipl_block_actual)
> +		return;
>  	os_info_entry_add_data(OS_INFO_REIPL_BLOCK, reipl_block_actual,
>  			       reipl_block_actual->hdr.len);
>  }

[Severity: High]
This is a pre-existing issue, but looking at dump_reipl_run(), if the system
boots without an IPL parameter block and sets reipl_block_actual to NULL,
won't this unconditionally dereference it?

arch/s390/kernel/ipl.c:dump_reipl_run() {
    ...
	csum = (__force unsigned int)cksm(reipl_block_actual,
					  reipl_block_actual->hdr.len, 0);
	abs_lc = get_abs_lowcore();
	abs_lc->ipib = __pa(reipl_block_actual);
    ...
}

Will this cause a recursive fault during a crash shutdown sequence when the
on_panic action is configured to dump_reipl?

[Severity: High]
This isn't a bug introduced by this patch, but looking at reipl_type_init(),
could an untrusted size from the crashed kernel memory overflow the
PAGE_SIZE buffers here?

arch/s390/kernel/ipl.c:reipl_type_init() {
    ...
	reipl_block = os_info_old_entry(OS_INFO_REIPL_BLOCK, &size);
	if (!reipl_block)
		goto out;
	/*
	 * If we have an OS info reipl block, this will be used
	 */
	if (reipl_block->pb0_hdr.pbt == IPL_PBT_FCP) {
		memcpy(reipl_block_fcp, reipl_block, size);
		reipl_type = IPL_TYPE_FCP;
	} else if (reipl_block->pb0_hdr.pbt == IPL_PBT_NVME) {
    ...
}

If a malicious primary kernel crafts an os_info payload with a valid
checksum but a size greater than PAGE_SIZE, wouldn't this cause a heap
buffer overflow in the dump kernel, preventing the crash dump from being
collected?

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