mount: fix a race when a free loop device is snatched under us by another mount.

function                                             old     new   delta
set_loop                                             850     809     -41

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

1 files changed, 73 insertions, 65 deletions

diff --git a/libbb/loop.c b/libbb/loop.c
index 85b2724e5..153990998 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -98,9 +98,7 @@ int FAST_FUNC get_free_loop(void)
 
 /* Returns opened fd to the loop device, <0 on error.
  * *device is loop device to use, or if *device==NULL finds a loop device to
- * mount it on and sets *device to a strdup of that loop device name.  This
- * search will re-use an existing loop device already bound to that
- * file/offset if it finds one.
+ * mount it on and sets *device to a strdup of that loop device name.
  */
 int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offset,
                 unsigned long long sizelimit, unsigned flags)
@@ -109,9 +107,7 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse
         char *try;
         bb_loop_info loopinfo;
         struct stat statbuf;
-        int i, dfd, ffd, mode, rc;
-
-        rc = dfd = -1;
+        int i, lfd, ffd, mode, rc;
 
         /* Open the file.  Barf if this doesn't work.  */
         mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
@@ -127,24 +123,23 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse
 
         try = *device;
         if (!try) {
+ get_free_loopN:
                 i = get_free_loop();
-                if (i == -2) { /* no /dev/loop-control */
-                        i = 0;
-                        try = dev;
-                        goto old_style;
-                }
                 if (i == -1) {
                         close(ffd);
                         return -1; /* no free loop devices */
                 }
-                try = *device = xasprintf(LOOP_FORMAT, i);
-                goto try_to_open;
+                if (i >= 0) {
+                        try = xasprintf(LOOP_FORMAT, i);
+                        goto open_lfd;
+                }
+                /* i == -2: no /dev/loop-control. Do an old-style search for a free device */
+                try = dev;
         }
 
- old_style:
-        /* Find a loop device.  */
-        /* 1048575 (0xfffff) is a max possible minor number in Linux circa 2010 */
-        for (i = 0; rc && i < 1048576; i++) {
+        /* Find a loop device */
+        /* 0xfffff is a max possible minor number in Linux circa 2010 */
+        for (i = 0; i <= 0xfffff; i++) {
                 sprintf(dev, LOOP_FORMAT, i);
 
                 IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
@@ -153,72 +148,85 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse
                          && errno == ENOENT
                          && try == dev
                         ) {
-                                /* Node doesn't exist, try to create it.  */
+                                /* Node doesn't exist, try to create it */
                                 if (mknod(dev, S_IFBLK|0644, makedev(7, i)) == 0)
-                                        goto try_to_open;
+                                        goto open_lfd;
                         }
-                        /* Ran out of block devices, return failure.  */
+                        /* Ran out of block devices, return failure */
                         rc = -1;
                         break;
                 }
- try_to_open:
-                /* Open the sucker and check its loopiness.  */
-                dfd = open(try, mode);
-                if (dfd < 0 && errno == EROFS) {
+ open_lfd:
+                /* Open the sucker and check its loopiness */
+                lfd = rc = open(try, mode);
+                if (lfd < 0 && errno == EROFS) {
                         mode = O_RDONLY;
-                        dfd = open(try, mode);
+                        lfd = rc = open(try, mode);
                 }
-                if (dfd < 0) {
+                if (lfd < 0) {
                         if (errno == ENXIO) {
                                 /* Happens if loop module is not loaded */
-                                rc = -1;
+                                /* rc is -1; */
                                 break;
                         }
-                        goto try_again;
+                        goto try_next_loopN;
                 }
 
-                rc = ioctl(dfd, BB_LOOP_GET_STATUS, &loopinfo);
+                rc = ioctl(lfd, BB_LOOP_GET_STATUS, &loopinfo);
 
-                /* If device is free, claim it.  */
+                /* If device is free, try to claim it */
                 if (rc && errno == ENXIO) {
-                        /* Associate free loop device with file.  */
-                        if (ioctl(dfd, LOOP_SET_FD, ffd) == 0) {
-                                memset(&loopinfo, 0, sizeof(loopinfo));
-                                safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
-                                loopinfo.lo_offset = offset;
-                                loopinfo.lo_sizelimit = sizelimit;
-                                /*
-                                 * Used by mount to set LO_FLAGS_AUTOCLEAR.
-                                 * LO_FLAGS_READ_ONLY is not set because RO is controlled by open type of the file.
-                                 * Note that closing LO_FLAGS_AUTOCLEARed dfd before mount
-                                 * is wrong (would free the loop device!)
-                                 */
-                                loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
-                                rc = ioctl(dfd, BB_LOOP_SET_STATUS, &loopinfo);
-                                if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
-                                        /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
-                                        /* (this code path is not tested) */
-                                        loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
-                                        rc = ioctl(dfd, BB_LOOP_SET_STATUS, &loopinfo);
-                                }
-                                if (rc != 0) {
-                                        ioctl(dfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+                        /* Associate free loop device with file */
+                        if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+                                /* Ouch. Are we racing with other mount? */
+                                if (!*device   /* yes */
+                                 && try != dev /* tried a _kernel-offered_ loopN? */
+                                ) {
+                                        free(try);
+                                        close(lfd);
+//TODO: add "if (--failcount != 0) ..."?
+                                        goto get_free_loopN;
                                 }
+                                goto try_next_loopN;
                         }
-                } else {
-                        rc = -1;
-                }
-                if (rc != 0) {
-                        close(dfd);
+                        memset(&loopinfo, 0, sizeof(loopinfo));
+                        safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+                        loopinfo.lo_offset = offset;
+                        loopinfo.lo_sizelimit = sizelimit;
+                        /*
+                         * Used by mount to set LO_FLAGS_AUTOCLEAR.
+                         * LO_FLAGS_READ_ONLY is not set because RO is controlled by open type of the file.
+                         * Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+                         * is wrong (would free the loop device!)
+                         */
+                        loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+                        rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
+                        if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+                                /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+                                /* (this code path is not tested) */
+                                loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+                                rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
+                        }
+                        if (rc == 0) {
+                                /* SUCCESS! */
+                                if (try != dev) /* tried a kernel-offered free loopN? */
+                                        *device = try; /* malloced */
+                                if (!*device)   /* was looping in search of free "/dev/loopN"? */
+                                        *device = xstrdup(dev);
+                                rc = lfd; /* return this */
+                                break;
+                        }
+                        /* failure, undo LOOP_SET_FD */
+                        ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
                 }
- try_again:
-                if (*device) break;
-        }
+                /* else: device is not free (rc == 0) or error other than ENXIO */
+                close(lfd);
+ try_next_loopN:
+                rc = -1;
+                if (*device) /* was looking for a particular "/dev/loopN"? */
+                        break; /* yes, do not try other names */
+        } /* for() */
+
         close(ffd);
-        if (rc == 0) {
-                if (!*device)
-                        *device = xstrdup(dev);
-                return dfd;
-        }
         return rc;
 }