[git commit] mount: fix a race when a free loop device is snatched under us by another mount.
Denys Vlasenko
vda.linux at googlemail.com
Thu Dec 17 14:05:14 UTC 2020
commit: https://git.busybox.net/busybox/commit/?id=4bc59a4cf73f1dd9326d150bf6654b643e740fe2
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master
function old new delta
set_loop 850 809 -41
Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
libbb/loop.c | 138 +++++++++++++++++++++++++++++++----------------------------
1 file 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;
}
More information about the busybox-cvs
mailing list