diff mbox series

[iptables] Use SOCK_CLOEXEC/O_CLOEXEC where available

Message ID 20230810112542.21382-1-phil@nwl.cc
State Accepted, archived
Headers show
Series [iptables] Use SOCK_CLOEXEC/O_CLOEXEC where available | expand

Commit Message

Phil Sutter Aug. 10, 2023, 11:25 a.m. UTC
No need for the explicit fcntl() call, request the behaviour when
opening the descriptor.

One fcntl() call setting FD_CLOEXEC remains in extensions/libxt_bpf.c,
the indirect syscall seems not to support passing the flag directly.

Reported-by: Gaurav Gupta <g.gupta@samsung.com>
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1104
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_set.h |  8 +-------
 libiptc/libiptc.c      |  8 +-------
 libxtables/xtables.c   | 15 ++-------------
 3 files changed, 4 insertions(+), 27 deletions(-)

Comments

Phil Sutter Aug. 10, 2023, 12:15 p.m. UTC | #1
On Thu, Aug 10, 2023 at 01:25:42PM +0200, Phil Sutter wrote:
> No need for the explicit fcntl() call, request the behaviour when
> opening the descriptor.
> 
> One fcntl() call setting FD_CLOEXEC remains in extensions/libxt_bpf.c,
> the indirect syscall seems not to support passing the flag directly.
> 
> Reported-by: Gaurav Gupta <g.gupta@samsung.com>
> Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1104
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Patch applied.
diff mbox series

Patch

diff --git a/extensions/libxt_set.h b/extensions/libxt_set.h
index 597bf7ebe575a..685bfab955597 100644
--- a/extensions/libxt_set.h
+++ b/extensions/libxt_set.h
@@ -10,7 +10,7 @@ 
 static int
 get_version(unsigned *version)
 {
-	int res, sockfd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
+	int res, sockfd = socket(AF_INET, SOCK_RAW | SOCK_CLOEXEC, IPPROTO_RAW);
 	struct ip_set_req_version req_version;
 	socklen_t size = sizeof(req_version);
 	
@@ -18,12 +18,6 @@  get_version(unsigned *version)
 		xtables_error(OTHER_PROBLEM,
 			      "Can't open socket to ipset.\n");
 
-	if (fcntl(sockfd, F_SETFD, FD_CLOEXEC) == -1) {
-		xtables_error(OTHER_PROBLEM,
-			      "Could not set close on exec: %s\n",
-			      strerror(errno));
-	}
-
 	req_version.op = IP_SET_OP_VERSION;
 	res = getsockopt(sockfd, SOL_IP, SO_IP_SET, &req_version, &size);
 	if (res != 0)
diff --git a/libiptc/libiptc.c b/libiptc/libiptc.c
index 29ff356f2324e..e475063367c26 100644
--- a/libiptc/libiptc.c
+++ b/libiptc/libiptc.c
@@ -1318,16 +1318,10 @@  TC_INIT(const char *tablename)
 		return NULL;
 	}
 
-	sockfd = socket(TC_AF, SOCK_RAW, IPPROTO_RAW);
+	sockfd = socket(TC_AF, SOCK_RAW | SOCK_CLOEXEC, IPPROTO_RAW);
 	if (sockfd < 0)
 		return NULL;
 
-	if (fcntl(sockfd, F_SETFD, FD_CLOEXEC) == -1) {
-		fprintf(stderr, "Could not set close on exec: %s\n",
-			strerror(errno));
-		abort();
-	}
-
 	s = sizeof(info);
 
 	strcpy(info.name, tablename);
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index e3e444acbbaa2..ba9ceaeb3da41 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -481,14 +481,9 @@  static char *get_modprobe(void)
 	char *ret;
 	int count;
 
-	procfile = open(PROC_SYS_MODPROBE, O_RDONLY);
+	procfile = open(PROC_SYS_MODPROBE, O_RDONLY | O_CLOEXEC);
 	if (procfile < 0)
 		return NULL;
-	if (fcntl(procfile, F_SETFD, FD_CLOEXEC) == -1) {
-		fprintf(stderr, "Could not set close on exec: %s\n",
-			strerror(errno));
-		exit(1);
-	}
 
 	ret = malloc(PATH_MAX);
 	if (ret) {
@@ -1023,7 +1018,7 @@  int xtables_compatible_revision(const char *name, uint8_t revision, int opt)
 	socklen_t s = sizeof(rev);
 	int max_rev, sockfd;
 
-	sockfd = socket(afinfo->family, SOCK_RAW, IPPROTO_RAW);
+	sockfd = socket(afinfo->family, SOCK_RAW | SOCK_CLOEXEC, IPPROTO_RAW);
 	if (sockfd < 0) {
 		if (errno == EPERM) {
 			/* revision 0 is always supported. */
@@ -1039,12 +1034,6 @@  int xtables_compatible_revision(const char *name, uint8_t revision, int opt)
 		exit(1);
 	}
 
-	if (fcntl(sockfd, F_SETFD, FD_CLOEXEC) == -1) {
-		fprintf(stderr, "Could not set close on exec: %s\n",
-			strerror(errno));
-		exit(1);
-	}
-
 	xtables_load_ko(xtables_modprobe_program, true);
 
 	strncpy(rev.name, name, XT_EXTENSION_MAXNAMELEN - 1);