LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace
       [not found] ` <e7f64d43-2a26-e386-b208-5c35d6a56ed4@ans.pl>
@ 2021-07-27 22:56   ` Krzysztof Olędzki
  2021-07-28  1:24     ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Olędzki @ 2021-07-27 22:56 UTC (permalink / raw)
  To: Sinan Kaya, Karel Zak, Jens Axboe
  Cc: util-linux, linux-block, Linux Kernel Mailing List

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? :)

Krzysztof


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace
  2021-07-27 22:56   ` Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace Krzysztof Olędzki
@ 2021-07-28  1:24     ` Jens Axboe
  2021-07-28  5:46       ` Krzysztof Olędzki
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2021-07-28  1:24 UTC (permalink / raw)
  To: Krzysztof Olędzki, Sinan Kaya, Karel Zak
  Cc: util-linux, linux-block, Linux Kernel Mailing List

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). But
current behavior is set in stone at this point, even if it is
technically incorrect.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace
  2021-07-28  1:24     ` Jens Axboe
@ 2021-07-28  5:46       ` Krzysztof Olędzki
  2021-07-28  9:14         ` Karel Zak
  0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Olędzki @ 2021-07-28  5:46 UTC (permalink / raw)
  To: Jens Axboe, Sinan Kaya, Karel Zak
  Cc: util-linux, linux-block, Linux Kernel Mailing List

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace
  2021-07-28  5:46       ` Krzysztof Olędzki
@ 2021-07-28  9:14         ` Karel Zak
  2021-07-28 18:00           ` Sinan Kaya
  0 siblings, 1 reply; 5+ messages in thread
From: Karel Zak @ 2021-07-28  9:14 UTC (permalink / raw)
  To: Krzysztof Olędzki
  Cc: Jens Axboe, Sinan Kaya, util-linux, linux-block,
	Linux Kernel Mailing List

On Tue, Jul 27, 2021 at 10:46:06PM -0700, Krzysztof Olędzki wrote:
> 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

We need both to make losetup/mount robust for already released kernels.
Fixed: https://github.com/karelzak/util-linux/commit/583990d25b5d65a9a9771a39d112de0ee16a1f3a

Thanks for your report!

 Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace
  2021-07-28  9:14         ` Karel Zak
@ 2021-07-28 18:00           ` Sinan Kaya
  0 siblings, 0 replies; 5+ messages in thread
From: Sinan Kaya @ 2021-07-28 18:00 UTC (permalink / raw)
  To: Karel Zak, Krzysztof Olędzki
  Cc: Jens Axboe, util-linux, linux-block, Linux Kernel Mailing List

Thanks for fixing the issue and making it more robust.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-07-28 18:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <a797f527-4599-e986-a326-4bb141487f2c@ans.pl>
     [not found] ` <e7f64d43-2a26-e386-b208-5c35d6a56ed4@ans.pl>
2021-07-27 22:56   ` Commit d5fd456c88aba4fcf77d35fe38024a8d5c814686 - "loopdev: use LOOP_CONFIG ioctl" broke loop on x86-64 w/ 32 bit userspace Krzysztof Olędzki
2021-07-28  1:24     ` Jens Axboe
2021-07-28  5:46       ` Krzysztof Olędzki
2021-07-28  9:14         ` Karel Zak
2021-07-28 18:00           ` Sinan Kaya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).