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

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

[Ștefan Ghețu]

Commit 58bb5081cba1 ("ASoC: SOF: Xtensa: dump ar registers to restore
call stack") added a shared Xtensa helper that iterates over a flexible
array of AR registers (`ar[]`) controlled by `plat_hdr.numaregs`.

While Intel IPC4 allocates dynamic storage for the AR block, the i.MX
IPC3 path reads the oops message into a stack-allocated struct without
backing storage for `ar[]`, while leaving `numaregs` unvalidated. This
causes a stack out-of-bounds read when printing a DSP panic.

Clear `numaregs` to 0 on i.MX since the AR block is not fetched or
supported on this platform, preventing unsafe out-of-bounds memory
accesses in the shared Xtensa helper.

Fixes: 58bb5081cba1 ("ASoC: SOF: Xtensa: dump ar registers to restore call stack")
Signed-off-by: Ștefan Ghețu <stefanghetu9@gmail.com>
---
 sound/soc/sof/imx/imx-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c
index 7a03c8cc5dd4..436fe49246ba 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;
 
 	/* then get panic info */
 	if (xoops->arch_hdr.totalsize > EXCEPT_MAX_HDR_SIZE) {
-- 
2.53.0

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

[sashiko-bot]

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

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

[Ștefan Ghețu]

struct sof_ipc_panic_info carries the panic location as a fixed 32 byte
array, and include/sound/sof/trace.h documents that the "filename array
will not include null terminator if fully filled".

sof_print_oops_and_stack() prints it with an unbounded %s, so firmware
that fills all 32 bytes leaves printk() with no terminator to stop at
within the array. It continues into the adjacent linenum field and, if
that holds no zero byte either, past the end of the structure into the
caller's stack frame, since every IPC3 dbg_dump callback passes a stack
allocated struct sof_ipc_panic_info.

Use %.*s with SOF_TRACE_FILENAME_SIZE so the print honours the
documented bound.

Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core")
Signed-off-by: Ștefan Ghețu <stefanghetu9@gmail.com>
---
 sound/soc/sof/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index 2d394389c945..9b0850e87bf6 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,
 		   panic_info->linenum);
 	sof_oops(sdev, level, oops);
 	sof_stack(sdev, level, oops, stack, stack_words);
-- 
2.53.0

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

[Ștefan Ghețu]

Commit 58bb5081cba1 ("ASoC: SOF: Xtensa: dump ar registers to restore
call stack") added a shared Xtensa helper that iterates over a flexible
array of AR registers (`ar[]`) controlled by `plat_hdr.numaregs`.

While Intel IPC4 allocates dynamic storage for the AR block, the i.MX
IPC3 path reads the oops message into a stack-allocated struct without
backing storage for `ar[]`, while leaving `numaregs` unvalidated. This
causes a stack out-of-bounds read when printing a DSP panic.

Clear `numaregs` to 0 on i.MX since the AR block is not fetched or
supported on this platform, preventing unsafe out-of-bounds memory
accesses in the shared Xtensa helper.

Fixes: 58bb5081cba1 ("ASoC: SOF: Xtensa: dump ar registers to restore call stack")
Signed-off-by: Ștefan Ghețu <stefanghetu9@gmail.com>
---
 sound/soc/sof/imx/imx-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c
index 7a03c8cc5dd4..436fe49246ba 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;
 
 	/* then get panic info */
 	if (xoops->arch_hdr.totalsize > EXCEPT_MAX_HDR_SIZE) {
-- 
2.53.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch provides an incomplete fix: a stack out-of-bounds read vulnerability in DSP oops dump remains unpatched in multiple other SOF platform drivers (Intel, AMD, MediaTek).
--

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

ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
    
This commit addresses a stack out-of-bounds read vulnerability during DSP
panic dumps on i.MX by clearing `numaregs` to 0 after reading the oops
message into a stack-allocated struct.

> 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 isn't a bug introduced by this patch, but does this leave the same stack
out-of-bounds read vulnerability unpatched in other SOF platform drivers?

It looks like several other platform drivers read the oops message into a
stack-allocated struct without clearing or validating `numaregs`:

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

If a compromised or malfunctioning DSP sends a panic IPC with a non-zero or
artificially large `numaregs` value in its header, could it still cause an
out-of-bounds stack read in these drivers when the shared Xtensa helper loops
over the unallocated `xoops->ar[]` flexible array?

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

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

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

[sashiko-bot]

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

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

[Péter Ujfalusi]

On 09/09/2026 23:40, Ștefan Ghețu wrote:
> struct sof_ipc_panic_info carries the panic location as a fixed 32 byte
> array, and include/sound/sof/trace.h documents that the "filename array
> will not include null terminator if fully filled".
> 
> sof_print_oops_and_stack() prints it with an unbounded %s, so firmware
> that fills all 32 bytes leaves printk() with no terminator to stop at
> within the array. It continues into the adjacent linenum field and, if
> that holds no zero byte either, past the end of the structure into the
> caller's stack frame, since every IPC3 dbg_dump callback passes a stack
> allocated struct sof_ipc_panic_info.
> 
> Use %.*s with SOF_TRACE_FILENAME_SIZE so the print honours the
> documented bound.

Same thing as for the ASoC: SOF: ipc3: bound firmware-supplied ext
header size.

The firmware internally constructs this and it makes sure that it is
terminated.

To change that you need to compromise the system first and when you are
there you don't need a compromised firmware.

We trust that the firmware has not been compromised as if it is it means
that the whole system has been already compromised.

> 
> Fixes: c16211d6226d ("ASoC: SOF: Add Sound Open Firmware driver core")
> Signed-off-by: Ștefan Ghețu <stefanghetu9@gmail.com>
> ---
>  sound/soc/sof/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
> index 2d394389c945..9b0850e87bf6 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,
>  		   panic_info->linenum);
>  	sof_oops(sdev, level, oops);
>  	sof_stack(sdev, level, oops, stack, stack_words);

-- 
Péter

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On Wed, Sep 09, 2026 at 11:40:42PM +0300, Ștefan Ghețu wrote:
> Commit 58bb5081cba1 ("ASoC: SOF: Xtensa: dump ar registers to restore
> call stack") added a shared Xtensa helper that iterates over a flexible
> array of AR registers (`ar[]`) controlled by `plat_hdr.numaregs`.

You've sent multiple tengentially related patches in a single thread
without anything indicating that it's a patch series.  This is really
confusing tooling, please resend as either a coherent series or
individual patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Péter Ujfalusi]

On 10/09/2026 16:17, Mark Brown wrote:
> On Wed, Sep 09, 2026 at 11:40:42PM +0300, Ștefan Ghețu wrote:
>> Commit 58bb5081cba1 ("ASoC: SOF: Xtensa: dump ar registers to restore
>> call stack") added a shared Xtensa helper that iterates over a flexible
>> array of AR registers (`ar[]`) controlled by `plat_hdr.numaregs`.
> 
> You've sent multiple tengentially related patches in a single thread
> without anything indicating that it's a patch series.  This is really
> confusing tooling, please resend as either a coherent series or
> individual patches.

I'm not sure if these patches should be applied for few reasons:
- orchestrating the exploit or error case require access to secret
signing key
- deploying the signed firmware needs root access
- in these cases the firmware could be prepared to pass the defensive
checks and still cause problems.
- creates false sense of security through obfuscation

Stefan, sorry for nacking it and thank you for the patches, I hope you
understand my side of the argument.

-- 
Péter