Commit 9cc6307f authored by Jakub Rzeszutko's avatar Jakub Rzeszutko Committed by Anas Nashif
Browse files

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: default avatarJakub Rzeszutko <jakub.rzeszutko@verkada.com>
(cherry picked from commit b0a0febe)
parent 2f93ce6c
Loading
Loading
Loading
Loading
+19 −7
Changes for subsys/shell/shell.c: 19 added lines, 7 removed lines.
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@
	"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_INIT_OPTION_PRINTER	(NULL)
#define SHELL_TX_MTX_TIMEOUT_MS		50

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

		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,
					"Shell thread error: %d", err);
			k_mutex_unlock(&sh->ctx->wr_mtx);
@@ -1440,7 +1443,9 @@ int shell_start(const struct shell *sh)
		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)) {
		z_shell_vt100_color_set(sh, SHELL_NORMAL);
@@ -1533,7 +1538,10 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
		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)) {
		z_shell_cmd_line_erase(sh);
	}
@@ -1542,6 +1550,7 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
		z_shell_print_prompt_and_cmd(sh);
	}
	z_transport_buffer_flush(sh);

	k_mutex_unlock(&sh->ctx->wr_mtx);
}

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

	static const size_t mtx_timeout_ms = 20;
	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;
	}

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

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);
	k_mutex_unlock(&sh->ctx->wr_mtx);
}
@@ -1685,7 +1695,9 @@ int shell_execute_cmd(const struct shell *sh, const char *cmd)
	sh->ctx->cmd_buff_len = 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);
	k_mutex_unlock(&sh->ctx->wr_mtx);