michaelh/zephyr
1
0
Fork 0
Code Releases Activity

shell: fix unsafe API calls and add configurable autoflush behavior

Browse source 
Fixes an issue where the shell API could block indefinitely when called
from threads other than the shell's processing thread, especially when
the transport (e.g. USB CDC ACM) was unavailable or inactive.

Replaced `k_mutex_lock` calls with an indefinite timeout (`K_FOREVER`)
by using a fixed timeout (`K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)`) in shell
API functions to prevent indefinite blocking.

Link: https://github.com/zephyrproject-rtos/zephyr/issues/84274

Signed-off-by: Jakub Rzeszutko <jakub.rzeszutko@verkada.com>
This commit is contained in:
Jakub Rzeszutko 2025-01-21 12:36:52 +01:00 • committed by Benjamin Cabé
commit b0a0febe58
1 changed files with 19 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

26
subsys/shell/shell.c
View file

@ -31,6 +31,7 @@
"WARNING: A print request was detected on not active shell backend.\n" "WARNING: A print request was detected on not active shell backend.\n"
#define SHELL_MSG_TOO_MANY_ARGS "Too many arguments in the command.\n" #define SHELL_MSG_TOO_MANY_ARGS "Too many arguments in the command.\n"
#define SHELL_INIT_OPTION_PRINTER (NULL) #define SHELL_INIT_OPTION_PRINTER (NULL)
#define SHELL_TX_MTX_TIMEOUT_MS 50
#define SHELL_THREAD_PRIORITY \ #define SHELL_THREAD_PRIORITY \
COND_CODE_1(CONFIG_SHELL_THREAD_PRIORITY_OVERRIDE, \ COND_CODE_1(CONFIG_SHELL_THREAD_PRIORITY_OVERRIDE, \
@ -1350,7 +1351,9 @@ void shell_thread(void *shell_handle, void *arg_log_backend,
K_FOREVER); K_FOREVER);
if (err != 0) { if (err != 0) {
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER); if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
return;
}
z_shell_fprintf(sh, SHELL_ERROR, z_shell_fprintf(sh, SHELL_ERROR,
"Shell thread error: %d", err); "Shell thread error: %d", err);
k_mutex_unlock(&sh->ctx->wr_mtx); k_mutex_unlock(&sh->ctx->wr_mtx);
@ -1441,7 +1444,9 @@ int shell_start(const struct shell *sh)
z_shell_log_backend_enable(sh->log_backend, (void *)sh, sh->ctx->log_level); z_shell_log_backend_enable(sh->log_backend, (void *)sh, sh->ctx->log_level);
} }
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER); if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
return -EBUSY;
}
if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS)) { if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS)) {
z_shell_vt100_color_set(sh, SHELL_NORMAL); z_shell_vt100_color_set(sh, SHELL_NORMAL);
@ -1543,7 +1548,10 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
return; return;
} }
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER); if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
return;
}
if (!z_flag_cmd_ctx_get(sh) && !sh->ctx->bypass && z_flag_use_vt100_get(sh)) { if (!z_flag_cmd_ctx_get(sh) && !sh->ctx->bypass && z_flag_use_vt100_get(sh)) {
z_shell_cmd_line_erase(sh); z_shell_cmd_line_erase(sh);
} }
@ -1552,6 +1560,7 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
z_shell_print_prompt_and_cmd(sh); z_shell_print_prompt_and_cmd(sh);
} }
z_transport_buffer_flush(sh); z_transport_buffer_flush(sh);
k_mutex_unlock(&sh->ctx->wr_mtx); k_mutex_unlock(&sh->ctx->wr_mtx);
} }
@ -1677,10 +1686,9 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
return -EINVAL; return -EINVAL;
} }
static const size_t mtx_timeout_ms = 20;
size_t prompt_length = z_shell_strlen(prompt); size_t prompt_length = z_shell_strlen(prompt);
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(mtx_timeout_ms))) { if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
return -EBUSY; return -EBUSY;
} }
@ -1703,7 +1711,9 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
void shell_help(const struct shell *sh) void shell_help(const struct shell *sh)
{ {
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER); if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
return;
}
shell_internal_help_print(sh); shell_internal_help_print(sh);
k_mutex_unlock(&sh->ctx->wr_mtx); k_mutex_unlock(&sh->ctx->wr_mtx);
} }
@ -1736,7 +1746,9 @@ int shell_execute_cmd(const struct shell *sh, const char *cmd)
sh->ctx->cmd_buff_len = cmd_len; sh->ctx->cmd_buff_len = cmd_len;
sh->ctx->cmd_buff_pos = cmd_len; sh->ctx->cmd_buff_pos = cmd_len;
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER); if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
return -ENOEXEC;
}
ret_val = execute(sh); ret_val = execute(sh);
k_mutex_unlock(&sh->ctx->wr_mtx); k_mutex_unlock(&sh->ctx->wr_mtx);