From 8d6571066d7b6287902db3e27b9753ddc87aae26 Mon Sep 17 00:00:00 2001 From: "Darryl L. Miles" Date: Mon, 24 Feb 2025 08:44:09 +0000 Subject: [PATCH] grMain.c select() usage fix and protect from higher numbered fd's The old code would only work is the fileno(stream) returned an fd in the range 0 <= 19. It would silently fail, if the fd was in the range 20..1023 because FD_SET() would work and syscall select() would be limited to only look at the first 20 fd's. Ignoring any fd's higher even if set. This would theoretically cause high CPU usage due to select() never blocking because there are no active fd's in the fd_set as far as the kernel interprets the request and the kernel would immediately return. But reading the code the 1st argument to select() seems self limiting for no good reason. It should be fileno(stream)+1 as documented in man select(2). Added the assertion as well, because we are trying to allow magic to use fd's beyond the standard environmental limits. So it becomes an assertion condition if the fd is outside the range 0..1023 because the FD_SET() macro will not operate correctly / undefined-behaviour. I can't find any user of this func in the codebase right now. If you look at sim/SimRsim.c and the use of select() there, it is correctly using select() to wait on a single fd over there. This commit changes this code to match this correct usage. --- graphics/grMain.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/graphics/grMain.c b/graphics/grMain.c index 9d565e3e..2e465297 100644 --- a/graphics/grMain.c +++ b/graphics/grMain.c @@ -457,8 +457,15 @@ grFgets(str, n, stream, name) twentySecs.tv_sec = 20; twentySecs.tv_usec = 0; + const int fd = fileno(stream); + ASSERT(fd >= 0 && fd < FD_SETSIZE, "fd>=0&&fd= FD_SETSIZE) + { + TxError("WARNING: grFgets(fd=%d) called with fd out of range 0..%d\n", fd, FD_SETSIZE-1); + return NULL; /* allowing things to continue is UB */ + } FD_ZERO(&fn); - FD_SET(fileno(stream), &fn); + FD_SET(fd, &fn); newstr = str; n--; if (n < 0) return (char *) NULL; @@ -470,13 +477,13 @@ grFgets(str, n, stream, name) int sel; f = fn; - sel = select(TX_MAX_OPEN_FILES, &f, (fd_set *) NULL, (fd_set *) NULL, &threeSec); + sel = select(fd + 1, &f, (fd_set *) NULL, (fd_set *) NULL, &threeSec); if (sel == 0) { TxError("The %s is responding slowly, or not at all.\n", name); TxError("I'll wait for 20 seconds and then give up.\n"); f = fn; - sel = select(TX_MAX_OPEN_FILES, &f, (fd_set *) NULL, + sel = select(fd + 1, &f, (fd_set *) NULL, (fd_set *) NULL, &twentySecs); if (sel == 0) {