On Fri, Oct 20 2017, "Todd C. Miller" <todd.mil...@courtesan.com> wrote:
> On Fri, 20 Oct 2017 16:25:32 +0200, Florian Riehm wrote:
>
>> cron(8) opens /var/run/cron.sock for communication with crontab(1).
>> The forked cronjobs have the socked still open.
>> This prevents restarting cron while a job is running:
>> (CRON) DEATH (already running)
>> 
>> I think cron's children should not inherit sockets.
>
> There is a similar issue with at jobs.  Here's a comprehensive diff.

Good catch.  The cron part ought to be committed now, but I think
atrun.c needs more love.

>  - todd
>
> Index: usr.sbin/cron/atrun.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/cron/atrun.c,v
> retrieving revision 1.46
> diff -u -p -u -r1.46 atrun.c
> --- usr.sbin/cron/atrun.c     8 Jun 2017 16:23:39 -0000       1.46
> +++ usr.sbin/cron/atrun.c     20 Oct 2017 15:09:57 -0000
> @@ -283,6 +283,9 @@ run_job(const atjob *job, int dfd, const
>               return;
>       }
>  
> +     /* close fds opened by the parent (e.g. cronSock) */
> +     closefrom(3);
> +

That doesn't work.  When here, we hold cronSock and dfd (should be 3 and
4).  We need dfd to get fd (should be 5), thus we can't just use
"closefrom(3)".  The straightest way IMO is to close what we should
close (including dfd), but this means making cronSock a global.

I would also propose sprinkling more O_CLOEXEC magic but that can be in
another diff.

Thoughts?


Index: atrun.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/cron/atrun.c,v
retrieving revision 1.46
diff -u -p -r1.46 atrun.c
--- atrun.c     8 Jun 2017 16:23:39 -0000       1.46
+++ atrun.c     23 Oct 2017 03:21:20 -0000
@@ -283,6 +283,10 @@ run_job(const atjob *job, int dfd, const
                return;
        }
 
+       /* Close fds opened by the parent. */
+       close(cronSock);
+       close(dfd);
+
        /*
         * We don't want the main cron daemon to wait for our children--
         * we will do it ourselves via waitpid().
Index: cron.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.76
diff -u -p -r1.76 cron.c
--- cron.c      7 Jun 2017 23:36:43 -0000       1.76
+++ cron.c      23 Oct 2017 02:10:54 -0000
@@ -60,7 +60,6 @@ static        int     open_socket(void);
 
 static volatile sig_atomic_t   got_sigchld;
 static time_t                  timeRunning, virtualTime, clockTime;
-static int                     cronSock;
 static long                    GMToff;
 static cron_db                 *database;
 static at_db                   *at_database;
@@ -68,6 +67,7 @@ static        double                  batch_maxload = BATCH_MA
 static int                     NoFork;
 static time_t                  StartTime;
        gid_t                   cron_gid;
+       int                     cronSock;
 
 static void
 usage(void)
Index: globals.h
===================================================================
RCS file: /d/cvs/src/usr.sbin/cron/globals.h,v
retrieving revision 1.14
diff -u -p -r1.14 globals.h
--- globals.h   7 Jun 2017 23:36:43 -0000       1.14
+++ globals.h   23 Oct 2017 02:03:18 -0000
@@ -18,5 +18,6 @@
  */
 
 extern gid_t   cron_gid;
+extern int     cronSock;
 extern int     LineNumber;
 extern char    *__progname;


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to