Re: [PATCH v9 2/6] firmware: meson: sm: Add video firmware loading SMC call

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Missing synchronization around shared memory accesses for SMC calls.
--

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

firmware: meson: sm: Add video firmware loading SMC call

Add SM_LOAD_VIDEO_FW at SMC ID 0xb200000f in the command
table to load video firmware.

> diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
> index ab9751a59b552..887caa5c625db 100644
> --- a/drivers/firmware/meson/meson_sm.c
> +++ b/drivers/firmware/meson/meson_sm.c
> @@ -48,6 +48,7 @@ static const struct meson_sm_chip gxbb_chip = {
>  		CMD(SM_THERMAL_CALIB_READ,	0x82000047),
>  		CMD(SM_A1_PWRC_SET,		0x82000093),
>  		CMD(SM_A1_PWRC_GET,		0x82000095),
> +		CMD(SM_LOAD_VIDEO_FW,		0xb200000f),

[Severity: Critical]
This is a pre-existing issue, but is there missing synchronization around
shared memory accesses for SMC calls when handling these commands?

Looking at meson_sm_call_write() and meson_sm_call_read(), data is copied to
and from shared, unprotected memory regions (sm_shmem_in_base and
sm_shmem_out_base) during the SMC calls:

meson_sm_call_write() {
	...
	memcpy(fw->sm_shmem_in_base, buffer, size);

	if (meson_sm_call(fw, cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0)
		return -EINVAL;
	...
}

meson_sm_call_read() {
	...
	if (meson_sm_call(fw, cmd_index, &size, arg0, arg1, arg2, arg3, arg4) < 0)
		return -EINVAL;

	if (buffer)
		memcpy(buffer, fw->sm_shmem_out_base, size);
	...
}

Since there are no mutexes in meson_sm.c or the upstream nvmem core's
read/write handlers (nvmem_reg_read/nvmem_reg_write), concurrent access to
the SMC functions is unsynchronized. 

For example, two root processes could concurrently write to nvmem, or
concurrently read the world-readable sysfs files.

One thread could overwrite sm_shmem_in_base while another thread is
mid-sequence (after its memcpy but before its SMC call executes), leading to
the SM receiving corrupted data. 

Similarly, SMC execution and the subsequent read from sm_shmem_out_base can
be interleaved by another thread's SMC call, causing a thread to read the
other thread's returned data.

Could this concurrent access cause data corruption during one-time hardware
operations like EFUSE writes, or corrupt thermal calibration data reads and
lead to system instability?

>  		{ /* sentinel */ },
>  	},
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-b4-s4-vdec-upstream-v9-0-72fdb0d71b02@amlogic.com?part=2

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