After some attempt to repair the current code with minimal changes I started over with my chainsaw and cut lock_mtab() into peaces. There are too many aspects tried to be handled simultaneously with the current lock_mtab() implementation. This seems extremely error prone.
So I tried to split it up into separate small procedures. Each of them fits on a single screen and performs an obvious task. This should help to avoid unexpected side effects. I also tried to minimize the communication via static variables. Instead of publishing a long patch, I directly post a cut from my current code to discuss, if it is usable this way. Dieter. /* Updating mtab ----------------------------------------------*/ /* This should go to path.h. Unfortunately I can't see how to * construct it from MOUNTED, to get it to the same directory */ static const char* MOUNTLOCK_LINKTARGET = ".mtab.lock"; /* Record created lockfile name and open file handle */ static const char* lock_file_created = NULL; static int lock_file_desc = -1; /* Ensure that the lock is released if we are interrupted. */ extern char *strsignal(int sig); /* not always in <string.h> */ static void handler (int sig) { die(EX_USER, "%s", strsignal(sig)); } static void setlkw_timeout (int sig) { /* nothing, fcntl will fail anyway */ } static void setup_signals() { /* Flag to indicate that signals have been set up. */ static int signals_have_been_setup = 0; if (signals_have_been_setup) return; int sig = 0; struct sigaction sa; sa.sa_handler = handler; sa.sa_flags = 0; sigfillset (&sa.sa_mask); while (sigismember (&sa.sa_mask, ++sig) != -1 && sig != SIGCHLD) { if (sig == SIGALRM) sa.sa_handler = setlkw_timeout; else sa.sa_handler = handler; sigaction (sig, &sa, (struct sigaction *) 0); } signals_have_been_setup = 1; } /* Remove lock file. */ void unlock_mtab (void) { if (lock_file_created) { unlink (lock_file_created); lock_file_created = NULL; } if(lock_file_desc>=0) { close(lock_file_desc); lock_file_desc = -1; } } /* * Try to open the global lockfile * and establish a file lock on it. * * Return a file handle that holds the flock * or die */ static int open_lock_file() { /* Either open an existing lockfile or silently * create a new one, which will persist for ever. */ int fd = open(MOUNTLOCK_LINKTARGET, O_WRONLY|O_CREAT, 0600); if(fd<0) { int errsv = errno; die(EX_FILEIO, _("Can't open lock file %s: %s\n"), MOUNTLOCK_LINKTARGET, strerror (errsv)); } struct flock flock; flock.l_type = F_WRLCK; flock.l_whence = SEEK_SET; flock.l_start = 0; flock.l_len = 0; /* Prepare timeout for waiting on flock. * Missing the timeout is treated as a fatal error. * It would be able to finger the process id of * the blocking process here to report it on error * using an other F_GETLK.... */ alarm(LOCK_TIMEOUT); if (fcntl (fd, F_SETLKW, &flock) == -1) { int errsv = errno; close(fd); /* is closed on exit, anyway */ die (EX_FILEIO, _("can't lock lock file %s: %s"), MOUNTLOCK_LINKTARGET, (errno == EINTR) ? _("timed out") : strerror (errsv)); } alarm(0); return fd; } /* * Try to establish the lock by creating the hardlink. * Return 1 on success, and 0 if the creation failed. * If we encounter a fatal problem, we die. */ static int link_created() { int fd = open_lock_file(); int i = link(MOUNTLOCK_LINKTARGET, MOUNTED_LOCK); /* If we got the lock, immediately setup the exit handler * to get it properly deleted in case any unexpected * signal raises. */ if(i==0) { lock_file_created = MOUNTED_LOCK; lock_file_desc = fd; at_die = unlock_mtab; return 1; } /* We failed to create the link. * Release the flock and analyze the reason. */ int errsv = errno; close(fd); /* * If an other lockfile exists, this is unexpected * but not fatal. Else throw a fatal error. */ if(errsv != EEXIST) { die (EX_FILEIO, _("can't link lock file %s: %s " "(use -n flag to override)"), MOUNTED_LOCK, strerror (errsv)); } /* signal failure, try again*/ return 0; } /* * Try to establish the lockfile link * within LOCK_TIMEOUT or die. */ static void create_link() { const int delay = 100000; /* 100 ms to sleep*/ int ntry = LOCK_TIMEOUT*(1000000/delay); /* Repeat until it was us who made the link */ while(!link_created()) { /* run out, give up */ if(--ntry<=0) { die (EX_FILEIO, _("possible stale lock file: %s"), MOUNTED_LOCK); } /* An unexpected lockfile already exists. * Until there is no save way to identify a * stale lock there is nothing much we can do * other than waiting for someone else to delete it. */ usleep(delay); } } /* Create the lock file. The lock file will be removed if we catch a signal or when we exit. */ /* The old code here used flock on a lock file /etc/mtab~ and deleted this lock file afterwards. However, as rgooch remarks, that has a race: a second mount may be waiting on the lock and proceed as soon as the lock file is deleted by the first mount, and immediately afterwards a third mount comes, creates a new /etc/mtab~, applies flock to that, and also proceeds, so that the second and third mount now both are scribbling in /etc/mtab. The new code uses a link() instead of a creat(), where we proceed only if it was us that created the lock, and hence we always have to delete the lock afterwards. Now the use of flock() is in principle superfluous, but avoids an arbitrary sleep(). */ /* 2007-01-10 Dieter Stueken <[EMAIL PROTECTED]> Instead of creating new inodes for mtab~ each time, a static file .mtab.lock is used, which is never deleted. This file is linked to mtab~ to perform the lock. F_WRLCKW may be used in addition to perform synchronization of outstanding mount processes. The flock may either be placed on mtab~ or .mtab.lock itself (which is the same), but it never gets lost, as the inode holding the flock remains. */ void lock_mtab (void) { setup_signals(); /* Allow all signals while trying to lock mtab */ sigset_t sigmask; sigemptyset(&sigmask); sigprocmask(SIG_SETMASK, &sigmask, &sigmask); create_link(); /* Restore original signal mask */ sigprocmask(SIG_SETMASK, &sigmask, NULL); } - To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html