On Wed, Jun 15, 2022 at 12:02:29PM +0200, Josselin Poiret via Grub-devel wrote: > This lets a LUKS2 cryptodisk have its cipher and hash filled out, > otherwise they wouldn't be initialized if cheat mounted.
Please add your Signed-off-by here. Same applies to first patch too. > --- > grub-core/osdep/devmapper/getroot.c | 87 ++++++++++++++++++++++++++++- > 1 file changed, 86 insertions(+), 1 deletion(-) > > diff --git a/grub-core/osdep/devmapper/getroot.c > b/grub-core/osdep/devmapper/getroot.c > index 2bf4264cf..80c7e54da 100644 > --- a/grub-core/osdep/devmapper/getroot.c > +++ b/grub-core/osdep/devmapper/getroot.c > @@ -51,6 +51,8 @@ > #include <grub/emu/misc.h> > #include <grub/emu/hostdisk.h> > > +#include <grub/cryptodisk.h> > + > static int > grub_util_open_dm (const char *os_dev, struct dm_tree **tree, > struct dm_tree_node **node) > @@ -186,7 +188,6 @@ grub_util_pull_devmapper (const char *os_dev) > && lastsubdev) > { > char *grdev = grub_util_get_grub_dev (lastsubdev); > - dm_tree_free (tree); > if (grdev) > { > grub_err_t err; > @@ -194,7 +195,91 @@ grub_util_pull_devmapper (const char *os_dev) > if (err) > grub_util_error (_("can't mount encrypted volume `%s': %s"), > lastsubdev, grub_errmsg); > + if (strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) == > 0) > + { > + /* set LUKS2 cipher from dm parameters, since it is not > + * possible to determine the correct one without > + * unlocking, as there might be multiple segments. > + */ Please format comments, here and below, properly [1]. > + grub_disk_t source; > + grub_cryptodisk_t cryptodisk; > + grub_uint64_t start, length; > + char *target_type; > + char *params; > + const char *name; > + char *cipher, *cipher_mode; > + struct dm_task *dmt; > + char *seek_head, *c; > + unsigned int remaining; > + > + source = grub_disk_open (grdev); This function may fail. Please check the returned value and fail if needed. > + cryptodisk = grub_cryptodisk_get_by_source_disk (source); Ditto. > + grub_disk_close (source); > + > + name = dm_tree_node_get_name (node); I think same applies here... > + grub_util_info ("populating parameters of cryptomount `%s' > from DM device `%s'", > + uuid, name); > + > + dmt = dm_task_create (DM_DEVICE_TABLE); > + if (dmt == 0) s/0/NULL/ Please use NULL instead of 0 for pointers. Same below... > + grub_util_error (_("can't create dm task DM_DEVICE_TABLE")); Yes, fail properly like here... > + if (dm_task_set_name (dmt, name) == 0) > + grub_util_error (_("can't set dm task name to `%s'"), name); > + if (dm_task_run (dmt) == 0) > + grub_util_error (_("can't run dm task for `%s'"), name); > + /* dm_get_next_target doesn't have any error modes, everything > has > + * been handled by dm_task_run. > + */ > + dm_get_next_target (dmt, NULL, &start, &length, > + &target_type, ¶ms); > + if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0) > + grub_util_error (_("dm target of type `%s' is not `crypt'"), > + target_type); I am OK with lines a bit longer than 80. So, you can put this in one line. > + /* dm target parameters for dm-crypt is > + * <cipher> <key> <iv_offset> <device path> <offset> > [<#opt_params> <opt_param1> ...] > + */ > + c = params; > + remaining = grub_strlen (c); > + > + /* first, get the cipher name from the cipher */ > + if (!(seek_head = grub_memchr (c, '-', remaining))) > + grub_util_error (_("can't get cipher from dm-crypt > parameters `%s'"), > + params); > + cipher = grub_strndup (c, seek_head - c); grub_strndup() may fail. Please check if cipher != NULL. > + remaining -= seek_head - c + 1; > + c = seek_head + 1; > + > + /* now, the cipher mode */ > + if (!(seek_head = grub_memchr (c, ' ', remaining))) seek_head = grub_memchr (c, ' ', remaining); if (seek_head == NULL) > + grub_util_error (_("can't get cipher mode from dm-crypt > parameters `%s'"), > + params); > + cipher_mode = grub_strndup (c, seek_head - c); Again, grub_strndup() may fail. In general please do not ignore errors from functions which may fail. > + remaining -= seek_head - c + 1; > + c = seek_head + 1; > + > + err = grub_cryptodisk_setcipher (cryptodisk, cipher, > cipher_mode); > + if (err) > + grub_util_error (_("can't set cipher of cryptodisk `%s' to > `%s' with mode `%s'"), > + uuid, cipher, cipher_mode); > + > + grub_free (cipher); > + grub_free (cipher_mode); > + > + /* This is the only hash usable by PBKDF2, and we don't > + * have Argon2 support yet, so set it by default, > + * otherwise grub-probe would miss the required > + * abstraction > + */ > + cryptodisk->hash = grub_crypto_lookup_md_by_name ("sha256"); > + if (cryptodisk->hash == 0) > + grub_util_error (_("can't lookup hash sha256 by name")); > + > + dm_task_destroy (dmt); > + } > } > + dm_tree_free (tree); > grub_free (grdev); > } > else Daniel [1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Comments _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel