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_initial() so we don't close() it along with any other file descriptors leaked into us by hte parent. More importantly, however, 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, 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_initial() made safe for --fd 1 and --fd 2, we can remove the logic excluding those from conf_fd_tap(). Signed-off-by: David Gibson --- conf.c | 4 +--- isolation.c | 21 +++++++++------------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/conf.c b/conf.c index f0bce41b..67f0db6e 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 c4d476d6..1a7b8527 100644 --- a/isolation.c +++ b/isolation.c @@ -207,7 +207,7 @@ static int move_root(void) */ int isolate_initial(int argc, char **argv) { - int rc, fd_tap = -1; + int fd_tap = -1, close_from = STDERR_FILENO + 1; uint64_t keep; /* We want to keep CAP_NET_BIND_SERVICE in the initial @@ -249,19 +249,16 @@ int isolate_initial(int argc, char **argv) drop_caps_ep_except(keep); fd_tap = conf_tap_fd(argc, argv); - - if (fd_tap == -1) { - rc = close_range(STDERR_FILENO + 1, ~0U, CLOSE_RANGE_UNSHARE); - } else if (fd_tap == 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_tap - 1, - CLOSE_RANGE_UNSHARE); - if (!rc) - rc = close_range(fd_tap + 1, ~0U, CLOSE_RANGE_UNSHARE); + if (fd_tap >= 0) { + /* Move the passed fd to a more convenient location */ + if (fd_tap != close_from && + (dup2(fd_tap, close_from) != close_from || + close(fd_tap))) + die_perror("Could not move --fd descriptor"); + fd_tap = 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