Merge pull request #2611 from pqarmitage/updates

Ensure file descriptors are not leaked to scripts

Author: pqarmitage

README.kernel_versions

@@ -78,6 +78,15 @@ dnl -- Since Linux 5.7
 NFT_SET_CONCAT
 NFTNL_SET_EXPR also libnftnl 1.1.7, nft 0.9.5
 
+Since Linux 5.9
+close_range syscall
+
+Since Linux 5.11
+CLOSE_RANGE_CLOEXEC
+
+Since glibc 2.34
+close_range() library function
+
 Since Linux 5.18
 IFA_PROTO
 

configure.ac

@@ -1380,8 +1380,7 @@ AC_FUNC_FORK
 # We add malloc and realloc to AC_CHECK_FUNCS instead.
 #AC_FUNC_MALLOC
 #AC_FUNC_REALLOC
-dnl - dup3 since Linux 2.6.27 and glibc 2.9
-AC_CHECK_FUNCS([dup2 dup3 getcwd gettimeofday malloc memmove memset realloc select setenv socket strcasecmp strchr strdup strerror strpbrk strstr strtol strtoul uname])
+AC_CHECK_FUNCS([getcwd gettimeofday malloc memmove memset realloc select setenv socket strcasecmp strchr strdup strerror strpbrk strstr strtol strtoul uname])
 dnl - vsyslog() Not defined by Posix, but available in glibc and musl
 AC_CHECK_FUNCS([vsyslog], [add_system_opt([VSYSLOG])])
 
@@ -1396,6 +1395,21 @@ AS_IF([test "x$ac_cv_func_memfd_create" != xyes],
       [[#include <sys/syscall.h>]])
   ])
 
+dnl - close_range() - since Linux 5.9 and glibc 2.34
+dnl - close_range() appeared in the kernel some time before it appeared in
+dnl -   glibc so we use the raw syscall if it is available
+dnl - CLOSE_RANGE_CLOEXEC added in Linux 5.11.
+AC_CHECK_FUNCS([close_range], [add_system_opt([CLOSE_RANGE])])
+AS_IF([test "x$ac_cv_func_close_range" != xyes],
+  [
+    AC_CHECK_DECLS([__NR_close_range],
+      [
+        AC_DEFINE([USE_CLOSE_RANGE_SYSCALL], [ 1 ], [Use syscall for close_range])
+	add_system_opt([CLOSE_RANGE_SYSCALL])
+      ], [], [[#include <sys/syscall.h>]])
+  ])
+AC_CHECK_DECLS([CLOSE_RANGE_CLOEXEC], [], [], [[#include <linux/close_range.h>]])
+
 dnl - Since Linux 3.17
 AC_CHECK_DECLS([O_TMPFILE], [], [], [[#include <fcntl.h>]])
 

keepalived/bfd/bfd_daemon.c

@@ -465,7 +465,7 @@ start_bfd_child(void)
 	/* Clear any child finder functions set in parent */
 	set_child_finder_name(NULL);
 
-	/* Create an independant file descriptor for the shared config file */
+	/* Create an independent file descriptor for the shared config file */
 	separate_config_file();
 
 	/* Child process part, write pidfile */

keepalived/bfd/bfd_scheduler.c

@@ -972,7 +972,7 @@ bfd_open_fd_in(const char *port)
 		return -1;
 	}
 
-	if ((fd_in = socket(AF_INET6, ai_in->ai_socktype, ai_in->ai_protocol)) == -1) {
+	if ((fd_in = socket(AF_INET6, ai_in->ai_socktype | SOCK_CLOEXEC, ai_in->ai_protocol)) == -1) {
 		sav_errno = errno;
 
 		freeaddrinfo(ai_in);
@@ -990,7 +990,7 @@ bfd_open_fd_in(const char *port)
 			return -1;
 		}
 
-		if ((fd_in = socket(AF_INET, ai_in->ai_socktype, ai_in->ai_protocol)) == -1) {
+		if ((fd_in = socket(AF_INET, ai_in->ai_socktype | SOCK_CLOEXEC, ai_in->ai_protocol)) == -1) {
 			log_message(LOG_ERR, "socket(AF_INET) error %d (%m)", errno);
 			freeaddrinfo(ai_in);
 			return -1;
@@ -1031,7 +1031,7 @@ read_local_port_range(uint32_t port_limits[2])
 	port_limits[0] = 49152;
 	port_limits[1] = 60999;
 
-	fd = open("/proc/sys/net/ipv4/ip_local_port_range", O_RDONLY);
+	fd = open("/proc/sys/net/ipv4/ip_local_port_range", O_RDONLY | O_CLOEXEC);
 	if (fd == -1)
 		return false;
 	len = read(fd, buf, sizeof(buf));
@@ -1068,7 +1068,7 @@ bfd_open_fd_out(bfd_t *bfd)
 	assert(bfd);
 	assert(bfd->fd_out == -1);
 
-	bfd->fd_out = socket(bfd->nbr_addr.ss_family, SOCK_DGRAM, IPPROTO_UDP);
+	bfd->fd_out = socket(bfd->nbr_addr.ss_family, SOCK_DGRAM | SOCK_CLOEXEC, IPPROTO_UDP);
 	if (bfd->fd_out == -1) {
 		log_message(LOG_ERR, "(%s) socket() error (%m)",
 			    bfd->iname);

keepalived/check/check_daemon.c

@@ -772,7 +772,7 @@ start_check_child(void)
 	/* Clear any child finder functions set in parent */
 	set_child_finder_name(NULL);
 
-	/* Create an independant file descriptor for the shared config file */
+	/* Create an independent file descriptor for the shared config file */
 	separate_config_file();
 
 	/* Child process part, write pidfile */

keepalived/check/check_ping.c

@@ -62,7 +62,7 @@ set_ping_group_range(bool set)
 	if (set == checked_ping_group_range)
 		return true;
 
-	fd = open(ping_group_range, O_RDWR);
+	fd = open(ping_group_range, O_RDWR | O_CLOEXEC);
 	if (fd == -1)
 		return false;
 	len = read(fd, buf, sizeof(buf));

keepalived/check/libipvs.c

@@ -218,7 +218,7 @@ dump_nl_msg(const char *msg, struct nl_msg *nlmsg)
 	const char *filename;
 
 	filename = make_tmp_filename("nlmsg.dmp");
-	fp = fopen(filename, "a");
+	fp = fopen(filename, "ae");
 	FREE_CONST(filename);
 
 	fprintf(fp, "\n%s\n\n", msg);

keepalived/core/config_notify.c

@@ -118,7 +118,7 @@ save_config(bool post, const char *process, void(*func)(FILE *))
 
 	sprintf(buf, "%s/keepalived_%s.%d.%u.%s", config_save_dir, process, our_pid, reload_num, post ? "post" : "pre");
 
-	file = fopen_safe(buf, "w");
+	file = fopen_safe(buf, "we");
 	if (!file) {
 		log_message(LOG_INFO, "Failed to open config_save file %s", buf);
 		return;

keepalived/core/global_data.c

@@ -586,7 +586,7 @@ open_dump_file(const char *default_file_name)
 			global_data->data_use_instance && global_data->instance_name ? global_data->instance_name : "",
 			dot);
 
-	fp = fopen_safe(full_file_name, "w");
+	fp = fopen_safe(full_file_name, "we");
 
 	if (!fp)
 		log_message(LOG_INFO, "Can't open dump file %s (%d: %s)",

keepalived/core/main.c

@@ -1163,7 +1163,7 @@ sigend(__attribute__((unused)) void *v, __attribute__((unused)) int sig)
 		}
 	}
 
-	efd = epoll_create(1);
+	efd = epoll_create1(EPOLL_CLOEXEC);
 	epoll_ctl(efd, EPOLL_CTL_ADD, signal_fd, &ev);
 
 	gettimeofday(&start_time, NULL);
@@ -1324,7 +1324,7 @@ update_core_dump_pattern(const char *pattern_str)
 	if (initialising)
 		orig_core_dump_pattern = MALLOC(CORENAME_MAX_SIZE);
 
-	fd = open ("/proc/sys/kernel/core_pattern", O_RDWR);
+	fd = open("/proc/sys/kernel/core_pattern", O_RDWR | O_CLOEXEC);
 
 	if (fd == -1 ||
 	    (initialising && read(fd, orig_core_dump_pattern, CORENAME_MAX_SIZE - 1) == -1) ||
@@ -1742,7 +1742,7 @@ set_debug_options(const char *options)
 static void
 report_distro(void)
 {
-	FILE *fp = fopen("/etc/os-release", "r");
+	FILE *fp = fopen("/etc/os-release", "re");
 	char buf[128];
 	const char * const var = "PRETTY_NAME=";
 	const size_t var_len = strlen(var);
@@ -1784,7 +1784,7 @@ read_config_opts(const char *filename)
 	if (stat(filename, &statbuf))
 		return NULL;
 
-	if ((fd = open(filename, O_RDONLY)) == -1) {
+	if ((fd = open(filename, O_RDONLY | O_CLOEXEC)) == -1) {
 		fprintf(stderr, "Failed to open %s\n", filename);
 		return NULL;
 	}
@@ -2214,7 +2214,7 @@ parse_cmdline(int argc, char **argv)
 			__set_bit(DONT_FORK_BIT, &debug);
 			__set_bit(NO_SYSLOG_BIT, &debug);
 			if (optarg && optarg[0]) {
-				int fd = open(optarg, O_WRONLY | O_APPEND | O_CREAT | O_NOFOLLOW, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+				int fd = open(optarg, O_WRONLY | O_APPEND | O_CREAT | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 				if (fd == -1) {
 					fprintf(stderr, "Unable to open config-test log file %s %d - %m\n", optarg, errno);
 					exit(EXIT_FAILURE);

keepalived/core/namespaces.c

@@ -275,7 +275,7 @@ set_namespaces(const char* net_namespace)
 	strcpy(netns_path, netns_dir);
 	strcat(netns_path, net_namespace);
 
-	fd = open(netns_path, O_RDONLY);
+	fd = open(netns_path, O_RDONLY | O_CLOEXEC);
 	if (fd == -1) {
 		log_message(LOG_INFO, "Failed to open %s", netns_path);
 		goto err;

keepalived/core/nftables.c

@@ -139,7 +139,7 @@ nl_socket_open(void)
 	}
 #endif
 
-	nl = mnl_socket_open(NETLINK_NETFILTER);
+	nl = mnl_socket_open2(NETLINK_NETFILTER, SOCK_CLOEXEC);
 	if (nl == NULL) {
 		log_message(LOG_INFO, "mnl_socket_open failed - %d", errno);
 
@@ -187,7 +187,7 @@ exchange_nl_msg(struct mnl_nlmsg_batch *batch)
 
 	if (prog_type == PROG_TYPE_VRRP) {
 		file_name = make_tmp_filename("nftrace");
-		fp = fopen(file_name, "a");
+		fp = fopen(file_name, "ae");
 		FREE_CONST(file_name);
 		unsigned char *p = mnl_nlmsg_batch_head(batch);
 		size_t i;
@@ -267,7 +267,7 @@ exchange_nl_msg_single(struct nlmsghdr *nlm, int (*cb_func)(const struct nlmsghd
 	FILE *fp;
 
 	filename = make_tmp_filename("nftrace");
-	fp = fopen(filename, "a");
+	fp = fopen(filename, "ae");
 	FREE_CONST(filename);
 
 	mnl_nlmsg_fprintf(fp, PTR_CAST(char, nlm), nlm->nlmsg_len, 0);
@@ -716,7 +716,7 @@ set_nf_ifname_type(void)
 	unsigned nft_version = 0;
 	int ifname_type;
 
-	fp = popen("nft -v 2>/dev/null", "r");
+	fp = popen("nft -v 2>/dev/null", "re");
 	if (fp) {
 		if (fgets(nft_ver_buf, sizeof(nft_ver_buf), fp)) {
 			if (!(p = strchr(nft_ver_buf, ' ')))

keepalived/core/reload_monitor.c

@@ -218,7 +218,7 @@ parse_datetime(const char *timestr, bool *date_specified)
 static void
 read_file(void)
 {
-	FILE *fp = fopen(global_data->reload_time_file, "r");
+	FILE *fp = fopen(global_data->reload_time_file, "re");
 	size_t len;
 	time_t reload_time;
 	char	time_buf[21];

keepalived/core/smtp.c

@@ -550,7 +550,7 @@ smtp_log_to_file(smtp_t *smtp)
 	email_t *email;
 
 	file_name = make_tmp_filename("smtp-alert.log");
-	fp = fopen_safe(file_name, "a");
+	fp = fopen_safe(file_name, "ae");
 	FREE_CONST(file_name);
 
 	if (fp) {

keepalived/core/snmp.c

@@ -23,6 +23,16 @@
 #include "config.h"
 
 #include <stdio.h>
+#if defined HAVE_CLOSE_RANGE_CLOEXEC
+#if !defined DEFINE_CLOSE_RANGE && !defined _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include <linux/close_range.h>
+#else
+#include <fcntl.h>
+#endif
+#include <unistd.h>
+
 #include "scheduler.h"
 #include "snmp.h"
 #include "logger.h"
@@ -456,9 +466,18 @@ snmp_unregister_mib(oid *myoid, size_t len)
 void
 snmp_agent_init(const char *snmp_socket_name, bool base_mib)
 {
+#ifndef HAVE_DECL_CLOSE_RANGE_CLOEXEC
+	uint64_t fds[2][16];
+	unsigned max_fd;
+#endif
+
 	if (snmp_running)
 		return;
 
+#ifndef HAVE_DECL_CLOSE_RANGE_CLOEXEC
+	get_open_fds(fds[0], sizeof(fds[0]) / sizeof(fds[0][0]));
+#endif
+
 	log_message(LOG_INFO, "Starting SNMP subagent");
 	netsnmp_enable_subagent();
 	snmp_disable_log();
@@ -509,6 +528,27 @@ snmp_agent_init(const char *snmp_socket_name, bool base_mib)
 	/* Set up the fd threads */
 	snmp_epoll_info(master);
 
+#ifdef HAVE_DECL_CLOSE_RANGE_CLOEXEC
+	/* This assumes that child processes should only have stdin, stdout and stderr open */
+	close_range(STDERR_FILENO + 1, ~0U, CLOSE_RANGE_CLOEXEC);
+#else
+	max_fd = get_open_fds(fds[1], sizeof(fds[1]) / sizeof(fds[1][0]));
+
+	for (size_t i = 0; i < sizeof(fds[0]) / sizeof(fds[0][0]) && i * 64 <= max_fd; i++) {
+		if (fds[0][i] != fds[1][i]) {
+			uint64_t fds_diff = fds[0][i] ^ fds[1][i], bit_mask;
+			unsigned j;
+			for (bit_mask = 1, j = i * 64; j < (i + 1) * 64 && j <= max_fd; j++, bit_mask <<= 1) {
+				if (j <= STDERR_FILENO)
+					continue;
+
+				if (fds_diff & bit_mask)
+					fcntl(j, F_SETFD, fcntl(j, F_GETFD) | FD_CLOEXEC);
+			}
+		}
+	}
+#endif
+
 	snmp_running = true;
 }
 

keepalived/core/track_process.c

@@ -293,7 +293,7 @@ read_procs(list_head_t *processes)
 		if (vrrp_data->vrrp_use_process_cmdline) {
 			snprintf(cmdline, sizeof(cmdline), "/proc/%.*s/cmdline", PID_MAX_DIGITS, ent->d_name);
 
-			if ((fd = open(cmdline, O_RDONLY)) == -1)
+			if ((fd = open(cmdline, O_RDONLY | O_CLOEXEC)) == -1)
 				continue;
 
 			/* Read max name len + null byte + 1 extra char */
@@ -307,7 +307,7 @@ read_procs(list_head_t *processes)
 		if (vrrp_data->vrrp_use_process_comm) {
 			snprintf(cmdline, sizeof(cmdline), "/proc/%.*s/stat", PID_MAX_DIGITS, ent->d_name);
 
-			if ((fd = open(cmdline, O_RDONLY)) == -1)
+			if ((fd = open(cmdline, O_RDONLY | O_CLOEXEC)) == -1)
 				continue;
 
 			len = read(fd, stat_buf, sizeof(stat_buf) - 1);
@@ -432,7 +432,7 @@ check_process(pid_t pid, char *comm, tracked_process_instance_t *tpi)
 		if (vrrp_data->vrrp_use_process_cmdline) {
 			snprintf(cmdline, sizeof(cmdline), "/proc/%d/cmdline", pid);
 
-			if ((fd = open(cmdline, O_RDONLY)) == -1) {
+			if ((fd = open(cmdline, O_RDONLY | O_CLOEXEC)) == -1) {
 #ifdef _TRACK_PROCESS_DEBUG_
 				if (do_track_process_debug_detail)
 					log_message(LOG_INFO, "check_process failed to open %s, errno %d", cmdline, errno);
@@ -461,7 +461,7 @@ check_process(pid_t pid, char *comm, tracked_process_instance_t *tpi)
 		if (vrrp_data->vrrp_use_process_comm) {
 			snprintf(cmdline, sizeof(cmdline), "/proc/%d/comm", pid);
 
-			if ((fd = open(cmdline, O_RDONLY)) == -1) {
+			if ((fd = open(cmdline, O_RDONLY | O_CLOEXEC)) == -1) {
 #ifdef _TRACK_PROCESS_DEBUG_
 				if (do_track_process_debug_detail)
 					log_message(LOG_INFO, "check_process failed to open %s, errno %d", cmdline, errno);

keepalived/trackers/track_file.c

@@ -497,7 +497,7 @@ track_file_end_handler(void)
 		if (!reload && !__test_bit(CONFIG_TEST_BIT, &debug) &&
 		    (ret || track_file_init == TRACK_FILE_OVERWRITE)) {	// the file doesn't exist or we want to overwrite it
 			/* Write the value to the file */
-			if ((tf = fopen_safe(track_file->file_path, "w"))) {
+			if ((tf = fopen_safe(track_file->file_path, "we"))) {
 				fprintf(tf, "%d\n", track_file_init_value);
 				fclose(tf);
 			}
@@ -809,7 +809,7 @@ process_track_file(tracked_file_t *tfile, bool init)
 	int fd;
 	ssize_t len;
 
-	if ((fd = open(tfile->file_path, O_RDONLY | O_NONBLOCK)) != -1) {
+	if ((fd = open(tfile->file_path, O_RDONLY | O_NONBLOCK | O_CLOEXEC)) != -1) {
 		len = read(fd, buf, sizeof(buf) - 1);
 		close(fd);
 		if (len > 0) {

keepalived/vrrp/vrrp.c

@@ -1336,7 +1336,7 @@ vrrp_build_vrrp_v2(vrrp_t *vrrp, char *buffer)
 	struct in6_addr *ip6arr;
 	ip_address_t *ip_addr;
 
-	/* Family independant */
+	/* Family independent */
 	hd->vers_type = (VRRP_VERSION_2 << 4) | VRRP_PKT_ADVERT;
 	hd->vrid = vrrp->vrid;
 	hd->priority = vrrp->effective_priority;
@@ -1388,7 +1388,7 @@ vrrp_build_vrrp_v3(vrrp_t *vrrp, char *buffer, struct iphdr *ip)
 	ip_address_t *ip_addr;
 	ipv4_phdr_t ipv4_phdr;
 
-	/* Family independant */
+	/* Family independent */
 	hd->vers_type = (VRRP_VERSION_3 << 4) | VRRP_PKT_ADVERT;
 	hd->vrid = vrrp->vrid;
 	hd->priority = vrrp->effective_priority;
@@ -4719,7 +4719,7 @@ check_vmac_conflicts(void)
 	ip_address_t *vip, *vip1;
 	list_head_t *vip_list, *vip_list1;
 
-	/* Now check that independant vrrp instances (i.e. not in a sync group)
+	/* Now check that independent vrrp instances (i.e. not in a sync group)
 	 * are not trying to use the same VMAC (macvlan) interface. */
 	list_for_each_entry(vrrp, &vrrp_data->vrrp, e_list) {
 		list_for_each_entry(vrrp1, &vrrp_data->vrrp, e_list) {