[trusty] mount -o loop is limited to 8 loop devices

Bug #1640823 reported by Martin Pitt on 2016-11-10
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
util-linux (Ubuntu)
Undecided
Unassigned
Trusty
Undecided
Unassigned

Bug Description

trusty has a very old util-linux which does not yet know about /dev/loop-control to create arbitrarily many loop devices. This feature was introduced in Linux 3.1 already (i. e. before precise even). This is a showstopper for backporting snappy as that needs a lot of loop mounts.

Support for loop-control got introduced later util-linux versions, but backporting full support for it (for losetup) is too intrusive. We only need a partial backport for "mount -o loop".

SRU TEST CASE:
First, use up all default 8 loop devices:
$ for i in `seq 8`; do echo $i; sudo losetup --find /etc/issue; done

Now try to use a 9th:
$ dd if=/dev/zero of=/tmp/img bs=1M count=50
$ mkfs.ext2 -F /tmp/img
$ sudo mount -o loop /tmp/img /mnt

With current trusty's "mount" package this will fail with "could not find any free loop device". With the proposed version, this should succeed, and "sudo losetup -a" should show "/dev/loop8: ... (/tmp/img)".

Now, reboot, disable loop-control with

  sudo mv /dev/loop-control{,.disabled}

and run the test case again. Now "mount -o loop" should fail with "could not find any free loop device" (as before). Ensure that there are no hangs, infinite loops, etc.

ADDITIONAL REGRESSION CHECKING TEST CASES

1. Check that every type of losetup call documented in the losetup manpage still works correctly.

2. Check that mount and umount commands that use loop devices still work correctly.

REGRESSION POTENTIAL: /dev/loop-control and the corresponding util-linux support has exited for a long time without known/major issues, so this should be fairly safe. Also, the patch falls back to the previous "iterate over loop0 to loop7" behaviour if loop-control is not available.

Martin Pitt (pitti) on 2016-11-10
Changed in util-linux (Ubuntu):
status: New → Fix Released
Changed in util-linux (Ubuntu Trusty):
status: New → Triaged
Martin Pitt (pitti) on 2016-11-10
description: updated
Martin Pitt (pitti) on 2016-11-10
description: updated
summary: - [trusty] limited to 8 loop devices
+ [trusty] mount -o loop is limited to 8 loop devices
Martin Pitt (pitti) on 2016-11-10
description: updated
Martin Pitt (pitti) wrote :

For the record: Commit 0b14bf7a is not sufficient. That neither fixes "losetup" nor "mount -o loop", and systemd's .mount units also just get translated into a /bin/mount -o loop" call.

Martin Pitt (pitti) on 2016-11-15
description: updated
description: updated

Hello Martin, or anyone else affected,

Accepted util-linux into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/util-linux/2.20.1-5.1ubuntu20.8 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in util-linux (Ubuntu Trusty):
status: Triaged → Fix Committed
Martin Pitt (pitti) wrote :

I successfully ran the test case with the packages from -proposed. The VM also still boots successfully and shows no other sign of trouble.

tags: added: verification-done
Robie Basak (racb) wrote :

This felt unusually risk to me, so I took a look at the patch before releasing. I found a number of problems:

1. This patch leaks a file descriptor every time the new code path runs. loop_ctl_fd is opened but not closed.

2. "int devnr = -1" is defined but never used.

3.

> ll->ncur = ioctl(loop_ctl_fd, LOOP_CTL_GET_FREE);

This is wrong. ncur is an integer iterator, not necessarily a loop number. It keeps state between multiple calls to looplist_next, and you're clobbering it. It may be the case that it happens not to get used again when LLFLG_LOOPCTL is set, but that's no reason to re-use an existing variable for a different purpose. This makes the code much more difficult to check for unexpected side effects and thus may hide bugs.

4. looplist_next() is called to acquire both free and used loop device fds (based on LLFLG_USEDONLY/LLFLG_FREEONLY; see looplist_open_dev()). The fashion in which it does this is quite convulated. Additionally, it appears designed to be called multiple times (like a C++ iterator). The new code will always return a new loop device when requested. This is of course the point of this SRU. However, a consequence of this is that calls to looplist_next with LLFLG_FREEONLY are now unbounded, and this may have implications throughout the code. I can't find any, but this is certainly Regression Potential territory, and IMHO all known use cases that call looplist_next() should be checked for correct behaviour in SRU verification.

5. The "mount" and "umount" commands also call functions defined by lomount.c, so loop-affecting behaviour should be checked in these, too.

It feels like luck to me that this code didn't introduce a bug that I could find. However, we want to minimise regression risk in SRUs. Even though I couldn't find a specific bug, issues 1 and 3 above unnecessarily add to the risk of regression in this SRU, IMHO, and are easy to mitigate. So I'm marking this bug verification-failed. You might as well fix issue 2 while you're there. For issues 4 and 5, I'll update the test plan in the bug description.

tags: added: verification-failed
removed: verification-done
description: updated
Thomas Voß (thomas-voss) wrote :

Thanks for your detailed feedback, I revisited the patch (please see the diff here http://paste.ubuntu.com/23522718/). I think (1.) - (3.) are covered.

Martin Pitt (pitti) wrote :

Hello Martin, or anyone else affected,

Accepted util-linux into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/util-linux/2.20.1-5.1ubuntu20.9 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Martin Pitt (pitti) wrote :

I successfully ran the test case against ubuntu20.9 in -proposed. I also verified that using losetup and mount on loop devices still works:

$ sudo losetup -f --show debian-stretch-DI-alpha7-amd64-netinst.iso
/dev/loop0
$ sudo mount /dev/loop0 /mnt
mount: block device /dev/loop0 is write-protected, mounting read-only
$ ls /mnt
autorun.inf debian [...]

With the -proposed version, losetup -f also succeeds to use /dev/loop-control, so you can do the above a few times and it works with /dev/loop12 too. When moving /dev/loop-control out of the way, the above also works up to loop7, and then errors out with "losetup: could not find any free loop device" as expected. "losetup -a" correctly shows the bound devices/files.

tags: added: verification-needed
removed: verification-failed
tags: added: verification-done
removed: verification-needed
Robie Basak (racb) wrote :

16:49 <rbasak> bdmurray: I hadn't looked at bug 1640823 again, no. Shall I review and release if appropriate during my SRU day tomorrow?

16:50 <bdmurray> rbasak: Since you had an opinion I think that's appropriate

The verification of the Stable Release Update for util-linux has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package util-linux - 2.20.1-5.1ubuntu20.9

---------------
util-linux (2.20.1-5.1ubuntu20.9) trusty; urgency=medium

  * mount/lomount.c: Query /dev/loop-control for next free loopback device.
    (LP: #1640823)

 -- Thomas Voß <email address hidden> Tue, 15 Nov 2016 09:52:37 +0100

Changed in util-linux (Ubuntu Trusty):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers