mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2024-11-08 18:36:17 +01:00
gspawn: Report errors with closing file descriptors between fork/exec
If a seccomp policy is set up incorrectly so that it returns `EPERM` for `close_range()` rather than `ENOSYS` due to it not being recognised, no error would previously be reported from GLib, but some file descriptors wouldn’t be closed, and that would cause a hung zombie process. The zombie process would be waiting for one half of a socket to be closed. Fix that by correctly propagating errors from `close_range()` back to the parent process so they can be reported correctly. Distributions which aren’t yet carrying the Docker fix to correctly return `ENOSYS` from unrecognised syscalls may want to temporarily carry an additional patch to fall back to `safe_fdwalk()` if `close_range()` fails with `EPERM`. This change will not be accepted upstream as `EPERM` is not the right error for `close_range()` to be returning. Signed-off-by: Philip Withnall <pwithnall@endlessos.org> Fixes: #2580
This commit is contained in:
parent
a7a5b24f86
commit
ce04a12404
@ -1520,7 +1520,7 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
|
|||||||
|
|
||||||
/* This function is called between fork() and exec() and hence must be
|
/* This function is called between fork() and exec() and hence must be
|
||||||
* async-signal-safe (see signal-safety(7)). */
|
* async-signal-safe (see signal-safety(7)). */
|
||||||
static void
|
static int
|
||||||
safe_fdwalk_set_cloexec (int lowfd)
|
safe_fdwalk_set_cloexec (int lowfd)
|
||||||
{
|
{
|
||||||
#if defined(HAVE_CLOSE_RANGE) && defined(CLOSE_RANGE_CLOEXEC)
|
#if defined(HAVE_CLOSE_RANGE) && defined(CLOSE_RANGE_CLOEXEC)
|
||||||
@ -1534,15 +1534,18 @@ safe_fdwalk_set_cloexec (int lowfd)
|
|||||||
* Handle ENOSYS in case it’s supported in libc but not the kernel; if so,
|
* Handle ENOSYS in case it’s supported in libc but not the kernel; if so,
|
||||||
* fall back to safe_fdwalk(). Handle EINVAL in case `CLOSE_RANGE_CLOEXEC`
|
* fall back to safe_fdwalk(). Handle EINVAL in case `CLOSE_RANGE_CLOEXEC`
|
||||||
* is not supported. */
|
* is not supported. */
|
||||||
if (close_range (lowfd, G_MAXUINT, CLOSE_RANGE_CLOEXEC) != 0 &&
|
int ret = close_range (lowfd, G_MAXUINT, CLOSE_RANGE_CLOEXEC);
|
||||||
(errno == ENOSYS || errno == EINVAL))
|
if (ret == 0 || !(errno == ENOSYS || errno == EINVAL))
|
||||||
|
return ret;
|
||||||
#endif /* HAVE_CLOSE_RANGE */
|
#endif /* HAVE_CLOSE_RANGE */
|
||||||
(void) safe_fdwalk (set_cloexec, GINT_TO_POINTER (lowfd));
|
return safe_fdwalk (set_cloexec, GINT_TO_POINTER (lowfd));
|
||||||
}
|
}
|
||||||
|
|
||||||
/* This function is called between fork() and exec() and hence must be
|
/* This function is called between fork() and exec() and hence must be
|
||||||
* async-signal-safe (see signal-safety(7)). */
|
* async-signal-safe (see signal-safety(7)).
|
||||||
static void
|
*
|
||||||
|
* On failure, `-1` will be returned and errno will be set. */
|
||||||
|
static int
|
||||||
safe_closefrom (int lowfd)
|
safe_closefrom (int lowfd)
|
||||||
{
|
{
|
||||||
#if defined(__FreeBSD__) || defined(__OpenBSD__) || \
|
#if defined(__FreeBSD__) || defined(__OpenBSD__) || \
|
||||||
@ -1560,6 +1563,7 @@ safe_closefrom (int lowfd)
|
|||||||
* On such systems, F_CLOSEFROM is defined.
|
* On such systems, F_CLOSEFROM is defined.
|
||||||
*/
|
*/
|
||||||
(void) closefrom (lowfd);
|
(void) closefrom (lowfd);
|
||||||
|
return 0;
|
||||||
#elif defined(__DragonFly__)
|
#elif defined(__DragonFly__)
|
||||||
/* It is unclear whether closefrom function included in DragonFlyBSD libc_r
|
/* It is unclear whether closefrom function included in DragonFlyBSD libc_r
|
||||||
* is safe to use because it calls a lot of library functions. It is also
|
* is safe to use because it calls a lot of library functions. It is also
|
||||||
@ -1567,12 +1571,13 @@ safe_closefrom (int lowfd)
|
|||||||
* direct system call here ourselves to avoid possible issues.
|
* direct system call here ourselves to avoid possible issues.
|
||||||
*/
|
*/
|
||||||
(void) syscall (SYS_closefrom, lowfd);
|
(void) syscall (SYS_closefrom, lowfd);
|
||||||
|
return 0;
|
||||||
#elif defined(F_CLOSEM)
|
#elif defined(F_CLOSEM)
|
||||||
/* NetBSD and AIX have a special fcntl command which does the same thing as
|
/* NetBSD and AIX have a special fcntl command which does the same thing as
|
||||||
* closefrom. NetBSD also includes closefrom function, which seems to be a
|
* closefrom. NetBSD also includes closefrom function, which seems to be a
|
||||||
* simple wrapper of the fcntl command.
|
* simple wrapper of the fcntl command.
|
||||||
*/
|
*/
|
||||||
(void) fcntl (lowfd, F_CLOSEM);
|
return fcntl (lowfd, F_CLOSEM);
|
||||||
#else
|
#else
|
||||||
|
|
||||||
#if defined(HAVE_CLOSE_RANGE)
|
#if defined(HAVE_CLOSE_RANGE)
|
||||||
@ -1582,9 +1587,11 @@ safe_closefrom (int lowfd)
|
|||||||
*
|
*
|
||||||
* Handle ENOSYS in case it’s supported in libc but not the kernel; if so,
|
* Handle ENOSYS in case it’s supported in libc but not the kernel; if so,
|
||||||
* fall back to safe_fdwalk(). */
|
* fall back to safe_fdwalk(). */
|
||||||
if (close_range (lowfd, G_MAXUINT, 0) != 0 && errno == ENOSYS)
|
int ret = close_range (lowfd, G_MAXUINT, 0);
|
||||||
|
if (ret == 0 || errno != ENOSYS)
|
||||||
|
return ret;
|
||||||
#endif /* HAVE_CLOSE_RANGE */
|
#endif /* HAVE_CLOSE_RANGE */
|
||||||
(void) safe_fdwalk (close_func, GINT_TO_POINTER (lowfd));
|
return safe_fdwalk (close_func, GINT_TO_POINTER (lowfd));
|
||||||
#endif
|
#endif
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1622,7 +1629,8 @@ enum
|
|||||||
CHILD_EXEC_FAILED,
|
CHILD_EXEC_FAILED,
|
||||||
CHILD_OPEN_FAILED,
|
CHILD_OPEN_FAILED,
|
||||||
CHILD_DUP2_FAILED,
|
CHILD_DUP2_FAILED,
|
||||||
CHILD_FORK_FAILED
|
CHILD_FORK_FAILED,
|
||||||
|
CHILD_CLOSE_FAILED,
|
||||||
};
|
};
|
||||||
|
|
||||||
/* This function is called between fork() and exec() and hence must be
|
/* This function is called between fork() and exec() and hence must be
|
||||||
@ -1738,12 +1746,14 @@ do_exec (gint child_err_report_fd,
|
|||||||
if (safe_dup2 (child_err_report_fd, 3) < 0)
|
if (safe_dup2 (child_err_report_fd, 3) < 0)
|
||||||
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);
|
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);
|
||||||
set_cloexec (GINT_TO_POINTER (0), 3);
|
set_cloexec (GINT_TO_POINTER (0), 3);
|
||||||
safe_closefrom (4);
|
if (safe_closefrom (4) < 0)
|
||||||
|
write_err_and_exit (child_err_report_fd, CHILD_CLOSE_FAILED);
|
||||||
child_err_report_fd = 3;
|
child_err_report_fd = 3;
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
safe_fdwalk_set_cloexec (3);
|
if (safe_fdwalk_set_cloexec (3) < 0)
|
||||||
|
write_err_and_exit (child_err_report_fd, CHILD_CLOSE_FAILED);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
@ -2544,6 +2554,14 @@ fork_exec (gboolean intermediate_child,
|
|||||||
g_strerror (buf[1]));
|
g_strerror (buf[1]));
|
||||||
break;
|
break;
|
||||||
|
|
||||||
|
case CHILD_CLOSE_FAILED:
|
||||||
|
g_set_error (error,
|
||||||
|
G_SPAWN_ERROR,
|
||||||
|
G_SPAWN_ERROR_FAILED,
|
||||||
|
_("Failed to close file descriptor for child process (%s)"),
|
||||||
|
g_strerror (buf[1]));
|
||||||
|
break;
|
||||||
|
|
||||||
default:
|
default:
|
||||||
g_set_error (error,
|
g_set_error (error,
|
||||||
G_SPAWN_ERROR,
|
G_SPAWN_ERROR,
|
||||||
|
Loading…
Reference in New Issue
Block a user