Skip to content

Commit

Permalink
passt, util: Close any open file that the parent might have leaked
Browse files Browse the repository at this point in the history
If a parent accidentally or due to implementation reasons leaks any
open file, we don't want to have access to them, except for the file
passed via --fd, if any.

This is the case for Podman when Podman's parent leaks files into
Podman: it's not practical for Podman to close unrelated files before
starting pasta, as reported by Paul.

Use close_range(2) to close all open files except for standard streams
and the one from --fd.

Given that parts of conf() depend on other files to be already opened,
such as the epoll file descriptor, we can't easily defer this to a
more convenient point, where --fd was already parsed. Introduce a
minimal, duplicate version of --fd parsing to keep this simple.

As we need to check that the passed --fd option doesn't exceed
INT_MAX, because we'll parse it with strtol() but file descriptor
indices are signed ints (regardless of the arguments close_range()
take), extend the existing check in the actual --fd parsing in conf(),
also rejecting file descriptors numbers that match standard streams,
while at it.

Suggested-by: Paul Holzinger <[email protected]>
Signed-off-by: Stefano Brivio <[email protected]>
Reviewed-by: David Gibson <[email protected]>
Reviewed-by: Paul Holzinger <[email protected]>
  • Loading branch information
sbrivio-rh committed Aug 8, 2024
1 parent 755f9fd commit 09603ca
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 7 deletions.
8 changes: 6 additions & 2 deletions conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,7 @@ void conf(struct ctx *c, int argc, char **argv)
const char *optstring;
size_t logsize = 0;
char *runas = NULL;
long fd_tap_opt;
int name, ret;
uid_t uid;
gid_t gid;
Expand All @@ -1260,6 +1261,7 @@ void conf(struct ctx *c, int argc, char **argv)
c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = FWD_UNSET;
c->udp.fwd_in.mode = c->udp.fwd_out.mode = FWD_UNSET;

optind = 1;
do {
name = getopt_long(argc, argv, optstring, options, NULL);

Expand Down Expand Up @@ -1424,11 +1426,13 @@ void conf(struct ctx *c, int argc, char **argv)
break;
case 'F':
errno = 0;
c->fd_tap = strtol(optarg, NULL, 0);
fd_tap_opt = strtol(optarg, NULL, 0);

if (c->fd_tap < 0 || errno)
if (errno ||
fd_tap_opt <= STDERR_FILENO || fd_tap_opt > INT_MAX)
die("Invalid --fd: %s", optarg);

c->fd_tap = fd_tap_opt;
c->one_off = true;
*c->sock_path = 0;
break;
Expand Down
12 changes: 9 additions & 3 deletions isolation.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
*
* Executed immediately after startup, drops capabilities we don't
* need at any point during execution (or which we gain back when we
* need by joining other namespaces).
* need by joining other namespaces), and closes any leaked file we
* might have inherited from the parent process.
*
* 2. isolate_user()
* =================
Expand Down Expand Up @@ -166,14 +167,17 @@ static void clamp_caps(void)
}

/**
* isolate_initial() - Early, config independent self isolation
* isolate_initial() - Early, mostly config independent self isolation
* @argc: Argument count
* @argv: Command line options: only --fd (if present) is relevant here
*
* Should:
* - drop unneeded capabilities
* - close all open files except for standard streams and the one from --fd
* Musn't:
* - remove filesytem access (we need to access files during setup)
*/
void isolate_initial(void)
void isolate_initial(int argc, char **argv)
{
uint64_t keep;

Expand Down Expand Up @@ -207,6 +211,8 @@ void isolate_initial(void)
keep |= BIT(CAP_SETFCAP) | BIT(CAP_SYS_PTRACE);

drop_caps_ep_except(keep);

close_open_files(argc, argv);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion isolation.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#ifndef ISOLATION_H
#define ISOLATION_H

void isolate_initial(void);
void isolate_initial(int argc, char **argv);
void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns,
enum passt_modes mode);
int isolate_prefork(const struct ctx *c);
Expand Down
2 changes: 1 addition & 1 deletion passt.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ int main(int argc, char **argv)

arch_avx2_exec(argv);

isolate_initial();
isolate_initial(argc, argv);

c.pasta_netns_fd = c.fd_tap = c.pidfile_fd = -1;

Expand Down
41 changes: 41 additions & 0 deletions util.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <errno.h>
#include <stdbool.h>
#include <linux/errqueue.h>
#include <getopt.h>

#include "util.h"
#include "iov.h"
Expand Down Expand Up @@ -694,3 +695,43 @@ const char *str_ee_origin(const struct sock_extended_err *ee)

return "<invalid>";
}

/**
* close_open_files() - Close leaked files, but not --fd, stdin, stdout, stderr
* @argc: Argument count
* @argv: Command line options, as we need to skip any file given via --fd
*/
void close_open_files(int argc, char **argv)
{
const struct option optfd[] = { { "fd", required_argument, NULL, 'F' },
{ 0 },
};
long fd = -1;
int name, rc;

do {
name = getopt_long(argc, argv, ":F", optfd, NULL);

if (name == 'F') {
errno = 0;
fd = strtol(optarg, NULL, 0);

if (errno || fd <= STDERR_FILENO || fd > INT_MAX)
die("Invalid --fd: %s", optarg);
}
} while (name != -1);

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 (rc)
die_perror("Failed to close files leaked by parent");
}
1 change: 1 addition & 0 deletions util.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ int __daemon(int pidfile_fd, int devnull_fd);
int fls(unsigned long x);
int write_file(const char *path, const char *buf);
int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip);
void close_open_files(int argc, char **argv);

/**
* af_name() - Return name of an address family
Expand Down

0 comments on commit 09603ca

Please sign in to comment.