[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: cron: do these bugs warrant RC?



On 12/16/2010 08:51 PM, Julien Cristau wrote:
> On Fri, Dec  3, 2010 at 23:33:27 +0100, Christian Kastner wrote:
> 
>> Hello RT,
>>
>> On 11/29/2010 08:28 PM, Christian Kastner wrote:
>>> If you agree with the changes, we would prepare an upload.
>> haven't heard back yet, so I thought I'd ping again.
>>
> Please send the actual diff, not just the changelog entry.

Full debdiff attached.

Note that the fix for #604181 (in do_command.c) indicates that the
previous code included the fix, but commented out. That part of the code
came from a large patch in the BTS, and when I included it I
misinterpreted the warning (I thought it applied to another context of
the patch). It appears that the original patch author ran into the same
issue with vfork().

The attached debdiff contains an additional fix (one-line), for a broken
example in crontab(5).

Thanks,
Christian
diff -u cron-3.0pl1/do_command.c cron-3.0pl1/do_command.c
--- cron-3.0pl1/do_command.c
+++ cron-3.0pl1/do_command.c
@@ -249,13 +249,13 @@
 
 	/* fork again, this time so we can exec the user's command.
 	 */
-	switch (vfork()) {
+	switch (fork()) {
 	case -1:
-		log_it("CRON",getpid(),"error","can't vfork");
+		log_it("CRON",getpid(),"error","can't fork");
 		exit(ERROR_EXIT);
 		/*NOTREACHED*/
 	case 0:
-		Debug(DPROC, ("[%d] grandchild process Vfork()'ed\n",
+		Debug(DPROC, ("[%d] grandchild process fork()'ed\n",
 			      getpid()))
 
 		/* write a log message.  we've waited this long to do it
@@ -299,7 +299,7 @@
 		/* close the pipe we just dup'ed.  The resources will remain.
 		 */
 		close(stdin_pipe[READ_PIPE]);
-		// Don't do this: fclose(tmpout);
+		fclose(tmpout);
 
 		/* set our login universe.  Do this in the grandchild
 		 * so that the child can invoke /usr/lib/sendmail
diff -u cron-3.0pl1/crontab.5 cron-3.0pl1/crontab.5
--- cron-3.0pl1/crontab.5
+++ cron-3.0pl1/crontab.5
@@ -247,7 +247,7 @@
 23 0-23/2 * * * echo "run 23 minutes after midn, 2am, 4am ..., everyday"
 5 4 * * sun     echo "run at 5 after 4 every sunday"
 # Run on every second Saturday of the month
-0 4 8-15 * *    test $(date +\%u) -eq 6 && echo "2nd Saturday"
+0 4 8-14 * *    test $(date +%u) -eq 6 && echo "2nd Saturday"
 .fi
 .SH EXAMPLE SYSTEM CRON FILE
 This has the username field, as used by /etc/crontab.
diff -u cron-3.0pl1/popen.c cron-3.0pl1/popen.c
--- cron-3.0pl1/popen.c
+++ cron-3.0pl1/popen.c
@@ -100,7 +100,7 @@
 #endif
 
 	iop = NULL;
-	switch(pid = vfork()) {
+	switch(pid = fork()) {
 	case -1:			/* error */
 		(void)close(pdes[0]);
 		(void)close(pdes[1]);
diff -u cron-3.0pl1/debian/cron.init cron-3.0pl1/debian/cron.init
--- cron-3.0pl1/debian/cron.init
+++ cron-3.0pl1/debian/cron.init
@@ -5,8 +5,8 @@
 # Provides:          cron
 # Required-Start:    $remote_fs $syslog $time
 # Required-Stop:     $remote_fs $syslog $time
-# Should-Start:      slapd autofs ypbind nscd
-# Should-Stop:       slapd autofs ypbind nscd
+# Should-Start:      $named slapd autofs ypbind nscd nslcd
+# Should-Stop:       $named slapd autofs ypbind nscd nslcd
 # Default-Start:     2 3 4 5
 # Default-Stop:
 # Short-Description: Regular background program processing daemon
diff -u cron-3.0pl1/debian/changelog cron-3.0pl1/debian/changelog
--- cron-3.0pl1/debian/changelog
+++ cron-3.0pl1/debian/changelog
@@ -1,3 +1,21 @@
+cron (3.0pl1-116) unstable; urgency=low
+
+  * do_command.c, popen.c:
+    - Use fork() instead of vfork().
+  * do_command.c:
+    - Close an unused stream in the fork()ed child prior to exec'ing the
+      user's command, thereby avoiding an fd leak. Closes: #604181, LP: #665912
+  * crontab.5:
+    - Fixed the example demonstrating how to run a job on a certain weekday of
+      the month (date range was off-by-one). Also, the same example contained
+      a superfluous escape, resulting in wrong output. Closes: #606325
+  * cron.init:
+    - Added $named to Should-Start, in case @reboot jobs need DNS resolution.
+      Closes: #602903
+    - Added nslcd to Should-Start. LP: #27520 
+
+ -- Christian Kastner <debian@kvr.at>  Wed, 10 Nov 2010 21:13:19 +0100
+
 cron (3.0pl1-115) unstable; urgency=high
 
   * cron.c: set LC_COLLATE to 'C' in order to properly work when locales
@@ -124,7 +142,7 @@
     information, warn that variables from /etc/environment will be ignored.
     Closes: #543895, #580942
     - Add autofs and NIS+ to Should-Starts, because especially NIS+ must be run
-    before cron, if present. Closes: #511757
+    before cron, if present. Closes: #512757
     - init script should be stopped in runlevel 0 and 6, too
   [ Javier Fernandez-Sanguino ]
   * do_command.c: Revert change so that mails are not sent if MAILTO

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: