netutils/telnetd: fix silent listen-socket loss and per-connection daemon death#3658
Open
ricardgb wants to merge 2 commits into
Open
netutils/telnetd: fix silent listen-socket loss and per-connection daemon death#3658ricardgb wants to merge 2 commits into
ricardgb wants to merge 2 commits into
Conversation
…ange When telnetd_daemon() starts without open standard streams - exactly what happens when nsh_telnetstart() spawns "telnetd &" before the console device exists (e.g. CONFIG_NSH_USBCONSOLE boards, where nsh_initialize() runs before the USB console is connected) - socket() returns a descriptor in the 0..2 range. The accept loop's own "go silent" close(0)..close(2) then destroys the listen socket on the first iteration: every subsequent accept4() fails with EBADF and the daemon serves nothing. Observed on hardware (RP2350, CDC-ACM console + CDC-NCM composite): the daemon task was alive but port 23 refused every connection; the thread list showed it looping on the failed accept. Duplicate the descriptor above the standard-stream range before using it. Assisted-by: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Ricard Rosson <ricard@groundbits.com>
Any single failed connection killed the whole daemon: - A peer that resets before the accept completes (a port scanner, or a simple "nc -z" probe) surfaces as an accept4() error such as ECONNABORTED, which took the errout path and closed the listener. - The per-connection error paths after a successful accept (setsockopt, opening /dev/telnet, SIOCTELNET, opening the session device, spawning the session) likewise exited the daemon instead of dropping the one connection. Either way, one bad or aborted connection and the telnet console is dead until reboot - a trivial remote way to take out the NuttX console on any reachable network. Verified on hardware: one "nc -zv" probe permanently killed the listener. Treat these as per-connection failures: drop the connection and keep accepting. Genuinely unrecoverable accept4() errors (EBADF, ENOTSOCK, EINVAL, EOPNOTSUPP - a bad listen socket) still exit loudly, and a short pause on repeated transient failures avoids busy-spinning while an interface is down. Daemon setup errors (socket/bind/listen) exit as before. Assisted-by: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Ricard Rosson <ricard@groundbits.com>
acassis
requested review from
acassis,
cederom,
jerpelea,
linguini1,
simbit18 and
xiaoxiang781216
July 24, 2026 01:46
acassis
approved these changes
Jul 24, 2026
xiaoxiang781216
approved these changes
Jul 24, 2026
| { | ||
| nerr("ERROR: accept failed: %d\n", errno); | ||
| goto errout_with_socket; | ||
| usleep(100 * 1000); |
| */ | ||
|
|
||
| if (err == EBADF || err == ENOTSOCK || err == EINVAL || | ||
| err == EOPNOTSUPP) |
Contributor
There was a problem hiding this comment.
why not ignore the error and continue directly
| * destroy the listen socket: every subsequent accept4() fails and the | ||
| * daemon serves nothing. Move the descriptor above the | ||
| * standard-stream range. | ||
| * nsh_telnetstart() before a USB console exists), socket() may have |
Contributor
There was a problem hiding this comment.
move to previous patch if you really want to modify
Author
There was a problem hiding this comment.
Thanks @xiaoxiang781216 . Good questions on the accept-loop error handling —
I'd like to hold this one briefly before I revise it. I'm coordinating a
related change with the maintainers and want the daemon's error handling to
line up with it rather than reworking this twice. I'll follow up here with the
updates (the fatal-vs-transient split, the sleep, and folding the comment
change into the earlier commit) once that's settled. Appreciate the review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two robustness fixes for the telnet daemon (
netutils/telnetd/telnetd_daemon.c). As it stands, the standard NuttX telnet console can be disabled by a single aborted connection, and on some configurations it silently serves nothing from boot.1. Keep the listen socket out of the standard-stream range.
When the daemon starts without open standard streams — exactly what happens when
nsh_telnetstart()spawnstelnetd &before the console device exists (e.g.CONFIG_NSH_USBCONSOLEboards, wherensh_initialize()runs before the USB console is connected) —socket()returns a descriptor in 0..2. The accept loop's own "go silent"close(0)..close(2)then destroys the listen socket on the first iteration; every subsequentaccept4()fails and the daemon serves nothing, while appearing alive in the task list. Fix:F_DUPFD_CLOEXECthe descriptor above 2 before use.2. Survive transient connection errors instead of exiting.
Previously any single failed connection killed the whole daemon:
nc -zprobe) surfaces as anaccept4()error such asECONNABORTEDand took the errout path;/dev/telnetopen,SIOCTELNET, session-device open, session spawn) likewise exited the daemon.One bad or aborted connection and the telnet console is dead until reboot — a trivial remote way to take out the console of any reachable NuttX device. Fix: treat these as per-connection failures (drop the connection, keep accepting). Genuinely unrecoverable
accept4()errors (EBADF/ENOTSOCK/EINVAL/EOPNOTSUPP) still exit loudly, and a short pause on repeated transient failures avoids busy-spinning while an interface is down. Daemon setup errors (socket/bind/listen) exit as before.Impact
Any board using
netutils/telnetd(directly or viaCONFIG_NSH_TELNET). Bug 1 affects configurations where the daemon can start before its standard streams exist (USB-console boards being the common case). Bug 2 affects everyone: a subnetnmapsweep kills every reachable NuttX telnet console.Testing
Reproduced and verified on hardware: Raspberry Pi Pico 2 W (RP2350), composite USB CDC-ACM console + CDC-NCM network,
CONFIG_NSH_USBCONSOLE, telnetd auto-started viansh_telnetstart().close(0).nc -zv <addr> 23probe permanently killed a manually started, correctly listening daemon.nc -zprobe barrages, and interactive nsh telnet sessions work.tools/nxstyle.cpasses on the modified file.Note
These changes were developed with the assistance of an AI agent (Claude) and have been human-reviewed and tested on real hardware as described above.