Re: [PATCH 6/8] firmware: meson_sm: make reg_write callback take const void *

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

Pre-existing issues:
- [High] Concurrent calls to `meson_sm_call_write` or `meson_sm_call_read` will corrupt the shared IOMEM buffer.
--

--- Patch [6]: [PATCH 6/8] firmware: meson_sm: make reg_write callback take const void * ---
commit 05903fc223bec5b04f497c8b4bf66c4c6d90ebfe
Author: Link Mauve <[email protected]>

firmware: meson_sm: make reg_write callback take const void *

This commit updates the meson_sm_call_write callback and its header
declaration to take a const void * buffer. This aligns with a previous
change to the nvmem_config API that switched from a mutable to an
immutable data pointer.

> diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
> index ab9751a59b552..5d4da2e3c3a94 100644
> --- a/drivers/firmware/meson/meson_sm.c
> +++ b/drivers/firmware/meson/meson_sm.c
> @@ -196,7 +196,7 @@ EXPORT_SYMBOL(meson_sm_call_read);
>   *
>   * Return:	size of sent data on success, a negative value on error
>   */
> -int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer,
> +int meson_sm_call_write(struct meson_sm_firmware *fw, const void *buffer,
>  			unsigned int size, unsigned int cmd_index, u32 arg0,
>  			u32 arg1, u32 arg2, u32 arg3, u32 arg4)
>  {

[ ... ]

>  	memcpy(fw->sm_shmem_in_base, buffer, size);
>  
>  	if (meson_sm_call(fw, cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0)
>  		return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but does this function need
locking to protect the shared IOMEM buffer?

It appears that concurrent calls to meson_sm_call_write() or
meson_sm_call_read() could corrupt the shared sm_shmem_in_base and
sm_shmem_out_base buffers. If multiple threads concurrently write or read
the nvmem sysfs files, they might overwrite each other's payloads before
the SMC executes, sending corrupted data to the Secure Monitor.

A similar pattern exists in meson_sm_call_read():

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

Are there external locks that guarantee serialized access to these functions,
or should we be synchronizing these shared memory operations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic