From: Tomas Mraz <tmraz@fedoraproject.org>
Date: Mon, 8 Jul 2019 08:57:52 +0000 (+0200)
Subject: getdtablesize() can return very high values in containers
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=584911514ce6aa2f16e1d79431bac816ea62cb2c;p=cronie

getdtablesize() can return very high values in containers

Avoid closing hundreds of millions descriptors or allocating
huge arrays by maxing the fd number to MAX_CLOSE_FD.
See rhbz#1723106
---

diff --git a/src/database.c b/src/database.c
index 66609d1..1986620 100644
--- a/src/database.c
+++ b/src/database.c
@@ -48,9 +48,6 @@
 #include "globals.h"
 #include "pathnames.h"
 
-#define TMAX(a,b) ((a)>(b)?(a):(b))
-#define TMIN(a,b) ((a)<(b)?(a):(b))
-
 /* size of the event structure, not counting name */
 #define EVENT_SIZE  (sizeof (struct inotify_event))
 
diff --git a/src/do_command.c b/src/do_command.c
index a15f9be..45c49f7 100644
--- a/src/do_command.c
+++ b/src/do_command.c
@@ -238,7 +238,7 @@ static int child_process(entry * e, char **jobenv) {
 		 */
 		{
 			char *shell = env_get("SHELL", jobenv);
-			int fd, fdmax = getdtablesize();
+			int fd, fdmax = TMIN(getdtablesize(), MAX_CLOSE_FD);
 
 			/* close all unwanted open file descriptors */
 			for(fd = STDERR + 1; fd < fdmax; fd++) {
diff --git a/src/macros.h b/src/macros.h
index 74ffa26..cba5fb2 100644
--- a/src/macros.h
+++ b/src/macros.h
@@ -61,6 +61,7 @@
 #define MAX_USER_ENVS	1000	/* maximum environment variables in user's crontab */
 #define MAX_USER_ENTRIES 1000	/* maximum crontab entries in user's crontab */
 #define MAX_GARBAGE	32768	/* max num of chars of comments and whitespaces between entries */
+#define MAX_CLOSE_FD	10000	/* max fd num to close when spawning a child process */
 
 				/* NOTE: these correspond to DebugFlagNames,
 				 *	defined below.
@@ -129,6 +130,9 @@
 #define	LAST_DOW	7
 #define	DOW_COUNT	(LAST_DOW - FIRST_DOW + 1)
 
+#define TMAX(a,b) ((a)>(b)?(a):(b))
+#define TMIN(a,b) ((a)<(b)?(a):(b))
+
 /*
  * Because crontab/at files may be owned by their respective users we
  * take extreme care in opening them.  If the OS lacks the O_NOFOLLOW
diff --git a/src/popen.c b/src/popen.c
index badddb6..4397264 100644
--- a/src/popen.c
+++ b/src/popen.c
@@ -81,12 +81,19 @@ FILE *cron_popen(char *program, const char *type, struct passwd *pw, char **jobe
 	if (!pids) {
 		if ((fds = getdtablesize()) <= 0)
 			return (NULL);
+		if (fds > MAX_CLOSE_FD)
+			fds = MAX_CLOSE_FD; /* avoid allocating too much memory */
 		if (!(pids = (PID_T *) malloc((u_int) ((size_t)fds * sizeof (PID_T)))))
 			return (NULL);
 		memset((char *) pids, 0, (size_t)fds * sizeof (PID_T));
 	}
 	if (pipe(pdes) < 0)
 		return (NULL);
+	if (pdes[0] >= fds || pdes[1] >= fds) {
+		(void) close(pdes[0]);
+		(void) close(pdes[1]);
+		return NULL;
+	}
 
 	/* break up string into pieces */
 	for (argc = 0, cp = program; argc < MAX_ARGS; cp = NULL)
@@ -145,14 +152,16 @@ FILE *cron_popen(char *program, const char *type, struct passwd *pw, char **jobe
 	}
 	/* parent; assume fdopen can't fail...  */
 	if (*type == 'r') {
+		fd = pdes[0];
 		iop = fdopen(pdes[0], type);
 		(void) close(pdes[1]);
 	}
 	else {
+		fd = pdes[1];
 		iop = fdopen(pdes[1], type);
 		(void) close(pdes[0]);
 	}
-	pids[fileno(iop)] = pid;
+	pids[fd] = pid;
 
   pfree:
 	return (iop);
@@ -168,7 +177,8 @@ int cron_pclose(FILE * iop) {
 	 * pclose returns -1 if stream is not associated with a
 	 * `popened' command, or, if already `pclosed'.
 	 */
-	if (pids == NULL || pids[fdes = fileno(iop)] == 0L)
+	fdes = fileno(iop);
+	if (pids == NULL || fdes >= fds || pids[fdes] == 0L)
 		return (-1);
 	(void) fclose(iop);