Re: [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch fixes a stack out-of-bounds read in the i.MX DSP panic dump by clearing `numaregs` to 0, but fails to apply this exact same fix to 5 other platform drivers that share the identical vulnerable code pattern.
- [High] Information leak of kernel stack memory when the firmware oops header size is invalid due to an early return bypassing the initialization of output variables.
- [High] Out-of-bounds string read due to a missing precision specifier for the `filename` array.
--

commit ca4aa4ca867a8171d023c31fffe2fa4cd73d904b
Author: Ștefan Ghețu <stefanghetu9@gmail.com>

ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump

This patch clears numaregs to 0 in the i.MX DSP oops parser to prevent 
a stack out-of-bounds read in the shared Xtensa helper. Since the AR block
is not supported on i.MX, this prevents unsafe memory accesses when dumping
a DSP panic.

> diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c
> index 7a03c8cc5dd47..436fe49246ba8 100644
> --- a/sound/soc/sof/imx/imx-common.c
> +++ b/sound/soc/sof/imx/imx-common.c
> @@ -34,6 +34,7 @@ void imx8_get_registers(struct snd_sof_dev *sdev,
>  
>  	/* first read registers */
>  	sof_mailbox_read(sdev, offset, xoops, sizeof(*xoops));
> +	xoops->plat_hdr.numaregs = 0;

[Severity: High]
This is a pre-existing issue, but does this fix also need to be applied
to other platform drivers?

The target patch successfully resolves the out-of-bounds read for imx8,
but there appear to be 5 other drivers that share the identical code
pattern where xoops is allocated on the stack without backing the
flexible ar[] array:

- amd_get_registers() in sound/soc/sof/amd/acp-common.c
- atom_get_registers() in sound/soc/sof/intel/atom.c
- bdw_get_registers() in sound/soc/sof/intel/bdw.c
- hda_dsp_get_registers() in sound/soc/sof/intel/hda-dsp.c
- mtk_adsp_get_registers() in sound/soc/sof/mediatek/mtk-adsp-common.c

If a compromised or malfunctioning DSP provides a non-zero numaregs
value in its oops header, could it still cause the kernel to iterate
over unbacked stack memory in xtensa_stack() and leak adjacent stack
contents via hex_dump_to_buffer() on those platforms?

>  
>  	/* then get panic info */
>  	if (xoops->arch_hdr.totalsize > EXCEPT_MAX_HDR_SIZE) {

[Severity: High]
This is a pre-existing issue, but if this header size validation fails,
can it cause an information leak?

When imx8_get_registers() aborts early here, it returns void, leaving
the caller's stack-allocated panic_info and stack arrays uninitialized.

Does the caller (such as imx8_dump()) consume these uninitialized
variables?

imx8_dump()
  imx8_get_registers(..., &panic_info, stack, ...)
  sof_print_oops_and_stack(..., &panic_info, stack, ...)

If the caller unconditionally passes these uninitialized structures to
sof_print_oops_and_stack(), could they be formatted and printed to
dmesg, leaking uninitialized kernel stack memory?

>  		dev_err(sdev->dev, "invalid header size 0x%x. FW oops is bogus\n",
>  			xoops->arch_hdr.totalsize);
>  		return;
>  	}

[Severity: High]
This is a pre-existing issue, but when sof_print_oops_and_stack()
processes the panic_info structure, can it read out of bounds?

struct sof_ipc_panic_info defines filename as an array of 32 bytes.
If the DSP provides a fully filled 32-byte filename string without a
null terminator:

core.c:sof_print_oops_and_stack() {
    ...
    dev_printk(level, sdev->dev, "panic at %s:%d\n", panic_info->filename,
               panic_info->linenum);
    ...
}

Will using the unbounded %s format specifier cause printk to read
past the end of the array into adjacent stack memory (like linenum)
until a null byte is encountered? Should this use the %.*s format
specifier with SOF_TRACE_FILENAME_SIZE?

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