passt, util: Close any open file that the parent might have leaked

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 <pholzing@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Paul Holzinger <pholzing@redhat.com>
This commit is contained in:
Stefano Brivio 2024-08-06 20:32:11 +02:00
parent 755f9fd911
commit 09603cab28
6 changed files with 59 additions and 7 deletions

8
conf.c
View file

@ -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;
@ -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);
@ -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;

View file

@ -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()
* =================
@ -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;
@ -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);
}
/**

View file

@ -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);

View file

@ -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;

41
util.c
View file

@ -26,6 +26,7 @@
#include <errno.h>
#include <stdbool.h>
#include <linux/errqueue.h>
#include <getopt.h>
#include "util.h"
#include "iov.h"
@ -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
util.h
View file

@ -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