From 62c3edd9575cb7dfef0d86392e4b87a4c14d8dee Mon Sep 17 00:00:00 2001 From: Stefano Brivio Date: Sun, 27 Mar 2022 13:10:26 +0200 Subject: treewide: Fix android-cloexec-* clang-tidy warnings, re-enable checks Signed-off-by: Stefano Brivio --- Makefile | 9 --------- conf.c | 6 +++++- passt.c | 9 +++++---- pasta.c | 16 +++++++++------- pcap.c | 5 +++-- tap.c | 3 ++- tcp_splice.c | 11 +++++------ util.c | 2 +- 8 files changed, 30 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index e972e88..2cb3735 100644 --- a/Makefile +++ b/Makefile @@ -180,13 +180,6 @@ pkgs: static # - readability-isolate-declaration # Dubious value, would kill readability # -# - android-cloexec-open -# - android-cloexec-pipe -# - android-cloexec-pipe2 -# - android-cloexec-epoll-create1 -# - android-cloexec-inotify-init1 -# TODO: check, fix except for the few cases where we need to share fds -# # - bugprone-narrowing-conversions # - cppcoreguidelines-narrowing-conversions # TODO: nice to fix eventually @@ -228,8 +221,6 @@ clang-tidy: $(wildcard *.c) $(wildcard *.h) -llvm-include-order,\ -cppcoreguidelines-avoid-magic-numbers,\ -readability-isolate-declaration,\ - -android-cloexec-open,-android-cloexec-pipe,-android-cloexec-pipe2,\ - -android-cloexec-epoll-create1,-android-cloexec-inotify-init1,\ -bugprone-narrowing-conversions,\ -cppcoreguidelines-narrowing-conversions,\ -cppcoreguidelines-avoid-non-const-global-variables,\ diff --git a/conf.c b/conf.c index 80f4303..ea51de4 100644 --- a/conf.c +++ b/conf.c @@ -285,7 +285,7 @@ static void get_dns(struct ctx *c) if (dns_set && dnss_set) return; - if ((fd = open("/etc/resolv.conf", O_RDONLY)) < 0) + if ((fd = open("/etc/resolv.conf", O_RDONLY | O_CLOEXEC)) < 0) goto out; while (!(*buf = 0) && line_read(buf, BUFSIZ, fd)) { @@ -406,13 +406,17 @@ static int conf_ns_opt(struct ctx *c, continue; } + /* Don't pass O_CLOEXEC here: ns_enter() needs those files */ if (!c->netns_only) { if (*conf_userns) + /* NOLINTNEXTLINE(android-cloexec-open) */ ufd = open(conf_userns, O_RDONLY); else if (*userns) + /* NOLINTNEXTLINE(android-cloexec-open) */ ufd = open(userns, O_RDONLY); } + /* NOLINTNEXTLINE(android-cloexec-open) */ nfd = open(netns, O_RDONLY); if (nfd == -1 || (ufd == -1 && !c->netns_only)) { diff --git a/passt.c b/passt.c index 6de9e5e..c63a3cb 100644 --- a/passt.c +++ b/passt.c @@ -202,7 +202,7 @@ static void check_root(void) if (getuid() && geteuid()) return; - if ((fd = open("/proc/self/uid_map", O_RDONLY)) < 0) + if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) return; if (read(fd, buf, BUFSIZ) > 0 && @@ -359,7 +359,7 @@ int main(int argc, char **argv) if (!c.debug && (c.stderr || isatty(fileno(stdout)))) __openlog(log_name, LOG_PERROR, LOG_DAEMON); - c.epollfd = epoll_create1(0); + c.epollfd = epoll_create1(c.foreground ? O_CLOEXEC : 0); if (c.epollfd == -1) { perror("epoll_create1"); exit(EXIT_FAILURE); @@ -405,11 +405,12 @@ int main(int argc, char **argv) pcap_init(&c); if (!c.foreground) + /* NOLINTNEXTLINE(android-cloexec-open): see __daemon() */ devnull_fd = open("/dev/null", O_RDWR); if (*c.pid_file) - pidfile_fd = open(c.pid_file, - O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR); + pidfile_fd = open(c.pid_file, O_CREAT | O_WRONLY | O_CLOEXEC, + S_IRUSR | S_IWUSR); if (sandbox(&c)) { err("Failed to sandbox process, exiting\n"); diff --git a/pasta.c b/pasta.c index 96866c6..18df5d2 100644 --- a/pasta.c +++ b/pasta.c @@ -78,6 +78,7 @@ void pasta_child_handler(int signal) static int pasta_wait_for_ns(void *arg) { struct ctx *c = (struct ctx *)arg; + int flags = O_RDONLY | O_CLOEXEC; char ns[PATH_MAX]; if (c->netns_only) @@ -85,14 +86,14 @@ static int pasta_wait_for_ns(void *arg) snprintf(ns, PATH_MAX, "/proc/%i/ns/user", pasta_child_pid); do - while ((c->pasta_userns_fd = open(ns, O_RDONLY)) < 0); + while ((c->pasta_userns_fd = open(ns, flags)) < 0); while (setns(c->pasta_userns_fd, CLONE_NEWUSER) && !close(c->pasta_userns_fd)); netns: snprintf(ns, PATH_MAX, "/proc/%i/ns/net", pasta_child_pid); do - while ((c->pasta_netns_fd = open(ns, O_RDONLY)) < 0); + while ((c->pasta_netns_fd = open(ns, flags)) < 0); while (setns(c->pasta_netns_fd, CLONE_NEWNET) && !close(c->pasta_netns_fd)); @@ -126,23 +127,23 @@ static int pasta_setup_ns(void *arg) snprintf(buf, BUFSIZ, "%i %i %i", 0, a->euid, 1); - fd = open("/proc/self/uid_map", O_WRONLY); + fd = open("/proc/self/uid_map", O_WRONLY | O_CLOEXEC); if (write(fd, buf, strlen(buf)) < 0) warn("Cannot set uid_map in namespace"); close(fd); - fd = open("/proc/self/setgroups", O_WRONLY); + fd = open("/proc/self/setgroups", O_WRONLY | O_CLOEXEC); if (write(fd, "deny", sizeof("deny")) < 0) warn("Cannot write to setgroups in namespace"); close(fd); - fd = open("/proc/self/gid_map", O_WRONLY); + fd = open("/proc/self/gid_map", O_WRONLY | O_CLOEXEC); if (write(fd, buf, strlen(buf)) < 0) warn("Cannot set gid_map in namespace"); close(fd); } - fd = open("/proc/sys/net/ipv4/ping_group_range", O_WRONLY); + fd = open("/proc/sys/net/ipv4/ping_group_range", O_WRONLY | O_CLOEXEC); if (write(fd, "0 0", strlen("0 0")) < 0) warn("Cannot set ping_group_range, ICMP requests might fail"); close(fd); @@ -231,13 +232,14 @@ void pasta_ns_conf(struct ctx *c) */ int pasta_netns_quit_init(struct ctx *c) { + int flags = O_NONBLOCK | (c->foreground ? O_CLOEXEC : 0); struct epoll_event ev = { .events = EPOLLIN }; int inotify_fd; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) return -1; - if ((inotify_fd = inotify_init1(O_NONBLOCK)) < 0) { + if ((inotify_fd = inotify_init1(flags)) < 0) { perror("inotify_init(): won't quit once netns is gone"); return -1; } diff --git a/pcap.c b/pcap.c index e5b8b72..296bbb5 100644 --- a/pcap.c +++ b/pcap.c @@ -170,6 +170,7 @@ fail: */ void pcap_init(struct ctx *c) { + int flags = O_WRONLY | O_CREAT | O_TRUNC; struct timeval tv; if (pcap_fd != -1) @@ -200,8 +201,8 @@ void pcap_init(struct ctx *c) strncpy(c->pcap, name, PATH_MAX); } - pcap_fd = open(c->pcap, O_WRONLY | O_CREAT | O_TRUNC, - S_IRUSR | S_IWUSR); + flags |= c->foreground ? O_CLOEXEC : 0; + pcap_fd = open(c->pcap, flags, S_IRUSR | S_IWUSR); if (pcap_fd == -1) { perror("open"); return; diff --git a/tap.c b/tap.c index 8415d5a..6333015 100644 --- a/tap.c +++ b/tap.c @@ -875,12 +875,13 @@ static int tun_ns_fd = -1; static int tap_ns_tun(void *arg) { struct ifreq ifr = { .ifr_flags = IFF_TAP | IFF_NO_PI }; + int flags = O_RDWR | O_NONBLOCK | O_CLOEXEC; struct ctx *c = (struct ctx *)arg; strncpy(ifr.ifr_name, c->pasta_ifn, IFNAMSIZ); if (ns_enter(c) || - (tun_ns_fd = open("/dev/net/tun", O_RDWR | O_NONBLOCK)) < 0 || + (tun_ns_fd = open("/dev/net/tun", flags)) < 0 || ioctl(tun_ns_fd, TUNSETIFF, &ifr) || !(c->pasta_ifi = if_nametoindex(c->pasta_ifn))) tun_ns_fd = -1; diff --git a/tcp_splice.c b/tcp_splice.c index 0f4f485..714571c 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -370,8 +370,8 @@ static int tcp_splice_connect_finish(const struct ctx *c, } if (conn->pipe_a_b[0] < 0) { - if (pipe2(conn->pipe_a_b, O_NONBLOCK) || - pipe2(conn->pipe_b_a, O_NONBLOCK)) { + if (pipe2(conn->pipe_a_b, O_NONBLOCK | O_CLOEXEC) || + pipe2(conn->pipe_b_a, O_NONBLOCK | O_CLOEXEC)) { conn_flag(c, conn, CLOSING); return -EIO; } @@ -773,7 +773,7 @@ static void tcp_set_pipe_size(struct ctx *c) smaller: for (i = 0; i < TCP_SPLICE_PIPE_POOL_SIZE * 2; i++) { - if (pipe2(probe_pipe[i], 0)) { + if (pipe2(probe_pipe[i], O_CLOEXEC)) { i++; break; } @@ -809,9 +809,9 @@ static void tcp_splice_pipe_refill(const struct ctx *c) for (i = 0; i < TCP_SPLICE_PIPE_POOL_SIZE; i++) { if (splice_pipe_pool[i][0][0] >= 0) break; - if (pipe2(splice_pipe_pool[i][0], O_NONBLOCK)) + if (pipe2(splice_pipe_pool[i][0], O_NONBLOCK | O_CLOEXEC)) continue; - if (pipe2(splice_pipe_pool[i][1], O_NONBLOCK)) { + if (pipe2(splice_pipe_pool[i][1], O_NONBLOCK | O_CLOEXEC)) { close(splice_pipe_pool[i][1][0]); close(splice_pipe_pool[i][1][1]); continue; @@ -832,7 +832,6 @@ void tcp_splice_init(struct ctx *c) { memset(splice_pipe_pool, 0xff, sizeof(splice_pipe_pool)); tcp_set_pipe_size(c); - tcp_splice_pipe_refill(c); } /** diff --git a/util.c b/util.c index 83eba82..81cf744 100644 --- a/util.c +++ b/util.c @@ -495,7 +495,7 @@ void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns, if (*fd != -1) lseek(*fd, 0, SEEK_SET); - else if ((*fd = open(path, O_RDONLY)) < 0) + else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) return; *line = 0; -- cgit v1.2.3