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 66609d13d2a6973186bbc9a5a0687d878debd83f..1986620a4133757dfefeb11c87d5b44b754b37ba 100644 (file)
--- 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 a15f9be26c7d238c7ced9e2f5ce8cac9048f953c..45c49f72db4ee76275fd9315b16e3252a4640862 100644 (file)
--- 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 74ffa26c610f40a55f914927aefaea9570ca49b4..cba5fb2d3f7ef826ac4b90bacf7046b28aa0d558 100644 (file)
--- 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 badddb62ac638bdfccf1642334337ab70f0cfe96..43972644c25c188e0971c38b00918abbcbd8abc0 100644 (file)
--- 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);