Some users of passt pass an fd for the tap interface in, with the --fd
parameter, rather than having passt open it itself. This requires some
slightly fiddly logic in isolate_fds() so we don't close() it along with
any other file descriptors leaked into us by the parent.
More importantly, this is broken if the passed fd is 0, 1 or 2, since in
that case we will assume it's a standard stream and close it in __daemon().
We explicitly disallow 1 or 2 in conf_tap_fd(), but 0 has been permitted
since aa1cc8922 ("conf: allow --fd 0"). It looks like the use case of the
contributor of that patch didn't involve daemonizing passt.
To fix this more robustly, use dup2() to move to the passed fd to 3. This
removes the possibility of mixing it up with a standard stream, and as a
bonus makes the close_range() logic much simpler. With isolate_fds() made
safe for --fd 1 and --fd 2, we can remove the logic excluding those from
conf_fd_tap().
Cc: Alyssa Ross
Signed-off-by: David Gibson
---
conf.c | 4 +---
isolation.c | 24 +++++++++++-------------
2 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/conf.c b/conf.c
index df204d13..0fcba5cf 100644
--- a/conf.c
+++ b/conf.c
@@ -1177,9 +1177,7 @@ int conf_tap_fd(int argc, char **argv)
return -1;
p = fdarg;
- if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) ||
- val > INT_MAX ||
- (val != STDIN_FILENO && val <= STDERR_FILENO))
+ if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) || val > INT_MAX)
die("Invalid --fd: %s", fdarg);
return val;
diff --git a/isolation.c b/isolation.c
index 725a72bb..94cbe7f8 100644
--- a/isolation.c
+++ b/isolation.c
@@ -248,7 +248,6 @@ void isolate_initial(void)
drop_caps_ep_except(keep);
}
-
/*
* isolate_fds() - Close leaked files, but not --fd, stdin, stdout, stderr
* @argc: Argument count
@@ -256,27 +255,26 @@ void isolate_initial(void)
*
* Should:
* - close all open files except for standard streams and the one from --fd
+ * - move the --fd descriptor out of the range 0-2
*
- * Return: fd number from --fd, or -1 if not specified
+ * Return: new fd number for descriptor from --fd, or -1 if not specified
*/
int isolate_fds(int argc, char **argv)
{
- int fd, rc;
+ int fd, close_from = STDERR_FILENO + 1;
fd = conf_tap_fd(argc, argv);
- if (fd == -1) {
- rc = close_range(STDERR_FILENO + 1, ~0U, CLOSE_RANGE_UNSHARE);
- } else if (fd == STDERR_FILENO + 1) { /* Still a single range */
- rc = close_range(STDERR_FILENO + 2, ~0U, CLOSE_RANGE_UNSHARE);
- } else {
- rc = close_range(STDERR_FILENO + 1, fd - 1,
- CLOSE_RANGE_UNSHARE);
- if (!rc)
- rc = close_range(fd + 1, ~0U, CLOSE_RANGE_UNSHARE);
+ if (fd >= 0) {
+ /* Move the passed fd to a more convenient location */
+ if (fd != close_from &&
+ (dup2(fd, close_from) != close_from ||
+ close(fd)))
+ die_perror("Could not move --fd descriptor");
+ fd = close_from++;
}
- if (rc) {
+ if (close_range(close_from, ~0U, CLOSE_RANGE_UNSHARE)) {
if (errno == ENOSYS || errno == EINVAL) {
/* This probably means close_range() or the
* CLOSE_RANGE_UNSHARE flag is not supported by the
--
2.55.0