Bug#829496: kpartx -d ( detach ) option silently fails

[Christian Kastner]

Control: tag -1 patch

On Sun, 03 Jul 2016 22:49:18 +0200 Emmanuel Kasper <emmanuel at libera.cc>
wrote:
> Since the 0.6.1-* version of kpartx, the '-d' option does not work anymore.
> 
> Simple test case:
> 
> # add loopdevice and partition mappings, freedos.raw is here a disk image
> kpartx -av freedos.raw
> add map loop0p1 (253:0): 0 262017 linear 7:0 63
> 
> # ask kpartx to detach the loop device
> kpartx -d freedos.raw
> 
> # but the loop device is still there
> file --special-files --dereference /dev/loop0 
> /dev/loop0: DOS/MBR boot sector; partition 1 : ID=0xe, active, start-CHS (0x0,1,1), end-CHS (0x13,15,63), startsector 63, 262017 sectors
> 
> losetup --list 
> NAME       SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE                                  DIO
> /dev/loop0         0      0         0  0 /home/manu/Projects/emul/i386/kvm/freedos.raw

I ran into this bug when creating bootable VM images using
vmdebootstrap. I noticed that all images, except the first one created
after a reboot, failed to boot.

The problem is that the code that is supposed to detect whether kpartx
is dealing with a loop-device-backed regular file is broken.

Assuming kpartx -d blockdev.img is called, the following happens in
kpartx.c:

>         if (stat(device, &buf)) {
>                 printf("failed to stat() %s\n", device);
>                 exit (1);
>         }

buf gets set with the status of device, which is blockdev.img.

>         if (S_ISREG (buf.st_mode)) {
>                 /* already looped file ? */
>                 loopdev = find_loop_by_file(device);

This identifies the backing loop device of blockdev.img. Let's assume
that the device is /dev/loop0.

>
>                 if (!loopdev && what == DELETE)
>                         exit (0);
>
>                 if (!loopdev) {
>                         loopdev = find_unused_loop_device();
>
>                         if (set_loop(loopdev, device, 0, &ro)) {
>                                 fprintf(stderr, "can't set up loop\n");
>                                 exit (1);
>                         }
>                         loopcreated = 1;
>                 }

The above is not executed, as loopdev has been previously set.

>                 device = loopdev;

device no longer points to "blockdev.img", but to "/dev/loop0".

>
>                 if (stat(device, &buf)) {
>                         printf("failed to stat() %s\n", device);
>                         exit (1);
>                 }
>         }

buf gets set again, but this time with the status of /dev/loop0.

Later:
>                         if (S_ISREG (buf.st_mode)) {
                                      ^^^
With buf being the status of /dev/loop0, S_ISREG evaluates to false.
Therefore, the following code which would normally remove the backing
loop device does not get called.

>                                 if (del_loop(device)) {
>                                         if (verbose)
>                                                 printf("can't del loop: %s\n",
>                                                         device);
>                                         exit(1);
>                                 }
>                                 printf("loop deleted : %s\n", device);
>                         }
>                         break;

I've sent the attached patch to the upstream mailing list, but it should
also apply cleanly to the Debian version, albeit with some offset.

Regards,
Christian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kpartx-Fix-check-whether-to-detach-loop-device.patch
Type: text/x-patch
Size: 1644 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-lvm-maintainers/attachments/20160713/137385a5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-lvm-maintainers/attachments/20160713/137385a5/attachment.sig>