zephyr: shell.c does not provide mechanism to avoid data race

As per the discussion, shell.c does not currently provide a mechanisms to synchronize access to the prompt-string. Since the user is providing said string and the shell simply points to the users data directly, there is no way to avoid a data-race, when the shell-thread is reading and the user is writing to the prompt.

The problematic line: https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/shell/shell.c#L1613

Suggestion would be to instead copy the prompt-string into a buffer private to the shell. For this operation the appropriate synchronization with the shell-thread can be done internally. This is also a more ituitiv api, since the prompt-buffer can live on the stack without invalidating any pointer in shell.c.

If instead the function is intended for use with string literals only, it would be desirable, that the documentation was clear on this and that for example a macro is provided, that asserts at compile-time, that only a string literal was used.

For example code like this

static char prompt[256];
strcpy(prompt, "Hello");
shell_prompt_change(sh, prompt);

is currently a data race, but seems perfectly fine. It boils down to something like:

char *a;
void shell_thread(void) {
    while(1) {
        // reads a
        if (a != NULL) {
            printf("%s\n", a);
        }
    }
}

void caller_thread(void) {
    static char prompt[256];
    strcpy(prompt, "Hello");
    a = prompt; // equivalent of shell_prompt_change(sh, prompt);
}

I wanted to share my idea on this but I am not 100% sure, so please correct me, if I am wrong.

About this issue

  • Original URL
  • State: closed
  • Created 8 months ago
  • Comments: 16 (5 by maintainers)

Commits related to this issue

Most upvoted comments

Thank you for finding it. I will take care of this next week.