Karel Zak <kzak@xxxxxxxxxx> writes: > On Tue, Jun 19, 2007 at 12:58:06PM +0200, Matthias Koenig wrote: >> Hi, >> >> in addition to the race described recently >> (http://article.gmane.org/gmane.linux.utilities.util-linux-ng/241), >> there is also a race if you have concurring processes running >> mount -o loop. Patch below. > > Good idea! We can also add the same logic to the losetup.c:main(). Ok, I will send a separate patch for this. > > A few suggestions: > >> --- util-linux-devel.orig/mount/lomount.c >> +++ util-linux-devel/mount/lomount.c >> @@ -341,8 +341,15 @@ set_loop(const char *device, const char >> } >> >> if (ioctl(fd, LOOP_SET_FD, ffd) < 0) { >> - perror("ioctl: LOOP_SET_FD"); >> - return 1; >> + switch (errno) { >> + case EBUSY: >> + if (verbose) >> + perror("ioctl: LOOP_SET_FD"); > close (ffd); Ah yes, and close(fd) is missing too. > >> + return 2; >> + default: >> + perror("ioctl: LOOP_SET_FD"); > close (ffd); > >> + return 1; >> + } >> } >> close (ffd); > > >> --- util-linux-devel.orig/mount/mount.c >> +++ util-linux-devel/mount/mount.c >> @@ -846,20 +846,34 @@ loop_check(const char **spec, const char > > >> + if (!*loopdev || !**loopdev) >> + *loopdev = find_unused_loop_device(); >> + if (!*loopdev) >> + return EX_SYSERR; /* no more loop devices */ >> if (verbose) >> - printf(_("mount: failed setting up loop device\n")); >> - return EX_FAIL; >> - } >> + printf(_("mount: going to use the loop device %s\n"), *loopdev); >> + offset = opt_offset ? strtoull(opt_offset, NULL, 0) : 0; >> + if (res = set_loop (*loopdev, *loopfile, offset, >> + opt_encryption, pfd, &loopro)) { >> + switch(res) { >> + case 2: >> + /* loop dev has been grabbed by some other process, >> + try again */ >> + if (verbose) >> + printf("mount: stolen loop=%s ...trying again\n", *loopdev); >> + *loopdev = NULL; > ^^^^^^^^^^^^^^ > memory leak? > > free(*loopdev); > *loopdev = NULL; Right, find_unused_loop_device is using xstrdup > >> + continue; > ^^^^^^^^ > > Please, don't use the find_unused_loop_device() if user explicitly > define a device. > > if (!opt_loop) { > if (verbose) > printf("mount: stolen loop=%s ...trying again\n", *loopdev); > continue; > } > return EX_FAIL; > > Because: > > mount foo.img /foo -o loop=/dev/loop0 Ok. Thanks for review, I will send a reworked patch. Matthias - To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
