public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top
Subject: [PATCH 3/3] Don't unnecessarily avoid CLOEXEC flags
Date: Tue, 23 Aug 2022 16:31:51 +1000	[thread overview]
Message-ID: <20220823063151.854034-4-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20220823063151.854034-1-david@gibson.dropbear.id.au>

[-- Attachment #1: Type: text/plain, Size: 2679 bytes --]

There are several places in the passt code where we have lint overrides
because we're not adding CLOEXEC flags to open or other operations.
Comments suggest this is because it's before we fork() into the background
but we'll need those file descriptors after we're in the background.

However, as the name suggests CLOEXEC closes on exec(), not on fork().  The
only place we exec() is either super early invoke the avx2 version of the
binary, or when we start a shell in pasta mode, which certainly *doesn't*
require the fds in question.

Add the CLOEXEC flag in those places, and remove the lint overrides.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 10 +++-------
 passt.c |  6 ++----
 pasta.c |  2 +-
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/conf.c b/conf.c
index ac81c15..d936157 100644
--- a/conf.c
+++ b/conf.c
@@ -562,18 +562,14 @@ 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);
+				ufd = open(conf_userns, O_RDONLY | O_CLOEXEC);
 			else if (*userns)
-				/* NOLINTNEXTLINE(android-cloexec-open) */
-				ufd = open(userns, O_RDONLY);
+				ufd = open(userns, O_RDONLY | O_CLOEXEC);
 		}
 
-		/* NOLINTNEXTLINE(android-cloexec-open) */
-		nfd = open(netns, O_RDONLY);
+		nfd = open(netns, O_RDONLY | O_CLOEXEC);
 
 		if (nfd == -1 || (ufd == -1 && !c->netns_only)) {
 			if (nfd >= 0)
diff --git a/passt.c b/passt.c
index 0113002..bbf53d9 100644
--- a/passt.c
+++ b/passt.c
@@ -329,8 +329,7 @@ int main(int argc, char **argv)
 
 	__setlogmask(LOG_MASK(LOG_EMERG));
 
-	/* NOLINTNEXTLINE(android-cloexec-epoll-create1): forking in a moment */
-	c.epollfd = epoll_create1(0);
+	c.epollfd = epoll_create1(EPOLL_CLOEXEC);
 	if (c.epollfd == -1) {
 		perror("epoll_create1");
 		exit(EXIT_FAILURE);
@@ -381,8 +380,7 @@ int main(int argc, char **argv)
 	pcap_init(&c);
 
 	if (!c.foreground) {
-		/* NOLINTNEXTLINE(android-cloexec-open): see __daemon() */
-		if ((devnull_fd = open("/dev/null", O_RDWR)) < 0) {
+		if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0) {
 			perror("/dev/null open");
 			exit(EXIT_FAILURE);
 		}
diff --git a/pasta.c b/pasta.c
index 5a78065..830748f 100644
--- a/pasta.c
+++ b/pasta.c
@@ -223,7 +223,7 @@ void pasta_ns_conf(struct ctx *c)
  */
 int pasta_netns_quit_init(struct ctx *c)
 {
-	int flags = O_NONBLOCK | (c->foreground ? O_CLOEXEC : 0);
+	int flags = O_NONBLOCK | O_CLOEXEC;
 	struct epoll_event ev = { .events = EPOLLIN };
 	int inotify_fd;
 
-- 
@@ -223,7 +223,7 @@ void pasta_ns_conf(struct ctx *c)
  */
 int pasta_netns_quit_init(struct ctx *c)
 {
-	int flags = O_NONBLOCK | (c->foreground ? O_CLOEXEC : 0);
+	int flags = O_NONBLOCK | O_CLOEXEC;
 	struct epoll_event ev = { .events = EPOLLIN };
 	int inotify_fd;
 
-- 
2.37.2


  parent reply	other threads:[~2022-08-23  6:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-23  6:31 [PATCH 0/3] Miscellaneous small fixes David Gibson
2022-08-23  6:31 ` [PATCH 1/3] conf: Fix incorrect bounds checking for sock_path parameter David Gibson
2022-08-23  6:31 ` [PATCH 2/3] gitignore README.plain.md David Gibson
2022-08-23  6:31 ` David Gibson [this message]
2022-08-24 20:20   ` [PATCH 3/3] Don't unnecessarily avoid CLOEXEC flags Stefano Brivio
2022-08-24 20:21 ` [PATCH 0/3] Miscellaneous small fixes Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220823063151.854034-4-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).