ricardgb commented on code in PR #3658:
URL: https://github.com/apache/nuttx-apps/pull/3658#discussion_r3642972778
##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -125,6 +125,27 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
goto errout;
}
+ /* If the daemon was started without standard streams (e.g. spawned by
+ * nsh_telnetstart() before a USB console exists), socket() may have
+ * returned a descriptor in 0..2. The "go silent" close(0)..close(2)
Review Comment:
Something like this?
```diff
diff --git a/netutils/telnetd/telnetd_daemon.c
b/netutils/telnetd/telnetd_daemon.c
index 3662fa4aa..e0a7df5be 100644
--- a/netutils/telnetd/telnetd_daemon.c
+++ b/netutils/telnetd/telnetd_daemon.c
@@ -81,6 +81,7 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
socklen_t addrlen;
int listensd;
int acceptsd;
+ int nullfd;
#ifdef CONFIG_NET_SOCKOPTS
int optval;
#endif
@@ -115,6 +116,31 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
}
#endif /* CONFIG_SCHED_HAVE_PARENT && CONFIG_ENABLE_ALL_SIGNALS */
+ /* If the daemon was started without standard streams (e.g. spawned by
+ * nsh_telnetstart() before a USB console exists), descriptors 0..2 are
+ * free and any descriptor created below -- the listen socket, an
+ * accepted socket or a session driver -- could be allocated in the
+ * standard-stream range, where the per-session dup2(0..2) would clobber
+ * it. Anchor any free slot in 0..2 to /dev/null first so that can
+ * never happen.
+ */
+
+ for (; ; )
+ {
+ nullfd = open("/dev/null", O_RDWR);
+ if (nullfd < 0)
+ {
+ nerr("ERROR: open(/dev/null) failed: %d\n", errno);
+ goto errout;
+ }
+
+ if (nullfd > 2)
+ {
+ close(nullfd);
+ break;
+ }
+ }
+
/* Create a new TCP socket to use to listen for connections */
listensd = socket(config->d_family, SOCK_STREAM | SOCK_CLOEXEC, 0);
@@ -125,27 +151,6 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
goto errout;
}
- /* If the daemon was started without standard streams (e.g. spawned by
- * nsh_telnetstart() before a USB console exists), socket() may have
- * returned a descriptor in 0..2. The "go silent" close(0)..close(2)
- * at the top of the accept loop below would then destroy the listen
- * socket: every subsequent accept4() fails and the daemon serves
- * nothing. Move the descriptor above the standard-stream range.
- */
-
- if (listensd <= 2)
- {
- int highsd = fcntl(listensd, F_DUPFD_CLOEXEC, 3);
- if (highsd < 0)
- {
- nerr("ERROR: F_DUPFD_CLOEXEC failed: %d\n", errno);
- goto errout_with_socket;
- }
-
- close(listensd);
- listensd = highsd;
- }
-
#ifdef CONFIG_NET_SOCKOPTS
/* Set socket to reuse address */
@@ -211,11 +216,24 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
#endif
int drvrfd;
- /* Now go silent. */
+ /* Now go silent: detach from the previous session's driver (or the
+ * startup streams) by re-anchoring 0..2 to /dev/null instead of
+ * closing them. Closing would leave 0..2 free, and accept4() or
+ * open() below could then allocate into the standard-stream range
+ * where the dup2(0..2) for the next session would clobber it.
+ */
- close(0);
- close(1);
- close(2);
+ nullfd = open("/dev/null", O_RDWR);
+ if (nullfd >= 0)
+ {
+ dup2(nullfd, 0);
+ dup2(nullfd, 1);
+ dup2(nullfd, 2);
+ if (nullfd > 2)
+ {
+ close(nullfd);
+ }
+ }
ninfo("Accepting connections on port %d\n", ntohs(config->d_port));
```
This removes the listen-socket relocation entirely and instead (1) anchors
any free slot in 0..2 to `/dev/null` before the socket is created, and (2)
re-anchors 0..2 to `/dev/null` at the top of the accept loop instead of closing
them — so no descriptor (listen, accept or driver) can ever be allocated into
the standard-stream range in the first place. Compile-tested (rp2350 telnet
config, builds clean, nxstyle clean); I'd re-run the on-hardware test before
pushing it. If you prefer this shape I'll update the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
