| 29 Dec 2025 |
 Randy Eckenrode | I wonder if the change should be localized to just the g_poll implementation. | 00:56:08 |
 Ihar Hrachyshka | the glib_conf meson declaration affects config.h used to build glib | 00:56:26 |
 emily | ok, going by glib_conf.set('_GNU_SOURCE', 1) I suppose it ought to be local :) | 00:56:43 |
| Randy Eckenrode | I pondered whether to use a heuristic to malloc enough, but doesn’t the set size have to cover all fds in the process? | 00:56:53 |
| emily | won't work, since it has to be set early enough for headers | 00:56:56 |
| emily | you'd at least need to make it scoped to the file | 00:57:03 |
| Ihar Hrachyshka |
I wonder if the change should be localized to just the g_poll implementation.
it is already. The BROKEN_POLL macro is only used in g_poll.
| 00:57:36 |
| emily | btw, doesn't the check against FD_SETSIZE prevent potential heap overflow? | 00:58:01 |
| 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 |
