bugs in GetLock() - lockname collisions

[Eric Sorenson]

I tracked down a bug today that has probably been affecting people for a while. 
The symptom is that an action in the middle of execution will not be run because
a spurious lock exists.

The following code snippet will expose the bug:

  control:
    actionsequence = ( directories )

  directories:

    /var/tmp/maildir mode=700 owner=root group=root
    /var/tmp/maildir/cur mode=700 owner=root group=root
    /var/tmp/maildir/new mode=700 owner=root group=root
    /var/tmp/maildir/tmp mode=700 owner=root group=root

The output looks like:

  MakePath(/var/tmp/maildir)
  MakePath(/var/tmp/maildir/cur)
  MakePath(/var/tmp/maildir/new)
  cfengine:: Nothing scheduled for directories.directories (0/1 minutes elapsed)
  MakePath(/var/tmp/maildir/tmp)
  
The problem is in the way cfengine creates the lockname for these actions.
It does the following in locks.c about line 200:

for (sp = operator; *sp != '\0'; sp++)
   {
   sum += (int)*sp;
   }

And then does the same thing for 'operand'. (operator is the action
being performed, and operand the pathname that we're working on)
This is very prone to namespace collisions; for instance 'cur' and
'new' both end up with the same value for sum.

Since there's already a nice non-colliding algorithm in use for 
SplayTime, the attached patch just implements it instead.

There is another odd problem though, that I was not able to patch.
If you look at the lockname for a 'directories:' action, it's something 
like:

    lock.cfagent_conf..directories.directories_3492

This is the value of 'CFLOCK', which is consists of (80-col formatted):

    snprintf(CFLOCK,CF_BUFSIZE,"lock.%.100s.%.40s.%s.%.100s_%d",
        VCANONICALFILE, host,CanonifyName(operator),
        CanonifyName(operand),sum);

So 'host' is empty, and both operator and operand are set to 'directories'.
The above checksum problem wouldn't even be looked at if operand were actually
the pathname of the directory in question (and, of course, that pathname was
less than 100 characters -- so it's still useful to have the 'sum' fix). I
stepped through this in gdb but couldn't see why this was happening. Both
CFLOCK and CFLAST do the same thing (this is locks.c:214).

-- 

 - Eric Sorenson - Explosive Networking - http://eric.explosive.net -

cfagent-locks-sum.patch
Description: Text document