Bug#400301: [Pkg-dspam-misc] Bug#400301: dspam should not be run with hash drivers in daemon mode

[Daniel Kahn Gillmor]

On November 25, abuse at hispalinux.es said:

> We should probably clarify it with the dspam developers, but dspam
> seems to be currently stalled, for one reason or another.
> 
> Any idea why?

If you mean upstream dspam development seems to be stalled, it seems
that way to me, too.  Jonathan Zdziarski has been quiet lately on the
users and developers list as well.  I think he may have picked up
another job or something.  The only thing recently updated on his
personal website (http://www.zdziarski.com/) in months is his stock
picks.

In the meantime, i've written a short test program (attached below)
that tries to emulate the fcntl locking used by dspam, which shows
pretty clearly that it doesn't work between threads:

[0 dkg at squeak testlocking]$ ./testlocking 2>&1 | ts
Nov 25 10:54:39 Getting the lock in thread XX A
Nov 25 10:54:39 I've got the lock in thread XX A (fd: 3)
Nov 25 10:54:40 Getting the lock in thread XX B
Nov 25 10:54:40 I've got the lock in thread XX B (fd: 4)
Nov 25 10:54:41 Releasing the lock in thread XX A
Nov 25 10:54:41 Lock freed in thread XX A
Nov 25 10:54:41 Finishing thread XX A
Nov 25 10:54:42 Releasing the lock in thread XX B
Nov 25 10:54:42 Lock freed in thread XX B
Nov 25 10:54:42 Finishing thread XX B
[0 dkg at squeak testlocking]$ 

So if there's any control that keeps threads separate between
usernames, it's not being done successfully with fcntl locking.

i note that src/daemon.c seems to wrap each call to process_users in a
pthread_mutex_lock(), which might be an alternate (non-fcntl) method
of controlling access to the storage drivers.  In particular, it locks
a pooled connection from thread context's storage driver's context
TTX->DTX->connections[i]->lock.  Since the hash storage driver should
only have one pooled "connection" to the data, that might be
acceptable.  I haven't audited the code completely to ensure that all
access to the hash driver happens within the lock, though.

However src/daemon.c also (outside the lock) calls initialize_atx()
from src/shared_agent.c.  initialize_atx() can potentially call
exit(EXIT_FAILURE) if getpwnam_r() fails.  Calling exit() (if i
understand correctly), would immediately kill all threads, including
any thread which might be holding the lock while accessing a hashdb,
which could result in corruption of that hashdb.  This could be a
problem for any of the storage driver backends, actually, unless
they're using explicit transaction-wrapped database access.

once a hashdb is corrupted, the next time a dspam thread tries to
access it, the daemon itself may well crash, since the hashdb is an
mmap()'ed structure.  If the daemon crashes, any other hashes open
concurrently may also become corrupt, leading to a cascading chain of
failures.  yuck.

I haven't found any other instances of exit() that might be called
from the daemon, but i'm not sure that my audit was completely
exhaustive.

However we decide to handle the hash/daemon combination, i still think
it would be useful for the storage drivers to be able to report their
own threadsafety, and for the daemon to refuse to run if the report
was missing or negative.

	--dkg

-------------- next part --------------
A non-text attachment was scrubbed...
Name: testlocking.c
Type: text/x-csrc
Size: 2943 bytes
Desc: example program to test fcntl locking as used in dspam
Url : http://lists.alioth.debian.org/pipermail/pkg-dspam-misc/attachments/20061125/ae3faa95/testlocking.c