[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] cryptodisk: Move global variables into grub_cryptomount_
From: |
Patrick Steinhardt |
Subject: |
Re: [PATCH 3/3] cryptodisk: Move global variables into grub_cryptomount_args struct |
Date: |
Mon, 30 Aug 2021 20:02:26 +0200 |
On Thu, Aug 26, 2021 at 12:08:52AM -0500, Glenn Washburn wrote:
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> grub-core/disk/cryptodisk.c | 26 +++++++++-----------------
> include/grub/cryptodisk.h | 3 +++
> 2 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index b6cf1835d..00a671a59 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -984,9 +984,6 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
>
> #endif
>
> -static int check_boot, have_it;
> -static char *search_uuid;
> -
> static void
> cryptodisk_close (grub_cryptodisk_t dev)
> {
> @@ -1014,7 +1011,7 @@ grub_cryptodisk_scan_device_real (const char *name,
>
> FOR_CRYPTODISK_DEVS (cr)
> {
> - dev = cr->scan (source, search_uuid, check_boot);
> + dev = cr->scan (source, cargs->search_uuid, cargs->check_boot);
> if (grub_errno)
> return grub_errno;
> if (!dev)
> @@ -1051,7 +1048,7 @@ grub_cryptodisk_scan_device_real (const char *name,
>
> grub_cryptodisk_insert (dev, name, source);
>
> - have_it = 1;
> + cargs->found_uuid = 1;
>
> goto cleanup;
> }
> @@ -1093,7 +1090,7 @@ grub_cryptodisk_cheat_mount (const char *sourcedev,
> const char *cheat)
>
> FOR_CRYPTODISK_DEVS (cr)
> {
> - dev = cr->scan (source, search_uuid, check_boot);
> + dev = cr->scan (source, NULL, 0);
> if (grub_errno)
> return grub_errno;
> if (!dev)
> @@ -1137,7 +1134,7 @@ grub_cryptodisk_scan_device (const char *name,
>
> if (err)
> grub_print_error ();
> - return have_it && search_uuid ? 1 : 0;
> + return (cargs->found_uuid && cargs->search_uuid) ? 1 : 0;
> }
>
> static grub_err_t
> @@ -1155,7 +1152,6 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
> argc, char **args)
> cargs.key_len = grub_strlen(state[3].arg);
> }
>
> - have_it = 0;
> if (state[0].set) /* uuid */
> {
> grub_cryptodisk_t dev;
> @@ -1168,21 +1164,18 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
> argc, char **args)
> return GRUB_ERR_NONE;
> }
>
> - check_boot = state[2].set;
> - search_uuid = args[0];
> + cargs.check_boot = state[2].set;
> + cargs.search_uuid = args[0];
> grub_device_iterate (&grub_cryptodisk_scan_device, &cargs);
> - search_uuid = NULL;
>
> - if (!have_it)
> + if (!cargs.found_uuid)
> return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk found");
> return GRUB_ERR_NONE;
> }
> else if (state[1].set || (argc == 0 && state[2].set)) /* -a|-b */
> {
> - search_uuid = NULL;
> - check_boot = state[2].set;
> + cargs.check_boot = state[2].set;
> grub_device_iterate (&grub_cryptodisk_scan_device, &cargs);
> - search_uuid = NULL;
> return GRUB_ERR_NONE;
> }
> else
> @@ -1194,8 +1187,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
> argc, char **args)
> char *disklast = NULL;
> grub_size_t len;
>
> - search_uuid = NULL;
> - check_boot = state[2].set;
> + cargs.check_boot = state[2].set;
> diskname = args[0];
> len = grub_strlen (diskname);
> if (len && diskname[0] == '(' && diskname[len - 1] == ')')
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index 1070140d9..11062f43a 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -69,6 +69,9 @@ typedef gcry_err_code_t
>
> struct grub_cryptomount_args
> {
> + grub_uint32_t check_boot : 1;
> + grub_uint32_t found_uuid : 1;
> + char *search_uuid;
> grub_uint8_t *key_data;
> grub_size_t key_len;
> };
Aren't these parameters in a different scope than the key data? These
are only used for device discovery via `scan()`, while the other ones
are for decrypting the key. Do we want to split those up into two
different structs?
Patrick
signature.asc
Description: PGP signature