luke
/
magic
mirror of https://github.com/RTimothyEdwards/magic.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Darryl L. Miles 2025-02-24 08:44:09 +00:00 committed by R. Timothy Edwards
parent bcd81c74b2
commit 8d6571066d
1 changed files with 10 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
graphics/grMain.c
View File

@ -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");
if (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)
{