Skip to content

Commit 4a1ffd8

Browse files
jakub-uCdkalowsk
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. Link: #84274 Signed-off-by: Jakub Rzeszutko <[email protected]> (cherry picked from commit b0a0feb)
1 parent 98a1a15 commit 4a1ffd8

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, \
@@ -1350,7 +1351,9 @@ void shell_thread(void *shell_handle, void *arg_log_backend,
13501351
K_FOREVER);
13511352

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

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

14461451
if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS)) {
14471452
z_shell_vt100_color_set(sh, SHELL_NORMAL);
@@ -1543,7 +1548,10 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
15431548
return;
15441549
}
15451550

1546-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1551+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1552+
return;
1553+
}
1554+
15471555
if (!z_flag_cmd_ctx_get(sh) && !sh->ctx->bypass && z_flag_use_vt100_get(sh)) {
15481556
z_shell_cmd_line_erase(sh);
15491557
}
@@ -1552,6 +1560,7 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
15521560
z_shell_print_prompt_and_cmd(sh);
15531561
}
15541562
z_transport_buffer_flush(sh);
1563+
15551564
k_mutex_unlock(&sh->ctx->wr_mtx);
15561565
}
15571566

@@ -1677,10 +1686,9 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
16771686
return -EINVAL;
16781687
}
16791688

1680-
static const size_t mtx_timeout_ms = 20;
16811689
size_t prompt_length = z_shell_strlen(prompt);
16821690

1683-
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(mtx_timeout_ms))) {
1691+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
16841692
return -EBUSY;
16851693
}
16861694

@@ -1703,7 +1711,9 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
17031711

17041712
void shell_help(const struct shell *sh)
17051713
{
1706-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1714+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1715+
return;
1716+
}
17071717
shell_internal_help_print(sh);
17081718
k_mutex_unlock(&sh->ctx->wr_mtx);
17091719
}
@@ -1736,7 +1746,9 @@ int shell_execute_cmd(const struct shell *sh, const char *cmd)
17361746
sh->ctx->cmd_buff_len = cmd_len;
17371747
sh->ctx->cmd_buff_pos = cmd_len;
17381748

1739-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1749+
if (k_mutex_lock(&sh->ctx->wr_mtx, K_MSEC(SHELL_TX_MTX_TIMEOUT_MS)) != 0) {
1750+
return -ENOEXEC;
1751+
}
17401752
ret_val = execute(sh);
17411753
k_mutex_unlock(&sh->ctx->wr_mtx);
17421754

0 commit comments

Comments
 (0)