[pkg-cryptsetup-devel] Bug#546610: Bug#546610: cryptsetup: root= cmdline call is not honoured by initrd in crypto+lvm (Was initramfs-tools: ...)

[Agustin Martin]

On Thu, Feb 16, 2012 at 02:26:57PM +0100, Jonas Meurer wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hey August,
> 
> thanks for commenting again on this bugreport. See my comments below.
> 
> Am 16.02.2012 13:14, schrieb Agustin Martin:
> > I have later extended the test in the lilo booted box to be able to
> > boot to two different partitions on lvm, one with testing (where
> > all kernels are installed and images created, although modules are
> > put in the shared partition) and one running stable. With my
> > patched cryptsetup I can boot from both.
> ...
> > By the way, patch still appplies cleanly to 1.4 and seems to work
> > properly, both in my lilo and grub booted boxes.
> 
> That reads like your patch is well tested. 

Hi again, Jonas, thanks for the quick reply

> questions, and need to do further testing with other unusual setups
> before applying it. 

I tested it in my particular unusual setup, that is indeed not very general.

> Maybe you can help here.

Happy to try helping.

> This is your patch:
> 
> > diff --git a/debian/initramfs/cryptroot-script
> b/debian/initramfs/cryptroot-script
> > index f83e52e..9de28a4 100644 ---
> > a/debian/initramfs/cryptroot-script +++
> > b/debian/initramfs/cryptroot-script @@ -326,7 +326,7 @@
> > setup_mapping() return 1 fi
> > 
> > -			NEWROOT="/dev/mapper/$cryptlvm" +
> > NEWROOT=${cmdline_root=/dev/mapper/$cryptlvm}
> 
> In my tests, this didn't work as expected. For some reason, $cryptlvm
> is ignored within the specified default value for NEWROOT.

Not sure if related to the above, but I should have used

NEWROOT=${cmdline_root:-/dev/mapper/$cryptlvm}

Not using ":" might cause portability issues, and there is no need to change
$cmdline_root value so ":-" instead of "=" seems better.

Do not have the lilo booted box here to test and remember the details. I
however added some debugging lines just after that definition,

message "cmdline_root: $cmdline_root"
message "cryptlvm: $cryptlvm"
message "NEWROOT: $NEWROOT"

and values seem correct in my grub booted box. Need to test more carefully.

> > @@ -354,18 +354,35 @@ setup_mapping() #
> > 
> > # Do we have any kernel boot arguments? -found='' 
> > +cmdline_cryptopts='' +unset cmdline_root for opt in $(cat
> > /proc/cmdline); do case $opt in cryptopts=*) -		found=yes -
> > setup_mapping "${opt#cryptopts=}" +
> > opt="${opt#cryptopts=}" +		if [ -n "$opt" ]; then +		    if [ -n
> > "$cmdline_cryptopts" ]; then +
> > cmdline_cryptopts="$cmdline_cryptopts,$opt" +		    else +
> > cmdline_cryptopts="$opt" +		    fi +		fi ;; +        root=*) +
> > opt="${opt#root=}" +		case $opt in +		    /*) # Absolute path
> > given. Not lilo major/minor number. +			cmdline_root=$opt +			;; +
> > *) # lilo major/minor number (See #398957). Ignore +		esac +
> > ;; esac done
> > 
> > -if [ -n "$found" ]; then -	exit 0 +if [ -n "$cmdline_cryptopts" ];
> > then +    setup_mapping "$cmdline_cryptopts" +    exit 0 fi
> > 
> > # Do we have any settings from the /conf/conf.d/cryptroot file?
> 
> This will break systems with more than one cryptopts-parameter.
> Currently, it should work to give several cryptopts-parameters as boot
> parameters and that way unlock more than one cryptdisk within the
> initramfs. your patch will give all cryptopts parameters as one
> argument seperated by commas to setup_mapping at once, instead of
> invoking setup_mapping for every single cryptopts-parameter. Do you
> get my point? I guess it would be enough to change the seperator from
> comma to space and do something like 'for cryptopt in
> $cmdline_cryptopts; do setup_mapping "$cryptopt"; done' later.

You are right. I have tested using 

if [ -n "$cmdline_cryptopts" ]; then
    # Call setup_mapping separately for each possible cryptopts= setting
    for cryptopt in $cmdline_cryptopts; do
       setup_mapping "$cryptopt"
    done
    exit 0
fi

as you propose and seems to work well in my simple case. Not tested with
multiple cryptopts= calls.

I am attaching a diff with current status of my changes. I hope to test in a
lilo booted box in some days. Will let you know.

Regards,

-- 
Agustin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: initramfs_cryptroot-script_20120216.diff
Type: text/x-diff
Size: 1509 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-cryptsetup-devel/attachments/20120216/16a15ebd/attachment.diff>