Re: cron: don't close crontab_fd twice

[Todd C. Miller]

On Mon, 05 Feb 2018 09:21:32 +1300, Theo Buehler wrote:

> The load_user() function gets a file descriptor from process_crontab().
> It fdopen()s it directly and fclose()s the resulting stream. Then
> process_crontab() closes the stream a second time before exiting.
>
> Since crontab_fd is not load_user()'s descriptor, let's dup it before
> opening the stream.

Alternately, we could just pass in a FILE * to load_user().

 - todd

Index: usr.sbin/cron/database.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/database.c,v
retrieving revision 1.36
diff -u -p -u -r1.36 database.c
--- usr.sbin/cron/database.c    25 Oct 2017 17:08:58 -0000      1.36
+++ usr.sbin/cron/database.c    5 Feb 2018 02:18:37 -0000
@@ -170,9 +170,10 @@ process_crontab(int dfd, const char *una
                struct stat *statbuf, cron_db *new_db, cron_db *old_db)
 {
        struct passwd *pw = NULL;
-       int crontab_fd = -1;
+       FILE *crontab_fp = NULL;
        user *u, *new_u;
        mode_t tabmask, tabperm;
+       int fd;
 
        /* Note: pw must remain NULL for system crontab (see below). */
        if (fname[0] != '/' && (pw = getpwnam(uname)) == NULL) {
@@ -182,16 +183,20 @@ process_crontab(int dfd, const char *una
                goto next_crontab;
        }
 
-       crontab_fd = openat(dfd, fname,
-           O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC);
-       if (crontab_fd < 0) {
+       fd = openat(dfd, fname, O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC);
+       if (fd < 0) {
                /* crontab not accessible?
                 */
                syslog(LOG_ERR, "(%s) CAN'T OPEN (%s)", uname, fname);
                goto next_crontab;
        }
+       if (!(crontab_fp = fdopen(fd, "r"))) {
+               syslog(LOG_ERR, "(%s) FDOPEN (%m)", fname);
+               close(fd);
+               goto next_crontab;
+       }
 
-       if (fstat(crontab_fd, statbuf) < 0) {
+       if (fstat(fileno(crontab_fp), statbuf) < 0) {
                syslog(LOG_ERR, "(%s) FSTAT FAILED (%s)", uname, fname);
                goto next_crontab;
        }
@@ -233,7 +238,7 @@ process_crontab(int dfd, const char *una
                syslog(LOG_INFO, "(%s) RELOAD (%s)", uname, fname);
        }
 
-       new_u = load_user(crontab_fd, pw, fname);
+       new_u = load_user(crontab_fp, pw, fname);
        if (new_u != NULL) {
                /* Insert user into the new database and remove from old. */
                new_u->mtime = statbuf->st_mtim;
@@ -249,7 +254,7 @@ process_crontab(int dfd, const char *una
        }
 
  next_crontab:
-       if (crontab_fd >= 0) {
-               close(crontab_fd);
+       if (crontab_fp != NULL) {
+               fclose(crontab_fp);
        }
 }
Index: usr.sbin/cron/funcs.h
===================================================================
RCS file: /cvs/src/usr.sbin/cron/funcs.h,v
retrieving revision 1.28
diff -u -p -u -r1.28 funcs.h
--- usr.sbin/cron/funcs.h       14 Nov 2015 13:09:14 -0000      1.28
+++ usr.sbin/cron/funcs.h       5 Feb 2018 02:18:56 -0000
@@ -48,7 +48,7 @@ char          *env_get(char *, char **),
                **env_copy(char **),
                **env_set(char **, char *);
 
-user           *load_user(int, struct passwd *, const char *),
+user           *load_user(FILE *, struct passwd *, const char *),
                *find_user(cron_db *, const char *);
 
 entry          *load_entry(FILE *,
Index: usr.sbin/cron/user.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/user.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 user.c
--- usr.sbin/cron/user.c        7 Jun 2017 23:36:43 -0000       1.20
+++ usr.sbin/cron/user.c        5 Feb 2018 02:17:51 -0000
@@ -58,19 +58,14 @@ parse_error(const char *msg)
 }
 
 user *
-load_user(int crontab_fd, struct passwd        *pw, const char *name)
+load_user(FILE *file, struct passwd *pw, const char *name)
 {
        char envstr[MAX_ENVSTR];
-       FILE *file;
        user *u;
        entry *e;
        int status, save_errno;
        char **envp = NULL, **tenvp;
 
-       if (!(file = fdopen(crontab_fd, "r"))) {
-               syslog(LOG_ERR, "(%s) FDOPEN (%m)", name);
-               return (NULL);
-       }
        CrontabFilename = name;
        LineNumber = 0;
 
@@ -134,6 +129,5 @@ load_user(int crontab_fd, struct passwd     
  done:
        if (envp != NULL)
                env_free(envp);
-       fclose(file);
        return (u);
 }