Re: [PATCH v2 2/3] commands/probe: improve support of LUKS2 devices

[Glenn Washburn]

On Tue, 29 Mar 2022 12:31:57 +0200
Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr> wrote:

> The --target=drive option of the probe command doesn't handle LUKS2
> devices:
> 
>   # dd if=/dev/zero of=data count=10 bs=1M
>   # losetup --show -f data
>   /dev/loop4
>   # echo -n pass | cryptsetup luksFormat -v --type luks2 /dev/loop4
>   Key slot 0 created.
>   Command successful.
>   # echo -n pass | cryptsetup -v open /dev/loop4 test
>   No usable token is available.
>   Key slot 0 unlocked.
>   Command successful.
>   # grub-probe --device /dev/mapper/test --target=drive
>   (hostdisk//dev/mapper/test)
> 
> The updated output:
> 
>   # grub-probe --device /dev/mapper/test --target=drive
>   (cryptouuid/be6f4e00637148a9a40453fe9cb7f0a5)
> ---
>  grub-core/osdep/devmapper/getroot.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/osdep/devmapper/getroot.c 
> b/grub-core/osdep/devmapper/getroot.c
> index 9ba5c9865..96781714c 100644
> --- a/grub-core/osdep/devmapper/getroot.c
> +++ b/grub-core/osdep/devmapper/getroot.c
> @@ -138,7 +138,8 @@ grub_util_get_dm_abstraction (const char *os_dev)
>        grub_free (uuid);
>        return GRUB_DEV_ABSTRACTION_LVM;
>      }
> -  if (strncmp (uuid, "CRYPT-LUKS1-", 12) == 0)
> +  if (strncmp (uuid, "CRYPT-LUKS1-", sizeof ("CRYPT-LUKS1-") - 1) == 0
> +      || strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) == 0)

I like using the sizeof() instead of a literal. And as mentioned in
comments on patch #3, it seems better to me to not add another strncmp,
but to only check for the prefix "CRYPT-LUKS".

>      {
>        grub_free (uuid);
>        return GRUB_DEV_ABSTRACTION_LUKS;

Glenn