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

[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

[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

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