gspawn: Ignore invalid FDs when using safe_fdwalk()

In `safe_closefrom()`, we thought it would be OK to assert that an FD
being closed is valid, when using `safe_fdwalk()`, as it only walks over
known-valid FDs.

However, there is a situation where that might not be true: the program
is being run under valgrind. Valgrind opens some FDs for its own use
which are ≥1024, and it emulates a lowered soft limit on FDs. So if we
were to use `safe_fdwalk_with_invalid_fds()` it would see the lowered
soft limit and not try to close valgrind’s internal FDs.

However, `safe_fdwalk()` looks at `/proc`, which valgrind does not emulate,
so it sees the secret valgrind internal FDs, and then tries to close them.
Valgrind doesn’t like this, prints ‘Warning: invalid file descriptor
1024 in syscall close()’ and returns `EBADF`. That return value
causes `g_close()` to warn about faulty FD refcounting, and that causes
unit test failures.

Fix that by relaxing our assumptions about FD validity: use
the `close_func_with_invalid_fds()` call back for closing FDs
from `safe_fdwalk()`, rather than using `close_func()`. That will
ignore `EBADF` return values.

This should fix valgrind failures like this one:
https://gitlab.gnome.org/GNOME/glib/-/jobs/2389977

Related prior art: https://bugs.freedesktop.org/show_bug.cgi?id=99839

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This commit is contained in:
Philip Withnall 2022-11-16 12:37:22 +00:00
parent 96bc6d6e7c
commit f0e08e3488

View File

@ -1343,21 +1343,6 @@ dupfd_cloexec (int old_fd, int new_fd_min)
return fd;
}
/* fdwalk()-compatible callback to close a valid fd.
* It is an error to pass an invalid fd (causing EBADF) to this function.
*
* This function is called between fork() and exec() and hence must be
* async-signal-safe (see signal-safety(7)).
*/
G_GNUC_UNUSED static int
close_func (void *data, int fd)
{
if (fd >= GPOINTER_TO_INT (data))
g_close (fd, NULL);
return 0;
}
/* fdwalk()-compatible callback to close a fd for non-compliant
* implementations of fdwalk() that potentially pass already
* closed fds.
@ -1678,7 +1663,7 @@ safe_closefrom (int lowfd)
if (ret == 0 || errno != ENOSYS)
return ret;
#endif /* HAVE_CLOSE_RANGE */
ret = safe_fdwalk (close_func, GINT_TO_POINTER (lowfd));
ret = safe_fdwalk (close_func_with_invalid_fds, GINT_TO_POINTER (lowfd));
if (ret < 0 && errno == ENOSYS)
ret = safe_fdwalk_with_invalid_fds (close_func_with_invalid_fds, GINT_TO_POINTER (lowfd));