Package: cron
Version: 3.0pl1-100
Severity: normal

While looking at the script, I couldn't help noticing what I believe to be a
race condition with potentially harmful conseqences.

In the beginning, you have:

    lockfile-touch $LOCKFILE &
    LOCKTOUCHPID="$!"

Then you do a number of things which can potentially take forever, and in
the end you have:

if [ -x /usr/bin/lockfile-create ] ; then
    kill $LOCKTOUCHPID
    lockfile-remove $LOCKFILE
fi

There are several things wrong with this:

#1. lockfile-touch may die in the meantime (for whatever reason, OOM is one
example that comes to mind). Its PID may be reused, so in the end, you may
end up killing the wrong process.

Also, if lockfile-touch dies, a second instance of the standard script may
obtain a lock on the lockfile and execute in parallel with the previous one,
and so on.

#2. lockfile-create may be uninstalled while the script is being executed,
so you start a lockfile-touch process in the beginning but don't kill it at
the end. The process may stick around forever. Better test for
[ -n "$LOCKTOUCHPID" ], if you must do locking this way.

Regarding #1, I think it would be a far better approach to launch the script
so that it itself holds a lock on the lockfile. chpst(8) from the runit
package could be used for that, like this:

#!/bin/sh
LOCKFILE=/var/lock/cron.daily.lock
[ "$1" != "have-lock" ] || exec chpst -L "$LOCKFILE" "$0" "have-lock"
# rest of script

This results in chpst getting a lock for $LOCKFILE and then exec()ing the
daily standard script again, with the lockfile open on one of its FDs and
the lock held. The lock automatically expires when the script exits. No need
to keep a "lockfile daemon" running or to use fragile heuristics to kill
random processes at the end. :)

chpst exits with an error if it can't get the lock.

I'm sure lockfile-progs could be enhanced to include similar (trivial)
functionality if you don't want to rely on chpst.

Andras

-- System Information:
Debian Release: lenny/sid
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.22.18-vs2.2.0.6-arcadia (SMP w/2 CPU cores)
Locale: LANG=C, LC_CTYPE=hu_HU (charmap=ISO-8859-2)
Shell: /bin/sh linked to /bin/bash

Versions of packages cron depends on:
ii  adduser                      3.105       add and remove users and groups
ii  debianutils                  2.25.2      Miscellaneous utilities specific t
ii  libc6                        2.7-6       GNU C Library: Shared libraries
ii  libpam0g                     0.99.7.1-5  Pluggable Authentication Modules l
ii  libselinux1                  2.0.15-2+b1 SELinux shared libraries
ii  lsb-base                     3.1-24      Linux Standard Base 3.1 init scrip

Versions of packages cron recommends:
ii  qmail [mail-tra 1.03-43+kqmail20051105-7 Secure, reliable, efficient, simpl

-- no debconf information

-- 
                 Andras Korn <korn at chardonnay.math.bme.hu>
                 <http://chardonnay.math.bme.hu/~korn/> QOTD:
             Ok, I pulled the pin. Now what? Where are you going?



-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to