LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

[Chris Samuel]

/*
 * CC'd to linux-kernel in case they have any feedback on this.
 *
 * Long thread, trying to work out why mkfs.btrfs failed to
 * make a filesystem on an encrypted loopback mount called
 * /dev/loop2. Cause turned out to be mkfs.btrfs calling
 * LOOP_GET_STATUS to find out if the block device was mounted
 * and getting a truncated device name back and so it later
 * fails when lstat() is called on the truncated device path.
 *
 * The long device name for the encrypted loopback mount was
 * because /dev/disk/by-id/$ID was used when Felix created it
 * to cope with devices moving around.
 */

On 25/01/11 00:01, Felix Blanke wrote:

> you were talking about the LOOP_GET_STATUS function. I'm not
> quite sure where does it came from. Is it part of the kernel?
> Or does it come from the util-linux package?

It's in the kernel, and there is both LOOP_GET_STATUS (old
implementation) and LOOP_GET_STATUS64 (new implementation).

They return structures called loop_info and loop_info64
respectively and both are defined in include/linux/loop.h .

Sadly in both cases the lengths of paths are defined to be
LO_NAME_SIZE which is currently 64 and hence either
implementation will cause the problematic:

lstat("/dev/disk/by-id/ata-INTEL_SSDSA2M160G2GC_CVPO939201JX160AGN-par",
0x7fffa30b3cf0) = -1 ENOENT (No such file or directory)

I've CC'd this to the LKML in case they have any feedback on
this apparent problem with the API.

cheers,
Chris
-- 
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC

Re: LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

[Felix Blanke]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

Hi,

attached is the answer from Jari Ruusu, (one of?!) the main developer of loop-aes. It
seems that checking if a loop device is mounted following the link isn't the best
idea :)

I'll have time to look deeper into his example about the 14.02. I'll then try to fix
that issue in mkfs.btrfs. If someone wants to fix it earlier, do it :) 



Regards,
Felix

[-- Attachment #2: Type: message/rfc822, Size: 5495 bytes --]

From: Jari Ruusu <jariruusu@users.sourceforge.net>
To: Felix Blanke <felixblanke@gmail.com>
Cc: linux-crypto@kernelnewbies.org
Subject: Re: Problem with losetup
Date: Wed, 26 Jan 2011 14:39:42 +0200
Message-ID: <4D40160E.605750DF@users.sourceforge.net>

First of all, I am not subscribed to linux-crypto@kernelnewbies.org mailing
list. But I do check web archive occasionally, at least for now. If you want
loop-AES maintainer to see your posts, please CC my @users.sourceforge.net
address, or linux-crypto@vger.kernel.org

Felix Blanke wrote:
> If I'm using the unpatched losetup (from util-linux-ng) and looping a
> device like
> 
> /dev/disk/by-id/ata-WDC_WD6400AAKS-22A7B2_WD-WCASY7780706-part3
> 
> it is using readlink to track down the link to e.g. /dev/sda3. But if
> I'm using the patched losetup I don't see any readlink in the strace.
> Because no readlink is used there is a problem with device names which
> have more then 64 characters (the field "lo_name" is an array of 64
> char). It is truncated like
> 
> /dev/loop0: [0010]:6229
> (/dev/disk/by-id/ata-INTEL_SSDSA2M160G2GC_CVPO939201JX160AGN-par)
> encryption=AES128
> 
> That does make trouble with e.g. btrfs, because mkfs.btrfs uses the
> devicename provided by losetup to check if a device is mounted. That
> can't work with an incomplete devicename.

mkfs.btrfs authors goofed. Using truncated name is doomed to fail. Correct
way to check that is to compare loop backing file inode and device numbers
to inode and device numbers of supected file. Here is sample losetup output:

/dev/loop6: [0902]:209029 (/dev/md3) offset=4096 encryption=AES128 multi-key-v3
             ^^^^  ^^^^^^
          device     inode

Device number (0902) is hexadecimal. Inode number (209029) is decimal
number. /dev/md3 block special device node is present at mounted file system
on device 0x0902. Anything that symlinks or hardlinks to that inode 209029
on device 0x0902, is same file or device.
    
Even if losetup were to follow symlinks for that backing file name, symlink
destination would not necessarily be shorter. Although in your case it would
be shorter. Also, old versions of cryptoloop do not store the backing file
name at all. They use that same space for cipher algorithm name.

Below is source code for correct way for mkfs.btrfs to test loop backing
file status. Feel free to forward it to mkfs.btrfs authors.

-- 
Jari Ruusu  1024R/3A220F51 5B 4B F9 BB D3 3F 52 E9  DB 1D EB E3 24 0E A9 DD



/* loop-backing-dev-check.c / (c) 2011 Jari Ruusu / GNU GPL */

#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdlib.h>
#include <sys/ioctl.h>
#include <linux/loop.h>

int loop_backing_dev_check(char *loopdev, char *suspect)
{
    int fd, ret = 0;
    struct stat statbuf;
    struct loop_info64 info64;
    struct loop_info info;

    if(!loopdev || !*loopdev || !suspect || !*suspect) goto outret0;
    if(stat(loopdev, &statbuf) || !S_ISBLK(statbuf.st_mode)) goto outret0;
    if((statbuf.st_rdev & 0xfff00) != 0x700) goto outret0;
    if(stat(suspect, &statbuf)) goto outret0;
    if((fd = open(loopdev, O_RDONLY)) < 0) goto outret0;
    if(!ioctl(fd, LOOP_GET_STATUS64, &info64)) {
	if(statbuf.st_dev != info64.lo_device) goto closeret0;
	if(statbuf.st_ino != info64.lo_inode) goto closeret0;
	ret = 1;   /* loop backing device/file is same as suspect */
    } else if(!ioctl(fd, LOOP_GET_STATUS, &info)) {
	if(statbuf.st_dev != info.lo_device) goto closeret0;
	if(statbuf.st_ino != info.lo_inode) goto closeret0;
	ret = 1;   /* loop backing device/file is same as suspect */
    }
 closeret0:
    close(fd);
 outret0:
    return ret;
}

/* usage: ./loop-backing-dev-check /dev/loop0 /dev/fd0 */
int main(int argc, char **argv)
{
    if(argc != 3) exit(1);
    if(loop_backing_dev_check(argv[1], argv[2])) {
        printf("loop device %s is associated with %s\n", argv[1], argv[2]);
        exit(0);
    }
    exit(1);
}

Re: LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

[Petr Uzel]

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On Tue, Jan 25, 2011 at 11:15:11AM +1100, Chris Samuel wrote:
> /*
>  * CC'd to linux-kernel in case they have any feedback on this.
>  *
>  * Long thread, trying to work out why mkfs.btrfs failed to
>  * make a filesystem on an encrypted loopback mount called
>  * /dev/loop2. Cause turned out to be mkfs.btrfs calling
>  * LOOP_GET_STATUS to find out if the block device was mounted
>  * and getting a truncated device name back and so it later
>  * fails when lstat() is called on the truncated device path.
>  *
>  * The long device name for the encrypted loopback mount was
>  * because /dev/disk/by-id/$ID was used when Felix created it
>  * to cope with devices moving around.
>  */
> 
> On 25/01/11 00:01, Felix Blanke wrote:
> 
> > you were talking about the LOOP_GET_STATUS function. I'm not
> > quite sure where does it came from. Is it part of the kernel?
> > Or does it come from the util-linux package?
> 
> It's in the kernel, and there is both LOOP_GET_STATUS (old
> implementation) and LOOP_GET_STATUS64 (new implementation).
> 
> They return structures called loop_info and loop_info64
> respectively and both are defined in include/linux/loop.h .
> 
> Sadly in both cases the lengths of paths are defined to be
> LO_NAME_SIZE which is currently 64 and hence either
> implementation will cause the problematic:
> 
> lstat("/dev/disk/by-id/ata-INTEL_SSDSA2M160G2GC_CVPO939201JX160AGN-par",
> 0x7fffa30b3cf0) = -1 ENOENT (No such file or directory)
> 
> I've CC'd this to the LKML in case they have any feedback on
> this apparent problem with the API.
 
Since 2.6.37, you can get full path to the backing file from sys:
cat /sys/block/loopX/loop/backing_file

See
http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-07/msg10996.html


HTH,

Petr

--
Petr Uzel
IRC: ptr_uzl @ freenode

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

[Felix Blanke]

Hi,

are you sure that patch is in the kernel?

I'm using 2.6.37 and don't have those attribues in my /sys.



Felix

On 10. February 2011 - 13:29, Petr Uzel wrote:
> Date: Thu, 10 Feb 2011 13:29:27 +0100
> From: Petr Uzel <petr.uzel@suse.cz>
> To: Chris Samuel <chris@csamuel.org>
> Cc: Felix Blanke <felixblanke@gmail.com>, kreijack@inwind.it, Hugo Mills
>  <hugo-lkml@carfax.org.uk>, linux-btrfs@vger.kernel.org, Linux Kernel
>  <linux-kernel@vger.kernel.org>
> Subject: Re: LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re:
>  Bug in mkfs.btrfs?!)
> Mail-Followup-To: Chris Samuel <chris@csamuel.org>, Felix Blanke
>  <felixblanke@gmail.com>, kreijack@inwind.it, Hugo Mills
>  <hugo-lkml@carfax.org.uk>, linux-btrfs@vger.kernel.org, Linux Kernel
>  <linux-kernel@vger.kernel.org>
> 
> On Tue, Jan 25, 2011 at 11:15:11AM +1100, Chris Samuel wrote:
> > /*
> >  * CC'd to linux-kernel in case they have any feedback on this.
> >  *
> >  * Long thread, trying to work out why mkfs.btrfs failed to
> >  * make a filesystem on an encrypted loopback mount called
> >  * /dev/loop2. Cause turned out to be mkfs.btrfs calling
> >  * LOOP_GET_STATUS to find out if the block device was mounted
> >  * and getting a truncated device name back and so it later
> >  * fails when lstat() is called on the truncated device path.
> >  *
> >  * The long device name for the encrypted loopback mount was
> >  * because /dev/disk/by-id/$ID was used when Felix created it
> >  * to cope with devices moving around.
> >  */
> > 
> > On 25/01/11 00:01, Felix Blanke wrote:
> > 
> > > you were talking about the LOOP_GET_STATUS function. I'm not
> > > quite sure where does it came from. Is it part of the kernel?
> > > Or does it come from the util-linux package?
> > 
> > It's in the kernel, and there is both LOOP_GET_STATUS (old
> > implementation) and LOOP_GET_STATUS64 (new implementation).
> > 
> > They return structures called loop_info and loop_info64
> > respectively and both are defined in include/linux/loop.h .
> > 
> > Sadly in both cases the lengths of paths are defined to be
> > LO_NAME_SIZE which is currently 64 and hence either
> > implementation will cause the problematic:
> > 
> > lstat("/dev/disk/by-id/ata-INTEL_SSDSA2M160G2GC_CVPO939201JX160AGN-par",
> > 0x7fffa30b3cf0) = -1 ENOENT (No such file or directory)
> > 
> > I've CC'd this to the LKML in case they have any feedback on
> > this apparent problem with the API.
>  
> Since 2.6.37, you can get full path to the backing file from sys:
> cat /sys/block/loopX/loop/backing_file
> 
> See
> http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-07/msg10996.html
> 
> 
> HTH,
> 
> Petr
> 
> --
> Petr Uzel
> IRC: ptr_uzl @ freenode


---end quoted text---

Re: LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

[Milan Broz]

On 02/11/2011 02:04 PM, Felix Blanke wrote:
> I'm using 2.6.37 and don't have those attribues in my /sys.

These attributes are there only when the loop device is configured,
IOW when backing file is attached.


Milan

Re: LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

[Milan Broz]

On 02/11/2011 08:23 PM, Felix Blanke wrote:
> What do you mean with "configured"?
> 
> I'm using loop devices with loop aes, and I've looked into /sys for a device which is actually in use.

Ehm. It is really Loop-AES?

Then ask author to backport it there, Loop-AES is not mainline code.
He usually replaces the whole upstream loop implementation with old patched version.

Milan

Re: LOOP_GET_STATUS(64) truncates pathnames to 64 chars (was Re: Bug in mkfs.btrfs?!)

[Felix Blanke]

Yeah, for me its loop-aes.
Ah ok, didn't knew that it replaces that whole loop thing :)


Felix

On Feb 11, 2011 8:32 PM, "Milan Broz" <mbroz@redhat.com> wrote:
> On 02/11/2011 08:23 PM, Felix Blanke wrote:
>> What do you mean with "configured"?
>>
>> I'm using loop devices with loop aes, and I've looked into /sys for a device which is actually in use.
>
> Ehm. It is really Loop-AES?
>
> Then ask author to backport it there, Loop-AES is not mainline code.
> He usually replaces the whole upstream loop implementation with old patched version.
>
> Milan