All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
@ 2026-09-08 17:07 Ștefan Ghețu
  2026-09-08 17:19 ` sashiko-bot
  0 siblings, 1 reply; 6+ messages in thread
From: Ștefan Ghețu @ 2026-09-08 17:07 UTC (permalink / raw)
  To: Liam Girdwood, Peter Ujfalusi, Bard Liao, Daniel Baluta,
	Mark Brown
  Cc: Kai Vehmanen, Pierre-Louis Bossart, Vijendar Mukunda,
	Jaroslav Kysela, Takashi Iwai, Frank Li, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, sound-open-firmware,
	linux-sound, imx, linux-arm-kernel, linux-kernel,
	Ș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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
  2026-09-08 17:07 [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump Ștefan Ghețu
@ 2026-09-08 17:19 ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-08 17:19 UTC (permalink / raw)
  To: Ștefan Ghețu; +Cc: Frank.Li, imx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
  2026-09-09 20:40 [PATCH] ASoC: SOF: Bound the panic filename print to its array size Ștefan Ghețu
@ 2026-09-09 20:40 ` Ștefan Ghețu
  2026-09-09 20:49   ` sashiko-bot
  2026-09-10 13:17   ` Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Ștefan Ghețu @ 2026-09-09 20:40 UTC (permalink / raw)
  To: Liam Girdwood, Peter Ujfalusi, Bard Liao, Daniel Baluta,
	Mark Brown
  Cc: Kai Vehmanen, Pierre-Louis Bossart, Vijendar Mukunda,
	Jaroslav Kysela, Takashi Iwai, Frank Li, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ranjani Sridharan,
	sound-open-firmware, linux-sound, linux-kernel, imx,
	linux-arm-kernel, Ș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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
  2026-09-09 20:40 ` [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump Ștefan Ghețu
@ 2026-09-09 20:49   ` sashiko-bot
  2026-09-10 13:17   ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-09 20:49 UTC (permalink / raw)
  To: Ștefan Ghețu; +Cc: imx, Frank.Li

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
  2026-09-09 20:40 ` [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump Ștefan Ghețu
  2026-09-09 20:49   ` sashiko-bot
@ 2026-09-10 13:17   ` Mark Brown
  2026-09-10 13:26     ` Péter Ujfalusi
  1 sibling, 1 reply; 6+ messages in thread
From: Mark Brown @ 2026-09-10 13:17 UTC (permalink / raw)
  To: Ștefan Ghețu
  Cc: Liam Girdwood, Peter Ujfalusi, Bard Liao, Daniel Baluta,
	Kai Vehmanen, Pierre-Louis Bossart, Vijendar Mukunda,
	Jaroslav Kysela, Takashi Iwai, Frank Li, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ranjani Sridharan,
	sound-open-firmware, linux-sound, linux-kernel, imx,
	linux-arm-kernel

[-- 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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
  2026-09-10 13:17   ` Mark Brown
@ 2026-09-10 13:26     ` Péter Ujfalusi
  0 siblings, 0 replies; 6+ messages in thread
From: Péter Ujfalusi @ 2026-09-10 13:26 UTC (permalink / raw)
  To: Mark Brown, Ștefan Ghețu
  Cc: Liam Girdwood, Bard Liao, Daniel Baluta, Kai Vehmanen,
	Pierre-Louis Bossart, Vijendar Mukunda, Jaroslav Kysela,
	Takashi Iwai, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Ranjani Sridharan, sound-open-firmware,
	linux-sound, linux-kernel, imx, linux-arm-kernel



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



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-10 13:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 17:07 [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump Ștefan Ghețu
2026-09-08 17:19 ` sashiko-bot
  -- strict thread matches above, loose matches on Subject: below --
2026-09-09 20:40 [PATCH] ASoC: SOF: Bound the panic filename print to its array size Ștefan Ghețu
2026-09-09 20:40 ` [PATCH] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump Ștefan Ghețu
2026-09-09 20:49   ` sashiko-bot
2026-09-10 13:17   ` Mark Brown
2026-09-10 13:26     ` Péter Ujfalusi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.