Re: Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-07-27 at 18:24, Jens Axboe wrote:
On 7/27/21 4:56 PM, Krzysztof Olędzki wrote:
On 2021-07-27 at 15:39, Krzysztof Olędzki wrote:
On 2021-07-27 at 14:53, Krzysztof Olędzki wrote:
Hi,

I have a number of (older) systems that are still based on 32 bit
userspace but are running a relatively modern 64 bit kernel -
5.4-stable, where BTW - LOOP_CONFIGURE is not yet available.

I noticed that starting with util-linux-2.37 it is no longer possible to
mount images using loop:

# mount /usr/install/iso/systemrescue-8.04-amd64.iso /mnt/cdrom
mount: /mnt/cdrom: failed to setup loop device for
/usr/install/iso/systemrescue-8.04-amd64.iso.

Reverting d5fd456c88aba4fcf77d35fe38024a8d5c814686 fixes the problem:

/tmp/util-linux-2.37# ./mount
/usr/install/iso/systemrescue-8.04-amd64.iso /mnt/cdrom
mount: /mnt/cdrom: WARNING: source write-protected, mounted read-only.

I have not tested if 32 bit kernel + 32 bit userspace is also affected,
but 64 bit kernel + 64 bit userspace works.

Some debugging data:

30399: loopdev:      CXT: [0xff8d0f98]: using loop-control
30399: loopdev:      CXT: [0xff8d0f98]: loop0 name assigned
30399: loopdev:      CXT: [0xff8d0f98]: find_unused by loop-control [rc=0]
30399: libmount:     LOOP: [0x57cbbcb0]: trying to use /dev/loop0
30399: loopdev:      CXT: [0xff8d0f98]: set backing file=/usr/install/iso/systemrescue-8.04-amd64.iso
30399: loopdev:      CXT: [0xff8d0f98]: set flags=4
30399: loopdev:    SETUP: [0xff8d0f98]: device setup requested
30399: loopdev:    SETUP: [0xff8d0f98]: backing file open: OK
30399: loopdev:      CXT: [0xff8d0f98]: open /dev/loop0 [rw]: Success
30399: loopdev:    SETUP: [0xff8d0f98]: device open: OK
30399: loopdev:    SETUP: [0xff8d0f98]: LOOP_CONFIGURE failed: Inappropriate ioctl for device
30399: loopdev:    SETUP: [0xff8d0f98]: failed [rc=-25]
30399: libmount:     LOOP: [0x57cbbcb0]: failed to setup device
30399: loopdev:      CXT: [0xff8d0f98]: de-initialize
30399: loopdev:      CXT: [0xff8d0f98]: closing old open fd
30399: loopdev:     ITER: [0xff8d1168]: de-initialize
30399: libmount:      CXT: [0x57cbbcb0]: mount: preparing failed
30399: libmount:      CXT: [0x57cbbcb0]: excode: rc=32 message="failed to setup loop device for /usr/install/iso/systemrescue-8.04-amd64.iso"
mount: /mnt/cdrom: failed to setup loop device for /usr/install/iso/systemrescue-8.04-amd64.iso.
30399: libmount:      CXT: [0x57cbbcb0]: <---- reset [status=0] ---->

Seems like the code expects EINVAL (-22) but gets ENOTTY (-25), confirmed with strace:
ioctl(4, LOOP_CONFIGURE, {fd=3, block_size=0, info={lo_offset=0, lo_number=0, lo_flags=LO_FLAGS_AUTOCLEAR, lo_file_name="/usr/install/iso/systemrescue-8.04-amd64.iso", ...}}) = -1 ENOTTY (Inappropriate ioctl for device)

Indeed, changing the code from:
     if (errno != EINVAL)
to:
     if (errno != EINVAL && errno != ENOTTY)
allows it to work.

Not that with 64-bit userspace, kernel returns EINVAL:

ioctl(4, LOOP_CONFIGURE, {fd=3, block_size=0, info={lo_offset=0, lo_number=0, lo_flags=LO_FLAGS_AUTOCLEAR, lo_file_name="/usr/src/PACKAGES/systemrescue-8.04-amd64.iso", ...}}) = -1 EINVAL (Invalid argument)

... which is because lo_compat_ioctl returns -ENOIOCTLCMD for
unsupported cmds, while lo_ioctl returns -EINVAL via lo_simple_ioctl.

And vfs_ioctl returns -ENOTTY for -ENOIOCTLCMD.

Now the question is if this inconsistency is intended? :)

That's unfortunate, but probably not something that can get corrected at
this time. The correct return value for an unknown ioctl is -ENOTTY
(ENOIOCTLCMD isn't user visible, should get turned into that).

Yes, it does - as I said, vfs_ioctl handles this properly. However, this only works for .compat_ioctl (via mentioned lo_compat_ioctl which returns -ENOIOCTLCMD) but not for .ioctl (via lo_ioctl, which returns -EINVAL).


But
current behavior is set in stone at this point, even if it is
technically incorrect.

Agreed. And even if this could be somehow fixed in further kernels, I believe we still need to fix the userspace to support and properly handle all the existing kernels.

So, to confirm - checking for both EINVAL and ENOTTY after LOOP_CONFIGURE is the proper way of taking care this?

https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/lib/loopdev.c?id=d4423cce9b9001c9de7ebc6f64f6cc2bb854944c#n1362

Thanks,
 Krzysztof




[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux