Re: [PATCH v2] lib/cmdline: prevent unintented access to address

[Andy Shevchenko]

On Thu, Aug 13, 2020 at 12:07:41PM +0900, Seungil Kang wrote:
> When args = "\"\0", "i" will be 0 and args[i-1] is used. (*lib/cmdline.c +238)

What I meant is to put hex dump of the args here in the parentheses, something like

"When args = "... \"\0" (... 0x22 0x00), 'i' will be..."

> Because of "i" is an unsigned int type, the function will access at args[0xFFFFFFFF].
> It can make a crash.

...

> > Can you point out to the code that calls this and leads to a crash?
> 
>  *lib/cmdlinc + 201 ~, next_arg function with args = "\"\0"

Not the next_arg() code :-) The code which calls here...

...

> > Can you provide a KUnit test module which can check the case?
> 
>  If necessary, I will make it and share it.

Please, do as a separate patch in the series.

...

> --- a/lib/cmdline.c
> +++ b/lib/cmdline.c
> @@ -200,7 +200,7 @@ bool parse_option_str(const char *str, const char *option)
>   */
>  char *next_arg(char *args, char **param, char **val)
>  {
> -	unsigned int i, equals = 0;
> +	int i, equals = 0;
>  	int in_quote = 0, quoted = 0;
>  	char *next;

At the first glance this is not correct fix for it: 0 - 1 is always 'all 1:s'
independently on signedness, but I need to think about.

And your test case / module would help a lot, if present.

-- 
With Best Regards,
Andy Shevchenko