Shell dereferences invalid pointer when printing demo command help

[aescolar]

On a shell over uart, typing "demo\r" without any option causes a bad pointer de-refence:

==30417== at 0x804AD0C: _vprintk (printk.c:196)
==30417== by 0x804FFA3: shell_fprintf_fmt (shell_fprintf.c:38)
==30417== by 0x804F7BD: shell_fprintf (shell.c:1409)
==30417== by 0x804FADC: help_item_print (shell.c:1537)
==30417== by 0x804FB62: help_options_print (shell.c:1561)
==30417== by 0x804FEB8: shell_help_print (shell.c:1662)
==30417== by 0x804EECB: shell_execute (shell.c:1080)
==30417== by 0x804E87C: shell_state_collect (shell.c:770)
==30417== by 0x804F69F: shell_process (shell.c:1366)
==30417== by 0x804F3B5: shell_thread (shell.c:1232)
==30417== Address 0xa is not stack'd, malloc'd or (recently) free'd

[aescolar]

This is what it prints before the issue:

uart:~$ demo
demo - Demo commands
Options:

[carlescufi]

@aescolar thanks for reporting. I have assigned it to the authors.

[jakub-uC]

Hi,
It seems it is related to #10192, I have compiled my application with no debug options and it work correctly.
I will fix it.

[jakub-uC]

@aescolar : Do you still observe this problem?

[aescolar]

@jarz-nordic : Yes, you can just fetch and checkout https://github.com/aescolar/zephyr/tree/native_uart , and see it happen there. (That branch is on top of your last shell fix 73c2178) Take the master branch,

just do:

cd $ZEPHYR_BASE
mkdir build/ ; cd build/
cmake ../samples/subsys/shell/shell_module/  -GNinja -DBOARD=native_posix  -DCONFIG_NO_OPTIMIZATIONS=y -DCONFIG_NATIVE_UART_0_ON_STDINOUT=y
ninja
echo -e "demo\r" | valgrind zephyr/zephyr.exe

[aescolar]

This being the output:

build$ echo -e "demo\r" | valgrind zephyr/zephyr.exe 
==28775== Memcheck, a memory error detector
==28775== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==28775== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==28775== Command: zephyr/zephyr.exe
==28775== 


uart:~$ demo
demo - Demo commands
Options:
==28775== Thread 5:
==28775== Invalid read of size 1
==28775==    at 0x804AB3C: _vprintk (printk.c:196)
==28775==    by 0x804FB56: shell_fprintf_fmt (shell_fprintf.c:38)
==28775==    by 0x804F38F: shell_fprintf (shell.c:1408)
==28775==    by 0x804F6AE: help_item_print (shell.c:1536)
==28775==    by 0x804F734: help_options_print (shell.c:1560)
==28775==    by 0x804FA6B: shell_help_print (shell.c:1661)
==28775==    by 0x804EC31: shell_execute (shell.c:1078)
==28775==    by 0x804E608: shell_state_collect (shell.c:768)
==28775==    by 0x804F293: shell_process (shell.c:1365)
==28775==    by 0x804F097: shell_thread (shell.c:1231)
==28775==    by 0x8049EDA: _thread_entry (thread_entry.c:29)
==28775==    by 0x8052539: posix_thread_starter (posix_core.c:301)
==28775==  Address 0xa is not stack'd, malloc'd or (recently) free'd
==28775== 
==28775== 
==28775== Process terminating with default action of signal 11 (SIGSEGV)
==28775==  Access not within mapped region at address 0xA
==28775==    at 0x804AB3C: _vprintk (printk.c:196)
==28775==    by 0x804FB56: shell_fprintf_fmt (shell_fprintf.c:38)
==28775==    by 0x804F38F: shell_fprintf (shell.c:1408)
==28775==    by 0x804F6AE: help_item_print (shell.c:1536)
==28775==    by 0x804F734: help_options_print (shell.c:1560)
==28775==    by 0x804FA6B: shell_help_print (shell.c:1661)
==28775==    by 0x804EC31: shell_execute (shell.c:1078)
==28775==    by 0x804E608: shell_state_collect (shell.c:768)
==28775==    by 0x804F293: shell_process (shell.c:1365)
==28775==    by 0x804F097: shell_thread (shell.c:1231)
==28775==    by 0x8049EDA: _thread_entry (thread_entry.c:29)
==28775==    by 0x8052539: posix_thread_starter (posix_core.c:301)

[aescolar]

@jarz-nordic : Have you been able to reproduce it in native_posix?

[jakub-uC]

@aescolar : Not yet, today I was polishing my PR with shell documentation. I will take care tomorrow.

[jakub-uC]

@aescolar : I cannot find this branch: native_uart on your fork.
The website you are linkig is not available.

[aescolar]

@jarz-nordic That branch is already merged merged in master. So no need to fetch my branch anymore.

[jakub-uC]

@aescolar : I found a bug.
Root cause of the fault: Function _vprintk cannot parse *

In function help_item_print is following code:

	/* print option name */
	shell_fprintf(shell, SHELL_NORMAL, "%s%-*s%s:",
		      tabulator,
		      item_name_width, item_name,
		      tabulator);

If you compile for POSIX this "%s%-*s%s:" is parsed by _vprintk, otherwise it is parsed by _prf.
It seems that _prf is able to parse *.

So I think we need to extend _vprintk.

[jakub-uC]

To prove my words you can modify function help_item_print in shell.c

change code:

	/* print option name */
	shell_fprintf(shell, SHELL_NORMAL, "%s%-*s%s:",
		      tabulator,
		      item_name_width, item_name,
		      tabulator);

to:

	/* print option name */
	shell_fprintf(shell, SHELL_NORMAL, "%s%-s%s:",
		      tabulator,
		      item_name,
		      tabulator);

[aescolar]

There is no need to prove it :)
We just need either the shell to not use vprintk in a way it cannot handle, or fix vprintk to be able to handle the way it is being used.

[jakub-uC]

I will close this task and start new one to update _vprintk function.