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
  2026-09-16 21:09 ` Mark Brown
  0 siblings, 2 replies; 7+ 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] 7+ 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
  2026-09-16 21:09 ` Mark Brown
  1 sibling, 0 replies; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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
@ 2026-09-16 21:09 ` Mark Brown
  1 sibling, 0 replies; 7+ messages in thread
From: Mark Brown @ 2026-09-16 21:09 UTC (permalink / raw)
  To: Liam Girdwood, Peter Ujfalusi, Bard Liao, Daniel Baluta,
	Ștefan Ghețu
  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

On Tue, 08 Sep 2026 20:07:40 +0300, Ștefan Ghețu wrote:
> ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.4

Thanks!

[1/1] ASoC: SOF: imx: Prevent stack OOB read in DSP panic dump
      https://git.kernel.org/broonie/sound/c/48ed992bfbe5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


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

end of thread, other threads:[~2026-09-17 20:18 UTC | newest]

Thread overview: 7+ 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
2026-09-16 21:09 ` Mark Brown
  -- 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.