[PATCH v10] plainmount: Support plain encryption mode

2022-12-28 Thread Maxim Fomin 
From e7f54a0b78353c01bf4c966f871b3e3590222743 Mon Sep 17 00:00:00 2001
From: Maxim Fomin 
Date: Wed, 28 Dec 2022 13:19:36 +
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 
---
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;
   }
   
  diff --git a/grub-core/fs/affs.c b/grub-core/fs/affs.c
  index 631d3d58a..ed606b3f1 100644
  --- a/grub-core/fs/affs.c
  +++ b/grub-core/fs/affs.c
  @@ -321,7 +321,6 @@ static int
   grub_affs_create_node (grub_fshelp_node_t dir,
   grub_fshelp_iterate_dir_hook_t hook, void *hook_data,
   struct grub_fshelp_node **node,
  -grub_uint32_t **hashtable,
   grub_uint32_t block, const struct grub_affs_file *fil)
   {
 struct grub_affs_data *data = dir->data;
  @@ -332,10 +331,7 @@ grub_affs_create_node (grub_fshelp_node_t dir,
   
 *node = grub_zalloc (sizeof (**node));
 if (!*node)
  -{
  -  grub_free (*hashtable);
  -  return 1;
  -}
  +return 1;
   
 (*node)->data = data;
 (*node)->block = block;
  @@ -395,7 +391,6 @@ grub_affs_create_node (grub_fshelp_node_t dir,
   
 if (hook ((char *) name_u8, type, *node, hook_data))
   {
  -  grub_free (*hashtable);
 *node = 0;
 return 1;
   }
  @@ -460,11 +455,11 @@ grub_affs_iterate_dir (grub_fshelp_node_t dir,
  if (grub_errno)
goto fail;
   
  -   if (grub_affs_create_node (dir, hook, hook_data, &node, &hashtable,
  -  next, &file))
  +   if (grub_affs_create_node (dir, hook, hook_data, &node, next, &file))
{
  /* Node has been replaced in function. */
  grub_free (orig_node);
  +   grub_free (hashtable);
  return 1;
}
   
  diff --git a/grub-core/fs/bfs.c b/grub-core/fs/bfs.c
  index a75876010..07cb3e3ac 100644
  --- a/grub-core/fs/bfs.c
  +++ b/grub-core/fs/bfs.c
  @@ -416,6 +416,8 @@ read_bfs_file (grub_disk_t disk,
len -= read_size;
buf = (char *) buf + read_size;
 }
  +grub_free (l1_entries);
  +grub_free (l2_entries);
   return GRUB_ERR_NONE;
 }
   }
  diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
  index ec72f7be3..19bff4610 100644
  --- a/grub-core/fs/btrfs.c
  +++ b/grub-core/fs/btrfs.c
  @@ -1982,7 +1982,12 @@ find_path (struct grub_btrfs_data *data,
{
  err = get_root (data, key, tree, type);
  if (err)
  - return err;
  + {
  +   grub_free (direl);
  +   grub_free (path_alloc);
  +   grub_free (origpath);
  +   return err;
  + }
}
  continue;
}
  diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
  index 6337cbfcb..11393ca34 100644
  --- a/grub-core/fs/hfsplus.c
  +++ b/grub-core/fs/hfsplus.c
  @@ -652,7 +652,10 @@ grub_hfsplus_btree_search (struct grub_hfsplus_btree 
*btree,
 + 2);
   
  if ((char *) pointer > node + btree->nodesize - 2)
  - return grub_error (GRUB_ERR_BAD_FS, "HFS+ key beyond end of 
node");
  + {
  +   grub_free (node);
  +   return grub_error (GRUB_ERR_BAD_FS, "HFS+ key beyond end of 
node");
  + }
   
  currnode = grub_be_to_cpu32 (grub_get_unaligned32 (pointer));

[PATCH v2] Fix possible integer overflow in i386-pc mode with large partitions

2022-12-28 Thread Maxim Fomin 
From f9e2970e5731f13fdc506dbf5c722fd24b20a1aa Mon Sep 17 00:00:00 2001
From: Maxim Fomin 
Date: Wed, 28 Dec 2022 20:25:05 +
Subject: [PATCH v2] Fix possible integer overflow in i386-pc mode with large
 partitions

The i386-pc mode supports MBR partition scheme
where maximum partition size is 2 TiB. In case
of large partitions left shift expression with
unsigned long int 'length' object may cause
integer overflow making calculated partition
size less than true value. This issue is fixed
by increasing the size of 'length' integer type.
---
 grub-core/kern/fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/kern/fs.c b/grub-core/kern/fs.c
index b9508296d..7ad0aaf4e 100644
--- a/grub-core/kern/fs.c
+++ b/grub-core/kern/fs.c
@@ -130,7 +130,7 @@ grub_fs_probe (grub_device_t device)
 struct grub_fs_block
 {
   grub_disk_addr_t offset;
-  unsigned long length;
+  grub_disk_addr_t length;
 };
 
 static grub_err_t
-- 
2.39.0



___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

2022-12-28 Thread Maxim Fomin 
--- Original Message ---
On Wednesday, December 28th, 2022 at 5:20 PM, Maxim Fomin  
wrote:

> From e7f54a0b78353c01bf4c966f871b3e3590222743 Mon Sep 17 00:00:00 2001
> From: Maxim Fomin ma...@fomin.one
> 
> Date: Wed, 28 Dec 2022 13:19:36 +
> 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 ma...@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. 

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'.

Best regards,
Maxim Fomin

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v8 1/1] plainmount: Support plain encryption mode

2022-12-28 Thread Maxim Fomin 
--- Original Message ---
On Saturday, December 24th, 2022 at 2:09 AM, Glenn Washburn 
 wrote:
> 
> On Fri, 23 Dec 2022 19:54:47 -0600
> Glenn Washburn developm...@efficientek.com wrote:
> 
> > On Fri, 02 Dec 2022 17:11:23 +
> > Maxim Fomin ma...@fomin.one wrote:
> > 
> > > --- Original Message ---
> > > On Friday, December 2nd, 2022 at 0:00, Glenn Washburn
> > > developm...@efficientek.com wrote:
> > > 
> > > > I'm now compiling this patch and found a few compile issues below.
> > > > You're compile testing this right?
> > > 
> > > First versions of the patch were tested in pure grub src directory.
> > > Later I switched to directly making and installing GRUB package for
> > > my distro using its source script syntax. It seems this process was
> > > affected by environment options which hided these errors/warnings.
> > > 
> > > I test the patch on my two old laptops - one with UEFI BIOS
> > > (x86_64-efi) and one is pre-UEFI (i386-pc). I was compiling i386-pc
> > > target too, because otherwise the second laptop was unbootable.
> > > During i386-pc compilation I noticed some warnings related to
> > > 'PRIuGRUB_XXX' macros which were absent during efi target
> > > compilation. I noticed that there are similar warnings in other
> > > modules and decided that there are issues with 'PRIuGRUB_XXX' macros
> > > at i386-pc platform at global level. In any case, these issues
> > > didn't cause compilation fail in my working environment because I
> > > would not be able to compile and boot pre-UEFI lap. Do you use
> > > -Werror?
> > 
> > I didn't see this until just now. In case you're still interested, no
> > I don't use -Werror or any special compiler flags. And I'm using gcc
> > version 10.2.1 from a Debian 11 container.
> 
> 
> Correction, -Werror is being used. Perhaps that's a default compiler
> flag on Debian systems.
> 
> Glenn
> 

This explains why you have found these issues. However, it does not explain
how you can compile grub with -Werror because currently there are following
warnings in x86_64-efi mode:
grub-core/lib/libgcrypt-grub/mpi/mpi-internal.h:150:24: warning: variable ‘_ql’ 
set but not used [-Wunused-but-set-variable]
grub-core/lib/libgcrypt-grub/mpi/mpih-div.c:53:9: warning: variable ‘dummy’ set 
but not used [-Wunused-but-set-variable]

When I was working with the patch earlier this year I remember having these
and several more warnings which prevented me from using -Werror. Back then
I have removed the switch and have forgotten about this issue completely.

Best regards,
Maxim Fomin


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel