3 ms·

I love well designed POSIX APIs, such as "this will silently corrupt memory if you use an FD above FD_SETSIZE, which you have no control over and have no sane w

by chc4 3y ago

I love well designed POSIX APIs, such as "this will silently corrupt memory if you use an FD above FD_SETSIZE, which you have no control over and have no sane way of remapping if it does happen".

deathanatos 3y ago
…right? The API design is patently insane, but why can't there be a simple if(nfds > FD_SETSIZE) { errno = EINVAL; return -1; } … or something to prevent "the API is garbage" from escalating all the way into "and now your memory is corrupt and the hackers are in"…?
- 95014_refugee 3y ago
  Because the code was written before unit tests were a thing, and nobody is willing to take the risk / do the work to fix it, especially when "it's been shipping for years and nobody has ever complained".
  - raldi 3y ago
    It’s been shipping for decades and people have been complaining the whole time.
- jcalvinowens 3y ago
  That's not really what the problem is. The actual code is fine. The issue is that the definition of `fd_set` has a constant size [1]. If you allocate the memory yourself, the select() system call will work with as many file descriptors as you care to pass to it. You can see that both glibc [2] and the kernel [3] support arbitrarily large arrays (well, in the kernel case you'll run into other limitations... but no memory corruption). [1] https://github.com/bminor/glibc/blob/master/misc/sys/select.h#L59 https://github.com/bminor/glibc/blob/master/misc/sys/select.... [2] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/select.c#L32 https://github.com/bminor/glibc/blob/master/sysdeps/unix/sys... [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/select.c#n625 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
  - deathanatos 3y ago
    > The issue is that the definition of `fd_set` has a constant size I'm well aware. > If you allocate the memory yourself, the select() system call will work with as many file descriptors as you care to pass to it. I'm aware of this, as well. That the kernel interface is more flexible is fine. But the glibc wrapper could sanity-check its arguments. If you're saying the glibc wrapper also is flexible (assuming one allocs their own fd_sets and … somehow that doesn't run afoul of strict aliasing…?) … and thus, to allow that backbreaking exercise of "you can still use it for all its other bugs" cannot sanity check its arguments (because nfds > the FD set size is permitted) … well … good grief.
    - jcalvinowens 3y ago
      > I'm well aware > I'm aware of this, as well. I'm really curious.. when you typed those sentences, what possible purpose did you imagine them serving? > If you're saying the glibc wrapper also is flexible Yes, it obviously is. There's a long and storied history around this, which you're clearly ignorant of: programmers have been abusing this interface in exactly the way I'm describing for decades. Suddenly breaking a bunch of old code in the name of making an antiquated interface nobody uses anymore "secure" is just bad policy IMHO.
- tedunangst 3y ago
  Because 1024 files ought to be enough for anyone? People wanted to write software to handle more files than that.
jcalvinowens 3y ago
> which you have no control over It's not quite that bad: UNIX has always guaranteed open() will return the lowest unused file descriptor. So in practice, it just limits you to 1024 total open files in the process, which in all fairness probably seemed like an absurdly large number at the time it was designed.
- klempner 3y ago
  And of course that guarantee has its own problem, namely that (especially in a multithreaded process) a use-after-close error is vastly more likely to cause corruption via a write to a newly opened file in the old file descriptor. And in all fairness, nobody was thinking of multithreading when these APIs were designed. We're lucky enough that errno mostly works as a thread local rather than a global.
  - toast0 3y ago
    Use after close is, of course, tons of fun. But the guaranteed order also means open and accept require a per-process lock.
deleted 3y ago
[deleted]
klempner 3y ago
The funny thing is that select's APIs are compatible with a non-broken implementation, such as something that heap allocates if above a constant size. My recollection is that some implementations (winsock?) even do this. Of course, that's never going to actually happen on implementations people care about between ABI breaking on the one hand and the existence of poll/epoll on the other. (My biggest concern in practice is random shitty libraries using select behind the scenes and then silently corrupting memory in processes that have more than a few file descriptors.)

by chc4 3y ago

I love well designed POSIX APIs, such as "this will silently corrupt memory if you use an FD above FD_SETSIZE, which you have no control over and have no sane way of remapping if it does happen".

deathanatos 3y ago

…right? The API design is patently insane, but why can't there be a simple if(nfds > FD_SETSIZE) { errno = EINVAL; return -1; } … or something to prevent "the API is garbage" from escalating all the way into "and now your memory is corrupt and the hackers are in"…?

95014_refugee 3y ago
Because the code was written before unit tests were a thing, and nobody is willing to take the risk / do the work to fix it, especially when "it's been shipping for years and nobody has ever complained".
- raldi 3y ago
  It’s been shipping for decades and people have been complaining the whole time.
jcalvinowens 3y ago
That's not really what the problem is. The actual code is fine. The issue is that the definition of `fd_set` has a constant size [1]. If you allocate the memory yourself, the select() system call will work with as many file descriptors as you care to pass to it. You can see that both glibc [2] and the kernel [3] support arbitrarily large arrays (well, in the kernel case you'll run into other limitations... but no memory corruption). [1] https://github.com/bminor/glibc/blob/master/misc/sys/select.h#L59 https://github.com/bminor/glibc/blob/master/misc/sys/select.... [2] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/select.c#L32 https://github.com/bminor/glibc/blob/master/sysdeps/unix/sys... [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/select.c#n625 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
- deathanatos 3y ago
  > The issue is that the definition of `fd_set` has a constant size I'm well aware. > If you allocate the memory yourself, the select() system call will work with as many file descriptors as you care to pass to it. I'm aware of this, as well. That the kernel interface is more flexible is fine. But the glibc wrapper could sanity-check its arguments. If you're saying the glibc wrapper also is flexible (assuming one allocs their own fd_sets and … somehow that doesn't run afoul of strict aliasing…?) … and thus, to allow that backbreaking exercise of "you can still use it for all its other bugs" cannot sanity check its arguments (because nfds > the FD set size is permitted) … well … good grief.
  - jcalvinowens 3y ago
    > I'm well aware > I'm aware of this, as well. I'm really curious.. when you typed those sentences, what possible purpose did you imagine them serving? > If you're saying the glibc wrapper also is flexible Yes, it obviously is. There's a long and storied history around this, which you're clearly ignorant of: programmers have been abusing this interface in exactly the way I'm describing for decades. Suddenly breaking a bunch of old code in the name of making an antiquated interface nobody uses anymore "secure" is just bad policy IMHO.
tedunangst 3y ago
Because 1024 files ought to be enough for anyone? People wanted to write software to handle more files than that.

jcalvinowens 3y ago

> which you have no control over It's not quite that bad: UNIX has always guaranteed open() will return the lowest unused file descriptor. So in practice, it just limits you to 1024 total open files in the process, which in all fairness probably seemed like an absurdly large number at the time it was designed.

klempner 3y ago
And of course that guarantee has its own problem, namely that (especially in a multithreaded process) a use-after-close error is vastly more likely to cause corruption via a write to a newly opened file in the old file descriptor. And in all fairness, nobody was thinking of multithreading when these APIs were designed. We're lucky enough that errno mostly works as a thread local rather than a global.
- toast0 3y ago
  Use after close is, of course, tons of fun. But the guaranteed order also means open and accept require a per-process lock.

deleted 3y ago

[deleted]

klempner 3y ago

The funny thing is that select's APIs are compatible with a non-broken implementation, such as something that heap allocates if above a constant size. My recollection is that some implementations (winsock?) even do this. Of course, that's never going to actually happen on implementations people care about between ABI breaking on the one hand and the existence of poll/epoll on the other. (My biggest concern in practice is random shitty libraries using select behind the scenes and then silently corrupting memory in processes that have more than a few file descriptors.)