Skip to content

Commit 9cc6307

Browse files
jakub-uCnashif
authored andcommitted
shell: fix unsafe API calls and add configurable autoflush behavior
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. Signed-off-by: Jakub Rzeszutko <[email protected]> (cherry picked from commit b0a0feb)
1 parent 2f93ce6 commit 9cc6307

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

subsys/shell/shell.c

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"WARNING: A print request was detected on not active shell backend.\n"
3232
#define SHELL_MSG_TOO_MANY_ARGS "Too many arguments in the command.\n"
3333
#define SHELL_INIT_OPTION_PRINTER (NULL)
34+
#define SHELL_TX_MTX_TIMEOUT_MS 50
3435

3536
#define SHELL_THREAD_PRIORITY \
3637
COND_CODE_1(CONFIG_SHELL_THREAD_PRIORITY_OVERRIDE, \
@@ -1349,7 +1350,9 @@ void shell_thread(void *shell_handle, void *arg_log_backend,
13491350
K_FOREVER);
13501351

13511352
if (err != 0) {
1352-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1353+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1354+
return;
1355+
}
13531356
z_shell_fprintf(sh, SHELL_ERROR,
13541357
"Shell thread error: %d", err);
13551358
k_mutex_unlock(&sh->ctx->wr_mtx);
@@ -1440,7 +1443,9 @@ int shell_start(const struct shell *sh)
14401443
z_shell_log_backend_enable(sh->log_backend, (void *)sh, sh->ctx->log_level);
14411444
}
14421445

1443-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1446+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1447+
return -EBUSY;
1448+
}
14441449

14451450
if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS)) {
14461451
z_shell_vt100_color_set(sh, SHELL_NORMAL);
@@ -1533,7 +1538,10 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
15331538
return;
15341539
}
15351540

1536-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1541+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1542+
return;
1543+
}
1544+
15371545
if (!z_flag_cmd_ctx_get(sh) && !sh->ctx->bypass && z_flag_use_vt100_get(sh)) {
15381546
z_shell_cmd_line_erase(sh);
15391547
}
@@ -1542,6 +1550,7 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
15421550
z_shell_print_prompt_and_cmd(sh);
15431551
}
15441552
z_transport_buffer_flush(sh);
1553+
15451554
k_mutex_unlock(&sh->ctx->wr_mtx);
15461555
}
15471556

@@ -1626,10 +1635,9 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
16261635
return -EINVAL;
16271636
}
16281637

1629-
static const size_t mtx_timeout_ms = 20;
16301638
size_t prompt_length = z_shell_strlen(prompt);
16311639

1632-
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(mtx_timeout_ms))) {
1640+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
16331641
return -EBUSY;
16341642
}
16351643

@@ -1652,7 +1660,9 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
16521660

16531661
void shell_help(const struct shell *sh)
16541662
{
1655-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1663+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1664+
return;
1665+
}
16561666
shell_internal_help_print(sh);
16571667
k_mutex_unlock(&sh->ctx->wr_mtx);
16581668
}
@@ -1685,7 +1695,9 @@ int shell_execute_cmd(const struct shell *sh, const char *cmd)
16851695
sh->ctx->cmd_buff_len = cmd_len;
16861696
sh->ctx->cmd_buff_pos = cmd_len;
16871697

1688-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1698+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1699+
return -ENOEXEC;
1700+
}
16891701
ret_val = execute(sh);
16901702
k_mutex_unlock(&sh->ctx->wr_mtx);
16911703

0 commit comments

Comments
 (0)