[PATCH] xtensa: iss: bound command line construction in platform_setup()

[PATCH] xtensa: iss: bound command line construction in platform_setup()

[Pengpeng Hou]

platform_setup() concatenates simulator arguments into the fixed
COMMAND_LINE_SIZE cmdline buffer with raw strcat() appends.

The code only checks the size of the argv pointer block that simc_argv()
fills, not the final length of the concatenated command line string, so a
long enough argument list can write past the end of cmdline.

Build the command line with scnprintf() and stop once the fixed buffer is
full.

Fixes: b26d0ab0e6fa ("[XTENSA] Concentrate platforms into one platforms directory.")

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 arch/xtensa/platforms/iss/setup.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/xtensa/platforms/iss/setup.c b/arch/xtensa/platforms/iss/setup.c
index 0f1fe132691e..9bc7f21c9a0c 100644
--- a/arch/xtensa/platforms/iss/setup.c
+++ b/arch/xtensa/platforms/iss/setup.c
@@ -69,15 +69,21 @@ void __init platform_setup(char **p_cmdline)
 			pr_err("%s: command line too long: argv_size = %d\n",
 			       __func__, argv_size);
 		} else {
-			int i;
+			int i, len = 0;
 
 			cmdline[0] = 0;
 			simc_argv((void *)argv);
 
 			for (i = 1; i < argc; ++i) {
-				if (i > 1)
-					strcat(cmdline, " ");
-				strcat(cmdline, argv[i]);
+				len += scnprintf(cmdline + len,
+						 COMMAND_LINE_SIZE - len,
+						 "%s%s", i > 1 ? " " : "",
+						 argv[i]);
+				if (len >= COMMAND_LINE_SIZE - 1) {
+					pr_err("%s: command line too long\n",
+					       __func__);
+					break;
+				}
 			}
 			*p_cmdline = cmdline;
 		}
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] xtensa: iss: bound command line construction in platform_setup()

[Max Filippov]

On Fri, Apr 17, 2026 at 12:42 AM Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
>
> platform_setup() concatenates simulator arguments into the fixed
> COMMAND_LINE_SIZE cmdline buffer with raw strcat() appends.
>
> The code only checks the size of the argv pointer block that simc_argv()
> fills, not the final length of the concatenated command line string, so a
> long enough argument list can write past the end of cmdline.

This cannot happen, because simc_argv_size() returns total size of the
args block. For each argument it includes an argv pointer (4 bytes), the
corresponding string and its null-terminator character). That means
that the total size of all argv strings with spaces between them and a null
terminator is less than the value returned by the simc_argv_size().

> Build the command line with scnprintf() and stop once the fixed buffer is
> full.
>
> Fixes: b26d0ab0e6fa ("[XTENSA] Concentrate platforms into one platforms directory.")
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  arch/xtensa/platforms/iss/setup.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/arch/xtensa/platforms/iss/setup.c b/arch/xtensa/platforms/iss/setup.c
> index 0f1fe132691e..9bc7f21c9a0c 100644
> --- a/arch/xtensa/platforms/iss/setup.c
> +++ b/arch/xtensa/platforms/iss/setup.c
> @@ -69,15 +69,21 @@ void __init platform_setup(char **p_cmdline)
>                         pr_err("%s: command line too long: argv_size = %d\n",
>                                __func__, argv_size);
>                 } else {
> -                       int i;
> +                       int i, len = 0;
>
>                         cmdline[0] = 0;
>                         simc_argv((void *)argv);
>
>                         for (i = 1; i < argc; ++i) {
> -                               if (i > 1)
> -                                       strcat(cmdline, " ");
> -                               strcat(cmdline, argv[i]);
> +                               len += scnprintf(cmdline + len,
> +                                                COMMAND_LINE_SIZE - len,
> +                                                "%s%s", i > 1 ? " " : "",
> +                                                argv[i]);
> +                               if (len >= COMMAND_LINE_SIZE - 1) {
> +                                       pr_err("%s: command line too long\n",
> +                                              __func__);
> +                                       break;
> +                               }
>                         }
>                         *p_cmdline = cmdline;

This assignment would still happen even in case an overflow's
been detected.

>                 }
> --
> 2.50.1 (Apple Git-155)
>

-- 
Thanks.
-- Max

Re: [PATCH] xtensa: iss: bound command line construction in platform_setup()

[kernel test robot]

Hi Pengpeng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jcmvbkbc-xtensa/xtensa-for-next]
[also build test WARNING on linus/master v7.0 next-20260417]
[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/Pengpeng-Hou/xtensa-iss-bound-command-line-construction-in-platform_setup/20260417-224100
base:   https://github.com/jcmvbkbc/linux-xtensa xtensa-for-next
patch link:    https://lore.kernel.org/r/20260417074226.9295-1-pengpeng%40iscas.ac.cn
patch subject: [PATCH] xtensa: iss: bound command line construction in platform_setup()
config: xtensa-allnoconfig (https://download.01.org/0day-ci/archive/20260419/202604190738.DDzpg4Ro-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260419/202604190738.DDzpg4Ro-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/202604190738.DDzpg4Ro-lkp@intel.com/

All warnings (new ones prefixed by >>):

   arch/xtensa/platforms/iss/setup.c: In function 'platform_setup':
>> arch/xtensa/platforms/iss/setup.c:75:54: warning: format '%s' expects argument of type 'char *', but argument 5 has type 'void *' [-Wformat=]
      75 |                                                  "%s%s", i > 1 ? " " : "",
         |                                                     ~^
         |                                                      |
         |                                                      char *
         |                                                     %p
      76 |                                                  argv[i]);
         |                                                  ~~~~~~~
         |                                                      |
         |                                                      void *


vim +75 arch/xtensa/platforms/iss/setup.c

    54	
    55	void __init platform_setup(char **p_cmdline)
    56	{
    57		static void *argv[COMMAND_LINE_SIZE / sizeof(void *)] __initdata;
    58		static char cmdline[COMMAND_LINE_SIZE] __initdata;
    59		int argc = simc_argc();
    60		int argv_size = simc_argv_size();
    61	
    62		if (argc > 1) {
    63			if (argv_size > sizeof(argv)) {
    64				pr_err("%s: command line too long: argv_size = %d\n",
    65				       __func__, argv_size);
    66			} else {
    67				int i, len = 0;
    68	
    69				cmdline[0] = 0;
    70				simc_argv((void *)argv);
    71	
    72				for (i = 1; i < argc; ++i) {
    73					len += scnprintf(cmdline + len,
    74							 COMMAND_LINE_SIZE - len,
  > 75							 "%s%s", i > 1 ? " " : "",

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] xtensa: iss: bound command line construction in platform_setup()

[Pengpeng Hou]

Hi Max,

Thanks for the explanation.

Agreed, I missed that simc_argv_size() already accounts for the
argument strings and their NUL terminators in the simulator argument
block, so the claimed overflow path is not valid. The proposed change
also had the bad truncation path you pointed out.

I'll drop this patch.

Thanks,
Pengpeng