| 29 Dec 2025 |
 emily | you can miscompute a too-high nfds, but then your fd_sets are still the standard size, so select trashes the heap | 00:58:45 |
| emily | so it seems like part of the reason _DARWIN_UNLIMITED_SELECT isn't default is probably security | 00:59:00 |
 Ihar Hrachyshka | yes but the problem is that while they allow FD_SETSIZE to be redefined, libSystem enforces the value as was used during libSystem compilation. | 00:59:09 |
| emily | sure | 00:59:22 |
| emily | I just mean it's a disadvantage of setting it | 00:59:29 |
| emily | with your patch, what happens if glib wants to select over more than 4096 fds? does it do the check itself and bail out, or would it trash the heap? | 01:00:21 |
| Ihar Hrachyshka | probably it will trash the heap. like it will do on *BSD or Windows. | 01:01:59 |
| emily | on Windows it has a custom implementation with the native API and on BSD it'll use poll | 01:02:52 |
 Randy Eckenrode | Windows doesn’t use the same implementation. See the Old New Thing article. | 01:03:00 |
| emily | it's only Darwin using this code path, and indeed it doesn't have checks https://gitlab.gnome.org/GNOME/glib/-/blob/ecef4b16cfe1a67f18c82f7b12f58241922c7b89/glib/gpoll.c#L544 | 01:03:00 |
| Ihar Hrachyshka | docs don't promise / mention fd limits: https://docs.gtk.org/glib/func.poll.html | 01:03:07 |
| emily | well it is only select that has this footgun in the first place | 01:03:32 |
| Ihar Hrachyshka | indeed windows uses a different implementation. It uses same macro but ends up with a different code path.. :( | 01:04:41 |
| emily | https://gitlab.gnome.org/GNOME/glib/-/blob/ecef4b16cfe1a67f18c82f7b12f58241922c7b89/glib/gpoll.c#L544 doesn't bother doing any checks, but Darwin's FD_SET will do overflow checks by default (see sys/_types/_fd_def.h), but _DARWIN_UNLIMITED_SELECT adjusts the behaviour | 01:04:43 |
| emily | so I think this turns something likely caught at runtime into a potential vulnerability, on Darwin | 01:05:06 |
| emily | I think what would be ideal is #define _DARWIN_UNLIMITED_SELECT just in this file and doing it as a heap allocation | 01:05:38 |
| Ihar Hrachyshka | ok. should I add an explicit nfds check against SET_FDSIZE in that g_poll implementation? would it save runtime? | 01:06:07 |
| emily | of a struct { int32_t fds_bits[]; } | 01:06:13 |
| emily | yeah that would also work as a cheaper fix | 01:06:40 |
| emily | but it should only be a few lines to do the heap allocation thing | 01:06:45 |
| emily | something like… | 01:08:49 |
| emily | fd_set rset_stack, wset_stack, xset_stack;
fd_set *rset = &rset_stack, *wset = &wset_stack, *xset = &xset_stack;
if (nfds > FD_SETSIZE)
{
size_t nelems = __DARWIN_howmany(nfds, __DARWIN_NFDBITS);
size_t size = sizeof (struct { int32_t fds_bits[nelems] });
rset = g_malloc0 (size);
wset = g_malloc0 (size);
xset = g_malloc0 (size);
}
…
if (nfds > FD_SETSIZE)
{
g_free (rset);
g_free (wset);
g_free (xset);
}
| 01:12:50 |
| emily | (I guess can g_malloc rather than g_malloc0, since FD_ZERO and all) | 01:13:41 |
| emily | thankfully guint is small enough that you don't have to worry about size overflow 🫣 | 01:14:42 |
| Ihar Hrachyshka | Seems like a better way - no need to choose a value for FD_SETSIZE. will adopt and add a test case as Randy suggested. | 01:16:57 |
| emily | yeah, would definitely not trust this without testing :D | 01:19:44 |
| emily | it does have the somewhat sad effect that every call will do heap allocation when you have lots of fds, but probably code relying on select isn't exactly hyperoptimized to begin with…? | 01:20:28 |
| Ihar Hrachyshka | if you have more fds than what the platform supports, your alternative is getting the fault, so... | 01:23:08 |
| Ihar Hrachyshka | we are not making it worse | 01:23:26 |
| Ihar Hrachyshka | * if you have more fds than what the platform supports, your alternative atm is getting the fault, so... | 01:24:00 |
