mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Zhentao Guo" <zhentao.guo@amlogic.com>
Cc: devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	robh@kernel.org, conor+dt@kernel.org,
	linux-amlogic@lists.infradead.org, media-ci@linuxtv.org
Subject: Re: [PATCH v8 1/6] firmware: meson: sm: video firmware loading via secure monitor
Date: Mon, 31 Aug 2026 08:43:31 +0000	[thread overview]
Message-ID: <20260831084331.86B4F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260831-b4-s4-vdec-upstream-v8-1-830195b9bd91@amlogic.com>

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

Pre-existing issues:
- [High] Information leak in `serial_show` due to uninitialized kernel heap memory being exposed on short reads.
--

commit 024494408ffd3466033a6280dd28a958f93f1072
Author: Zhentao Guo <zhentao.guo@amlogic.com>

firmware: meson: sm: video firmware loading via secure monitor

Add SM_LOAD_VIDEO_FW to the secure monitor command enum to allow decoder
drivers to load firmware through the meson_sm interface.

> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
> index 3ebc2bd9a9760..3b90b49d05529 100644
> --- a/include/linux/firmware/meson/meson_sm.h
> +++ b/include/linux/firmware/meson/meson_sm.h
> @@ -15,6 +15,7 @@ enum {
>  	SM_THERMAL_CALIB_READ,
>  	SM_A1_PWRC_SET,
>  	SM_A1_PWRC_GET,
> +	SM_LOAD_VIDEO_FW,
>  };

[Severity: High]
This is a pre-existing issue, but while reviewing this secure monitor
addition I noticed a potential information leak in serial_show() in
drivers/firmware/meson/meson_sm.c.

Could a short read in meson_sm_call_read() leak uninitialized kernel memory
to unprivileged local users reading the sysfs serial attribute?

drivers/firmware/meson/meson_sm.c:serial_show() {
    ...
	id_buf = kmalloc(SM_CHIP_ID_LENGTH, GFP_KERNEL);
	if (!id_buf)
		return -ENOMEM;

	ret = meson_sm_call_read(fw, id_buf, SM_CHIP_ID_LENGTH, SM_GET_CHIP_ID,
				 0, 0, 0, 0, 0);
	if (ret < 0) {
		kfree(id_buf);
		return ret;
	}

	ret = sprintf(buf, "%12phN\n", &id_buf[SM_CHIP_ID_OFFSET]);
    ...
}

If the secure monitor returns a positive size smaller than 16,
meson_sm_call_read() only copies those bytes. The check for ret < 0 will
not catch this short read.

Because id_buf is allocated without zeroing and the sysfs attribute is
world-readable (DEVICE_ATTR_RO), sprintf() will unconditionally format 12
bytes starting at SM_CHIP_ID_OFFSET, which could expose uninitialized
kernel heap memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-b4-s4-vdec-upstream-v8-0-830195b9bd91@amlogic.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2026-08-31  8:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31  8:32 [PATCH v8 0/6] Add Amlogic stateless H.264 video decoder for S4 Zhentao Guo via B4 Relay
2026-08-31  8:32 ` [PATCH v8 1/6] firmware: meson: sm: video firmware loading via secure monitor Zhentao Guo via B4 Relay
2026-08-31  8:43   ` sashiko-bot [this message]
2026-08-31  8:32 ` [PATCH v8 2/6] firmware: meson: sm: Add video firmware loading SMC call Zhentao Guo via B4 Relay
2026-08-31  8:45   ` sashiko-bot
2026-08-31  8:32 ` [PATCH v8 3/6] media: dt-bindings: Add Amlogic V4L2 video decoder Zhentao Guo via B4 Relay
2026-08-31  8:42   ` sashiko-bot
2026-08-31 16:04   ` Conor Dooley
2026-09-01  2:16     ` Zhentao Guo
2026-08-31  8:32 ` [PATCH v8 4/6] decoder: Add V4L2 stateless H.264 decoder driver Zhentao Guo via B4 Relay
2026-08-31  8:56   ` sashiko-bot
2026-08-31  8:32 ` [PATCH v8 5/6] arm64: dts: amlogic: Add video decoder driver support for S4 SOCs Zhentao Guo via B4 Relay
2026-08-31  8:32 ` [PATCH v8 6/6] arm64: defconfig: Enable CONFIG_VIDEO_AMLOGIC_VDEC Zhentao Guo via B4 Relay
2026-08-31  8:58   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260831084331.86B4F1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=media-ci@linuxtv.org \
    --cc=neil.armstrong@linaro.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=zhentao.guo@amlogic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®