On Wed, Mar 15, 2017 at 3:16 AM, Denys Vlasenko
<[email protected]> wrote:
> On Tue, Mar 7, 2017 at 6:12 PM, Mirko Vogt <[email protected]> wrote:
>>> "foo" is a cramfs filesystem file which might help illustrate the issue
>>>
>>> 1) loopback mount "foo" to mount /bar
>>> 2) umount /bar
>>> 3) append new files and re-generate the "foo" cramfs image
>>> 4) loopback mount "foo" to mount /bar
>>> 5) the contents of /bar are the same as in 1) and not 3)
>>>
>>> the reason for this is because busybox has not deleted the "foo" <->
>>> loopback device mapping, so when we attempt to mount again "foo", the
>>> mapping is already there, so the loopback device is not deleted then
>>> re-created.
>>>
>>> Obviously using umount -d in 2) fixes the issue, but I was wondering
>>> whether it would not be preferable to unconditionnaly delete the
>>> loopback device upon umount? util-linux does this actually, so other
>>> users might also be puzzled by such a case.
>>
>> I'm quoting most of the mail since it has been a while.. this "feature"
>> hit me today, however from a different angle.
>>
>> It seems LOOP_CLR_FD called on a loop-*partition* removes the mapping of
>> the whole *device* - which results in the following:
>>
>> root@LEDE:/# loop=$(losetup -f)
>> root@LEDE:/# echo ${loop}
>> /dev/loop2
>> root@LEDE:/# losetup ${loop} /IMAGE
>> root@LEDE:/# ls -l ${loop}*
>> brw------- 1 root root 7, 2 Mar 6 20:09 /dev/loop2
>> root@LEDE:/# partprobe ${loop}
>> root@LEDE:/# ls -l ${loop}*
>> brw------- 1 root root 7, 2 Mar 6 20:09 /dev/loop2
>> brw------- 1 root root 259, 8 Mar 6 21:59 /dev/loop2p1
>> brw------- 1 root root 259, 9 Mar 6 21:59 /dev/loop2p2
>> brw------- 1 root root 259, 10 Mar 6 21:59 /dev/loop2p3
>> brw------- 1 root root 259, 11 Mar 6 21:59 /dev/loop2p4
>> brw------- 1 root root 259, 12 Mar 6 21:59 /dev/loop2p5
>> brw------- 1 root root 259, 13 Mar 6 21:59 /dev/loop2p6
>> brw------- 1 root root 259, 14 Mar 6 21:59 /dev/loop2p7
>> brw------- 1 root root 259, 15 Mar 6 21:59 /dev/loop2p8
>> root@LEDE:/# mount ${loop}p8 /MOUNT # mount loop partition
>> root@LEDE:/# losetup -a | grep $loop # loop dev mapping still there
>> /dev/loop2: 0 /mnt/IMAGE
>> root@LEDE:/# strace umount /MOUNT 2> /log # unmount loop partition
>> root@LEDE:/# losetup -a | grep ${loop} # loop device mapping is gone
>> root@LEDE:/# grep -i loop /log
>> open("/dev/loop2p7", O_RDONLY|O_LARGEFILE) = 3
>> ioctl(3, LOOP_CLR_FD) = 0
>> root@LEDE:/#
>>
>> The strace was done to figure out, if maybe umount wrongly ioctl()'s the
>> parent device instead of the partition - it doesn't.
>>
>> I already wasn't a fan of umount implicitly removing the mapping in the
>> first place (as I usually setup and release loop devices with `losetup`
>> and scripts needed to call umount differently in order to work and
>> outside busybox).
>>
>> However taking above (kernel-)behaviour into account - umount calling
>> ioctl(LOOP_CLR_FD) unconditionally potentially causes some nasty side
>> effects, why I'd like to re-open that discussion.
>
>
> What a surprise. No one foresaw this to bite, right? ;)
> [to understand the snark, read the entire thread]
>
> Now seriously.
> You can use umount -D to suppress this.
> Unfortunately, util-linux's umount does not have that option, thus
> your scripts would become incompatible with util-linux if you go that route.
>
> Their manpage says:
>
> LOOP DEVICE
> The umount command will free the loop device associated with a mount
> when it finds the option loop=... in /etc/mtab, or when the -d option
> was given. Any still associated loop devices can be freed
> by using losetup -d; see losetup(8).
>
> We can try to match this description. However, it is lying:
> util-linux's umount somehow detects automatic loop mounts
> even when /etc/mtab does not contain "loop".
>
> First, I'm using util-linux's mount on a 1mbyte ext2 image:
>
> # mount -oloop z zz
> # ls -l /etc/mtab
> lrwxrwxrwx. 1 root root 19 May 24 2016 /etc/mtab -> ../proc/self/mounts
> # cat /etc/mtab | grep loop
> /dev/loop0 /home/srcdevel/bbox/fix/busybox.t6/zz ext2
> rw,relatime,block_validity,barrier,user_xattr,acl 0 0
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no "loop"
> # umount zz
> # cat /etc/mtab | grep loop
> # losetup
> <nothing>: /dev/loop0 freed
>
> Now the same with bbox mount:
>
> # ./busybox mount -oloop z zz
> # cat /etc/mtab | grep loop
> /dev/loop0 /home/srcdevel/bbox/fix/busybox.t6/zz ext4
> rw,relatime,block_validity,delalloc,barrier,user_xattr,acl 0 0
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no "loop"
> # umount zz
> # losetup
> NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE
> DIO
> /dev/loop0 0 0 0 0
> /home/srcdevel/bbox/fix/busybox.t6/z 0
>
> Clearly, util-linux umount looks somewhere else to determine whether
> to free the loop device.
> Hmm...
>
> # mount -oloop z zz
> # losetup
> NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE
> DIO
> /dev/loop0 0 0 1 0
> /home/srcdevel/bbox/fix/busybox.t6/z 0
>
>
> It's probably the "AUTOCLEAR" thing.
I propose this as the fix:
diff --git a/include/libbb.h b/include/libbb.h
index b054e05..e97efcb 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1329,10 +1329,15 @@ extern int get_linux_version_code(void) FAST_FUNC;
extern char *query_loop(const char *device) FAST_FUNC;
extern int del_loop(const char *device) FAST_FUNC;
-/* If *devname is not NULL, use that name, otherwise try to find free one,
+/*
+ * If *devname is not NULL, use that name, otherwise try to find free one,
* malloc and return it in *devname.
- * return value: 1: read-only loopdev was setup, 0: rw, < 0: error */
-extern int set_loop(char **devname, const char *file, unsigned long long
offset, int ro) FAST_FUNC;
+ * return value is the opened fd to the loop device, or < on error
+ */
+extern int set_loop(char **devname, const char *file, unsigned long long
offset, unsigned flags) FAST_FUNC;
+/* These constants match linux/loop.h (without BB_ prefix): */
+#define BB_LO_FLAGS_READ_ONLY 1
+#define BB_LO_FLAGS_AUTOCLEAR 4
/* Like bb_ask below, but asks on stdin with no timeout. */
char *bb_ask_stdin(const char * prompt) FAST_FUNC;
diff --git a/libbb/loop.c b/libbb/loop.c
index d30b378..f0d4296 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -78,22 +78,24 @@ int FAST_FUNC del_loop(const char *device)
return rc;
}
-/* Returns 0 if mounted RW, 1 if mounted read-only, <0 for 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.
+/* 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.
*/
-int FAST_FUNC set_loop(char **device, const char *file, unsigned long long
offset, int ro)
+int FAST_FUNC set_loop(char **device, const char *file, unsigned long long
offset, unsigned flags)
{
char dev[LOOP_NAMESIZE];
char *try;
bb_loop_info loopinfo;
struct stat statbuf;
- int i, dfd, ffd, mode, rc = -1;
+ int i, dfd, ffd, mode, rc;
+
+ rc = dfd = -1;
/* Open the file. Barf if this doesn't work. */
- mode = ro ? O_RDONLY : O_RDWR;
+ mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
open_ffd:
ffd = open(file, mode);
if (ffd < 0) {
@@ -144,20 +146,35 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* If device is free, claim it. */
if (rc && errno == ENXIO) {
- memset(&loopinfo, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
/* Associate free loop device with file. */
if (ioctl(dfd, LOOP_SET_FD, ffd) == 0) {
- if (ioctl(dfd, BB_LOOP_SET_STATUS, &loopinfo)
== 0)
- rc = 0;
- else
+ memset(&loopinfo, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name,
file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ /*
+ * 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);
+ }
}
} else {
rc = -1;
}
- close(dfd);
+ if (rc != 0) {
+ close(dfd);
+ }
try_again:
if (*device) break;
}
@@ -165,7 +182,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc == 0) {
if (!*device)
*device = xstrdup(dev);
- return (mode == O_RDONLY); /* 1:ro, 0:rw */
+ return dfd;
}
return rc;
}
diff --git a/util-linux/losetup.c b/util-linux/losetup.c
index 4424d9c..d356f49 100644
--- a/util-linux/losetup.c
+++ b/util-linux/losetup.c
@@ -127,12 +127,37 @@ int losetup_main(int argc UNUSED_PARAM, char **argv)
d = *argv++;
if (argv[0]) {
- if (set_loop(&d, argv[0], offset, (opt & OPT_r)) < 0)
+ if (set_loop(&d, argv[0], offset, (opt & OPT_r) ?
BB_LO_FLAGS_READ_ONLY : 0) < 0)
bb_simple_perror_msg_and_die(argv[0]);
return EXIT_SUCCESS;
}
}
+ /* TODO: util-linux 2.28 shows this when run w/o params:
+ * NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE DIO
+ * /dev/loop0 0 0 1 0 /PATH/TO/FILE 0
+ *
+ * implemented by reading /sys:
+ *
+ * open("/sys/block", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
+ * newfstatat(3, "loop0/loop/backing_file", {st_mode=S_IFREG|0444,
st_size=4096, ...}, 0) = 0
+ * stat("/dev/loop0", {st_mode=S_IFBLK|0660, st_rdev=makedev(7, 0),
...}) = 0
+ * open("/sys/dev/block/7:0/loop/offset", O_RDONLY|O_CLOEXEC) = 5
+ * read(5, "0\n", 4096) = 2
+ * open("/sys/dev/block/7:0/loop/sizelimit", O_RDONLY|O_CLOEXEC) = 5
+ * read(5, "0\n", 4096) = 2
+ * open("/sys/dev/block/7:0/loop/offset", O_RDONLY|O_CLOEXEC) = 5
+ * read(5, "0\n", 4096) = 2
+ * open("/sys/dev/block/7:0/loop/autoclear", O_RDONLY|O_CLOEXEC) = 5
+ * read(5, "1\n", 4096) = 2
+ * open("/sys/dev/block/7:0/ro", O_RDONLY|O_CLOEXEC) = 5
+ * read(5, "0\n", 4096) = 2
+ * open("/sys/dev/block/7:0/loop/backing_file", O_RDONLY|O_CLOEXEC) = 5
+ * read(5, "/PATH/TO/FILE", 4096) = 37
+ * open("/sys/dev/block/7:0/loop/dio", O_RDONLY|O_CLOEXEC) = 5
+ * read(5, "0\n", 4096) = 2
+ */
+
bb_show_usage(); /* does not return */
/*return EXIT_FAILURE;*/
}
diff --git a/util-linux/mount.c b/util-linux/mount.c
index f0245f7..6bb1852 100644
--- a/util-linux/mount.c
+++ b/util-linux/mount.c
@@ -1887,6 +1887,7 @@ static int nfsmount(struct mntent *mp, unsigned long
vfsflags, char *filteropts)
// NB: mp->xxx fields may be trashed on exit
static int singlemount(struct mntent *mp, int ignore_busy)
{
+ int loopfd = -1;
int rc = -1;
unsigned long vfsflags;
char *loopFile = NULL, *filteropts = NULL;
@@ -2026,7 +2027,20 @@ static int singlemount(struct mntent *mp, int
ignore_busy)
if (ENABLE_FEATURE_MOUNT_LOOP && S_ISREG(st.st_mode)) {
loopFile = bb_simplify_path(mp->mnt_fsname);
mp->mnt_fsname = NULL; // will receive malloced loop
dev name
- if (set_loop(&mp->mnt_fsname, loopFile, 0, /*ro:*/
(vfsflags & MS_RDONLY)) < 0) {
+
+ // mount always creates AUTOCLEARed loopdevs, so that
umounting
+ // drops them without any code in the userspace.
+ // This happens since circa linux-2.6.25:
+ // commit 96c5865559cee0f9cbc5173f3c949f6ce3525581
+ // Date: Wed Feb 6 01:36:27 2008 -0800
+ // Subject: Allow auto-destruction of loop devices
+ loopfd = set_loop(&mp->mnt_fsname,
+ loopFile,
+ 0,
+ ((vfsflags & MS_RDONLY) ?
BB_LO_FLAGS_READ_ONLY : 0)
+ | BB_LO_FLAGS_AUTOCLEAR
+ );
+ if (loopfd < 0) {
if (errno == EPERM || errno == EACCES)
bb_error_msg(bb_msg_perm_denied_are_you_root);
else
@@ -2074,6 +2088,8 @@ static int singlemount(struct mntent *mp, int ignore_busy)
}
// If mount failed, clean up loop file (if any).
+ // (Newer kernels which support LO_FLAGS_AUTOCLEAR should not need this,
+ // merely "close(loopfd)" should do it?)
if (ENABLE_FEATURE_MOUNT_LOOP && rc && loopFile) {
del_loop(mp->mnt_fsname);
if (ENABLE_FEATURE_CLEAN_UP) {
@@ -2086,6 +2102,9 @@ static int singlemount(struct mntent *mp, int ignore_busy)
if (ENABLE_FEATURE_CLEAN_UP)
free(filteropts);
+ if (loopfd >= 0)
+ close(loopfd);
+
if (errno == EBUSY && ignore_busy)
return 0;
if (errno == ENOENT && (vfsflags & MOUNT_NOFAIL))
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
