[dm-devel] multipath tools racy on RUN file.

[Dave Olien <dmo osdl org>]

Hi, Christophe,

I was looking at the multipath/main.c source file.  I notice at the
beginning, it has code:

        /*
         * Don't run in parallel
         */
        while (filepresent(RUN) && try++ < MAXTRY)
                usleep(100000);
                                                                                
        if (filepresent(RUN)) {
                fprintf(stderr, "waited for to long. exiting\n");
                exit(1);
        }
                                                                                
        /*
         * Our turn
         */
        if (!open(RUN, O_CREAT)) {
                fprintf(stderr, "can't create runfile\n");
                exit(1);
        }

This is racy.  This isn't sufficient to guarantee that two multipath
binaries will never run concurrently.  The multipathd could just happen
to begin execution at the same instant an administrator runs multipath.
Both could find the RUN file not present.  Both would then do the
open(), and both would succeed.

Plus, you have the issue of removing this file when multipath exits.
If multipath should exit abnormally (due to an exception for example),
it leaves behind the RUN file.

I suggest doing something like:

	fd = open(RUN, O_CREAT)
	if (fd < 0)
		error

	struct flock fl;
	fl.l_type = FWRLCK;
	fl.l_whence = 0;
	fl.l_start = 0;
	fl.l_len = 0;
	alarm(5);		/* or however many seconds you want to wait */
	if (fcntl(fd, F_SETLKW, &fl) == -1) {
       		if (errno == EINTR) /* timed out */
       	 	else /* other error */
	}


There should be no need to remove the RUN file.  If it already exists
when entering this code, there's no harm done.  If the binary exits
abnormally, the record lock will be removed as part of the kernel's
exit handling.

How does this sound to you?

Thanks,
Dave Olien