remoteproc: mtk_scp: remove unnecessary checking
Subsystems (3)
PATCH 1/1 remoteproc: mtk_scp: remove unnecessary checking
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.
--- drivers/remoteproc/mtk_scp.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/remoteproc/mtk_scp.c +2 -4
@@ -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);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," )];
Signature · 1 line
2.51.0
kernel test robot
build botkernel test robot noticed the following build warnings:
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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Quoted 15 lines · Dan Carpenter
The kernel implementation of
snprintf()cannot return negative error codes. Also these particular calls tosnprintf()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 <[email protected]>--- drivers/remoteproc/mtk_scp.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/remoteproc/mtk_scp.c +0
-0
drivers/remoteproc/mtk_scp.c static const char *scp_get_default_fw_path(struct device *dev, int core_id) · line 1130
Quoted 5 lines
drivers/remoteproc/mtk_scp.c +1
-1@@ -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~
Quoted 8 lines
drivers/remoteproc/mtk_scp.c +1
-3else - 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," )];
Signature · 2 lines
Thx and BRs, Zhongqiu Han