From 0d423c4a78984dd02f6596d6fd9dd40446eec517 Mon Sep 17 00:00:00 2001 From: Alexey Romanov Date: Wed, 30 Aug 2023 17:08:50 +0300 Subject: [PATCH] drivers: meson: sm: correct meson_sm_* API retval handling 1. Following the ARM SMC32 calling convention, the return value from secure monitor is a 32-bit signed integer. This patch changes the type of the return value of the function meson_sm_call(). 2. Now, when meson_sm_call() returns a 32-bit signed integer, we need to ensure that this value is not negative. It is important to check that the return value is not negative in both the meson_sm_call_read() and meson_sm_call_write() functions. 3. Add a comment explaining why it is necessary to check if the SMC return value is equal to 0 in the function meson_sm_call_read(). It is not obvious when reading this code. Signed-off-by: Alexey Romanov Reviewed-by: Neil Armstrong Link: https://lore.kernel.org/r/20230830140850.17130-1-avromanov@salutedevices.com Signed-off-by: Neil Armstrong --- drivers/firmware/meson/meson_sm.c | 20 +++++++++++++------- include/linux/firmware/meson/meson_sm.h | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c index 9a2656d73600..53bf56e18743 100644 --- a/drivers/firmware/meson/meson_sm.c +++ b/drivers/firmware/meson/meson_sm.c @@ -67,7 +67,7 @@ static u32 meson_sm_get_cmd(const struct meson_sm_chip *chip, return cmd->smc_id; } -static u32 __meson_sm_call(u32 cmd, u32 arg0, u32 arg1, u32 arg2, +static s32 __meson_sm_call(u32 cmd, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { struct arm_smccc_res res; @@ -102,9 +102,10 @@ static void __iomem *meson_sm_map_shmem(u32 cmd_shmem, unsigned int size) * Return: 0 on success, a negative value on error */ int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index, - u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) + s32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { - u32 cmd, lret; + u32 cmd; + s32 lret; if (!fw->chip) return -ENOENT; @@ -143,7 +144,7 @@ int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer, unsigned int bsize, unsigned int cmd_index, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { - u32 size; + s32 size; int ret; if (!fw->chip) @@ -158,11 +159,16 @@ int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer, if (meson_sm_call(fw, cmd_index, &size, arg0, arg1, arg2, arg3, arg4) < 0) return -EINVAL; - if (size > bsize) + if (size < 0 || size > bsize) return -EINVAL; ret = size; + /* In some cases (for example GET_CHIP_ID command), + * SMC doesn't return the number of bytes read, even + * though the bytes were actually read into sm_shmem_out. + * So this check is needed. + */ if (!size) size = bsize; @@ -192,7 +198,7 @@ int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer, unsigned int size, unsigned int cmd_index, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { - u32 written; + s32 written; if (!fw->chip) return -ENOENT; @@ -208,7 +214,7 @@ int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer, if (meson_sm_call(fw, cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0) return -EINVAL; - if (!written) + if (written <= 0 || written > size) return -EINVAL; return written; diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h index 95b0da2326a9..8eaf8922ab02 100644 --- a/include/linux/firmware/meson/meson_sm.h +++ b/include/linux/firmware/meson/meson_sm.h @@ -19,7 +19,7 @@ enum { struct meson_sm_firmware; int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index, - u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4); + s32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4); int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer, unsigned int b_size, unsigned int cmd_index, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4);