Re: [PATCH v10] plainmount: Support plain encryption mode

[Daniel Kiper]

On Wed, Dec 28, 2022 at 05:47:29PM +0000, Maxim Fomin wrote:
> ------- Original Message -------
> On Wednesday, December 28th, 2022 at 5:20 PM, Maxim Fomin <maxim@fomin.one> 
> wrote:
>
> > From e7f54a0b78353c01bf4c966f871b3e3590222743 Mon Sep 17 00:00:00 2001
> > From: Maxim Fomin maxim@fomin.one
> >
> > Date: Wed, 28 Dec 2022 13:19:36 +0000
> > Subject: [PATCH v10] plainmount: Support plain encryption mode
> >
> > This patch adds support for plain encryption mode (plain dm-crypt) via
> > new module/command named 'plainmount'.
> >
> > Signed-off-by: Maxim Fomin maxim@fomin.one
> >
> > ---
> > Interdiff against v9:
> > diff --git a/grub-core/disk/plainmount.c b/grub-core/disk/plainmount.c
> > index eabedf4c3..47e64805f 100644
> > --- a/grub-core/disk/plainmount.c
> > +++ b/grub-core/disk/plainmount.c
> > @@ -326,7 +326,7 @@ grub_cmd_plainmount (grub_extcmd_context_t ctxt, int 
> > argc, char *args)
> > * Arbitrary data keys are supported via keyfiles.
> > /
> > password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> > - grub_memcpy (key_data, state[OPTION_PASSWORD].arg, password_size);
> > + grub_strcpy ((char) key_data, state[OPTION_PASSWORD].arg);
> > }
> >
> > / Set cipher mode (tested above) */
> > @@ -334,9 +334,16 @@ grub_cmd_plainmount (grub_extcmd_context_t ctxt, int 
> > argc, char **args)
> > mode++ = '\0';
> >
> > / Check cipher */
> > - if (grub_cryptodisk_setcipher (dev, cipher, mode) != GRUB_ERR_NONE)
> > + err = grub_cryptodisk_setcipher (dev, cipher, mode);
> > + if (err != GRUB_ERR_NONE)
> > {
> > - err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid cipher %s"), cipher);
> > + if (err == GRUB_ERR_FILE_NOT_FOUND)
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid cipher %s"), cipher);
> > + else if (err == GRUB_ERR_BAD_ARGUMENT)
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid mode %s"), mode);
> > + else
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid cipher %s or mode 
> > %s"),
> > + cipher, mode);
> > goto fail;
> > }
> >
>
> Hi, Daniel. Unfortunately I was very busy last time and could not get back to 
> the patch earlier.

No problem...

> I have removed one instance of memcpy() where the function operates on C 
> string. Other calls to
> memcpy() operate on data buffers. Also, I improved error message where cipher 
> was correct, but
> the mode was wrong. In previous version for input 'aes-xts-blah' error was 
> 'invalid cipher aes'.

Now your patch is in the upstream. Thank you for doing this work!

Daniel