Telnet shell asserts if connection is closed

[jukkar]

Describe the bug

Some extra discussion about the issue can be found in PR #67596, basically telnet shell asserts if telnet connection is closed.
If asserts are disabled no visible error was seen, but according to the code in

zephyr/subsys/shell/shell_ops.c

Line 425 in 11f7fa2

void z_shell_write(const struct shell *sh, const void *data,

the internal variables in the z_shell_write() could contain invalid values which could corrupt internal state of the shell.

To Reproduce
Setup the network by executing tools/net-tools/net-setup.sh that is found in net-tools zephyr project.

west build -p -b native_sim samples/net/telnet/ -d ../build/telnet \
  -t run -- -DCONFIG_ASSERT=y -DCONFIG_NATIVE_UART_0_ON_STDINOUT=y \
  -DCONFIG_LOG_BACKEND_UART=y -DCONFIG_SHELL_LOG_BACKEND=n

and then execute echo x | telnet 192.0.2.1 from the host.

Expected behavior
The telnet connection is closed without any errors / crashes.

Impact
If the asserts are enabled, the crashing of the system is not necessary as we just closed the connection.
If the asserts are disabled, the internal state of the shell might cause corruption.

Logs and console output

*** Booting Zephyr OS build zephyr-v3.5.0-4132-gf37d150c1052 ***

uart:~$ ASSERTION FAIL [err == 0] @ WEST_TOPDIR/zephyr/subsys/shell/shell_ops.c:438
@ WEST_TOPDIR/zephyr/lib/os/assert.c:43
Exiting due to fatal error

Environment (please complete the following information):

• OS: Linux
• Commit SHA or Version used: zephyr-v3.5.0-4132-gf37d150c1052

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[kevinior]

This issue is still present in the rewritten telnet server in 3.7.0:
ASSERTION FAIL [err == 0] @ WEST_TOPDIR/zephyr/subsys/shell/shell_ops.c:438

As a result we're having to maintain a patched version of the telnet server that returns 0 when the connection closes, to avoid crashing our whole application.

[jukkar]

@kevinior if you have a patch for this, could you submit a PR for it?

[kevinior]

@kevinior if you have a patch for this, could you submit a PR for it?

We just patched the telnet_write function to always return 0 and log some diagnostics about errors. I don't know if that would be an acceptable fix for others.

[jukkar]

We just patched the telnet_write function to always return 0 and log some diagnostics about errors. I don't know if that would be an acceptable fix for others.

Yeah, that sounds a bit hackish to me. But you could try to send it still, the review could give ideas how to solve it better.

[kevinior]

@jukkar OK, I've created a PR for the hackish fix.

[kevinior]

@jukkar No responses to the PR so far