Re: cron: don't close crontab_fd twice

2018-02-04 Thread Theo Buehler
On Sun, Feb 04, 2018 at 07:19:57PM -0700, Todd C. Miller wrote:
> 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().

That's much nicer. ok



Re: cron: don't close crontab_fd twice

2018-02-04 Thread 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.c25 Oct 2017 17:08:58 -  1.36
+++ usr.sbin/cron/database.c5 Feb 2018 02:18:37 -
@@ -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 -  1.28
+++ usr.sbin/cron/funcs.h   5 Feb 2018 02:18:56 -
@@ -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.c7 Jun 2017 23:36:43 -   1.20
+++ usr.sbin/cron/user.c5 Feb 2018 02:17:51 -
@@ -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);
 }