* Re: [PATCH] remoteproc: mtk_scp: remove unnecessary checking
2025-10-22 11:05 [PATCH] remoteproc: mtk_scp: remove unnecessary checking Dan Carpenter
@ 2025-10-23 9:58 ` kernel test robot
2025-10-23 10:10 ` Zhongqiu Han
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2025-10-23 9:58 UTC (permalink / raw)
To: Dan Carpenter, Bjorn Andersson
Cc: oe-kbuild-all, Mathieu Poirier, Matthias Brugger,
AngeloGioacchino Del Regno, linux-remoteproc, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel-janitors
Hi Dan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on remoteproc/rproc-next]
[also build test WARNING on next-20251023]
[cannot apply to linus/master v6.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dan-Carpenter/remoteproc-mtk_scp-remove-unnecessary-checking/20251022-200619
base: https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rproc-next
patch link: https://lore.kernel.org/r/aPi6eBlFLH43A4C0%40stanley.mountain
patch subject: [PATCH] remoteproc: mtk_scp: remove unnecessary checking
config: arm-randconfig-003-20251023 (https://download.01.org/0day-ci/archive/20251023/202510231714.dlrsh9El-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251023/202510231714.dlrsh9El-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510231714.dlrsh9El-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/remoteproc/mtk_scp.c: In function 'scp_rproc_init':
>> drivers/remoteproc/mtk_scp.c:1130:56: warning: '%1d' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=]
snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp_c%1d", core_id);
^~~
drivers/remoteproc/mtk_scp.c:1130:50: note: directive argument in the range [0, 1073741824]
snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp_c%1d", core_id);
^~~~~~~~~~
drivers/remoteproc/mtk_scp.c:1130:3: note: 'snprintf' output between 7 and 16 bytes into a destination of size 7
snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp_c%1d", core_id);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +1130 drivers/remoteproc/mtk_scp.c
1096
1097 /**
1098 * scp_get_default_fw_path() - Get default SCP firmware path
1099 * @dev: SCP Device
1100 * @core_id: SCP Core number
1101 *
1102 * This function generates a path based on the following format:
1103 * mediatek/(soc_model)/scp(_cX).img; for multi-core or
1104 * mediatek/(soc_model)/scp.img for single core SCP HW
1105 *
1106 * Return: A devm allocated string containing the full path to
1107 * a SCP firmware or an error pointer
1108 */
1109 static const char *scp_get_default_fw_path(struct device *dev, int core_id)
1110 {
1111 struct device_node *np = core_id < 0 ? dev->of_node : dev->parent->of_node;
1112 const char *compatible, *soc;
1113 char scp_fw_file[7];
1114 int ret;
1115
1116 /* Use only the first compatible string */
1117 ret = of_property_read_string_index(np, "compatible", 0, &compatible);
1118 if (ret)
1119 return ERR_PTR(ret);
1120
1121 /* If the compatible string's length is implausible bail out early */
1122 if (strlen(compatible) < strlen("mediatek,mtXXXX-scp"))
1123 return ERR_PTR(-EINVAL);
1124
1125 /* If the compatible string starts with "mediatek,mt" assume that it's ok */
1126 if (!str_has_prefix(compatible, "mediatek,mt"))
1127 return ERR_PTR(-EINVAL);
1128
1129 if (core_id >= 0)
> 1130 snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp_c%1d", core_id);
1131 else
1132 snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp");
1133
1134 /* Not using strchr here, as strlen of a const gets optimized by compiler */
1135 soc = &compatible[strlen("mediatek,")];
1136
1137 return devm_kasprintf(dev, GFP_KERNEL, "mediatek/%.*s/%s.img",
1138 (int)strlen("mtXXXX"), soc, scp_fw_file);
1139 }
1140
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] remoteproc: mtk_scp: remove unnecessary checking
2025-10-22 11:05 [PATCH] remoteproc: mtk_scp: remove unnecessary checking Dan Carpenter
2025-10-23 9:58 ` kernel test robot
@ 2025-10-23 10:10 ` Zhongqiu Han
1 sibling, 0 replies; 3+ messages in thread
From: Zhongqiu Han @ 2025-10-23 10:10 UTC (permalink / raw)
To: Dan Carpenter, Bjorn Andersson
Cc: Mathieu Poirier, Matthias Brugger, AngeloGioacchino Del Regno,
linux-remoteproc, linux-kernel, linux-arm-kernel, linux-mediatek,
kernel-janitors, zhongqiu.han
On 10/22/2025 7:05 PM, Dan Carpenter wrote:
> The kernel implementation of snprintf() cannot return negative error
> codes. Also these particular calls to snprintf() can't return zero
> and the code to handle a zero return is sort of questionable. Just
> delete this impossible code.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/remoteproc/mtk_scp.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index 10e3f9eb8cd2..9b624948a572 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1127,11 +1127,9 @@ static const char *scp_get_default_fw_path(struct device *dev, int core_id)
> return ERR_PTR(-EINVAL);
>
> if (core_id >= 0)
> - ret = snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp_c%1d", core_id);
> + snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp_c%1d", core_id);
Hello Dan Carpenter,
The patch looks fine to me functionally. However, one concern beyond the
current scope: if core_id >= 10 in future extensions, the
snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp_c%1d", core_id) may
cause truncation.
scp_add_multi_core
|
v
scp_rproc_init
|
v
scp_get_default_fw_path
char scp_fw_file[7];
To guard against this, maybe should we consider adding:
if (ret >= ARRAY_SIZE(scp_fw_file))
return ERR_PTR(-ENAMETOOLONG);
or just expand the scp_fw_file[7] array?
Thank you~
> else
> - ret = snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp");
> - if (ret <= 0)
> - return ERR_PTR(ret);
> + snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp");
>
> /* Not using strchr here, as strlen of a const gets optimized by compiler */
> soc = &compatible[strlen("mediatek,")];
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply [flat|nested] 3+ messages in thread