Re: [PATCH] ASoC: SOF: Bound the panic filename print to its array size

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized kernel stack memory (infoleak) is printed to the kernel log due to early returns in DSP get_registers functions on malformed firmware data.
--

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

ASoC: SOF: Bound the panic filename print to its array size

This commit limits the printk format specifier for the panic filename
to SOF_TRACE_FILENAME_SIZE to avoid over-reading a non-null-terminated
string array.

> diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
> index 2d394389c945b..9b0850e87bf60 100644
> --- a/sound/soc/sof/core.c
> +++ b/sound/soc/sof/core.c
> @@ -152,7 +152,8 @@ void sof_print_oops_and_stack(struct snd_sof_dev *sdev, const char *level,
>  	dev_printk(level, sdev->dev, "trace point: %#010x\n", tracep_code);
>  
>  out:
> -	dev_printk(level, sdev->dev, "panic at %s:%d\n", panic_info->filename,
> +	dev_printk(level, sdev->dev, "panic at %.*s:%d\n",
> +		   SOF_TRACE_FILENAME_SIZE, panic_info->filename,

[Severity: High]
This isn't a bug introduced by this patch, but does this function leak
uninitialized kernel stack memory if the DSP firmware sends a malformed oops
header?

Looking at caller sequences like hda_dsp_dump(), the panic_info and stack
variables are allocated on the stack without zero-initialization:

sound/soc/sof/intel/hda-dsp.c:hda_dsp_dump() {
	struct sof_ipc_panic_info panic_info;
	u32 stack[HDA_DSP_STACK_DUMP_SIZE];
	...
	hda_dsp_get_registers(sdev, &xoops, &panic_info, stack, ...);
	sof_print_oops_and_stack(..., &panic_info, stack, ...);
}

However, architecture-specific register getters like hda_dsp_get_registers()
and atom_get_registers() can return early without initializing this memory
if the header size is invalid:

sound/soc/sof/intel/hda-dsp.c:hda_dsp_get_registers() {
	if (xoops->arch_hdr.totalsize > EXCEPT_MAX_HDR_SIZE) {
		dev_err(sdev->dev, "invalid header size 0x%x. FW oops is bogus\n", ...);
		return;
	}
	...
}

Because these getter functions return void rather than an error code, the
callers continue unconditionally and pass the uninitialized variables to
sof_print_oops_and_stack().

This function then logs panic_info->filename, panic_info->linenum, and
loops through the uninitialized stack array via sof_stack(). Could this
allow a buggy or compromised firmware to trigger a dump of uninitialized
kernel stack memory into the kernel log?

>  		   panic_info->linenum);
>  	sof_oops(sdev, level, oops);
>  	sof_stack(sdev, level, oops, stack, stack_words);

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