On 10/23/17 09:05, Jeremie Courreges-Anglas wrote:
On Fri, Oct 20 2017, "Todd C. Miller" <[email protected]> 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.
Good catch, indeed.
I have commited the cron part.
- 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.
I guess it is ok to make cronSock global. closefrom() is good if
you have obvious situations. In more complex scenarios like this, I
prefer close() since it shows more clearly what happens.
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;
ok friehm
