From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mNQyx-0002L5-VV for mharc-grub-devel@gnu.org; Mon, 06 Sep 2021 22:37:07 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:47638) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mNQyw-0002Ji-Qb for grub-devel@gnu.org; Mon, 06 Sep 2021 22:37:06 -0400 Received: from mail-io1-xd2d.google.com ([2607:f8b0:4864:20::d2d]:42675) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mNQyu-0000Od-F9 for grub-devel@gnu.org; Mon, 06 Sep 2021 22:37:06 -0400 Received: by mail-io1-xd2d.google.com with SMTP id b10so10846344ioq.9 for ; Mon, 06 Sep 2021 19:37:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references:reply-to :mime-version:content-transfer-encoding; bh=QIGNGlP7xuh1YpQrt8nKCadP8+mm6b7v1eR+eryWOLs=; b=euVU2QH7ep1NqhbZCjaXoqUCLsX8f03nqTRHvv7u6083kJpEq+B9Yp9ddvbwQjZpFz 5x7+Mkkz3RABlL+BRN6Y+/gviM3Ci01v/63w8r+CA/VL3qmstRl3wyeRakf7M9YjLuQT 6UG/u4bBUP1LXKdCME2yt1EUwhl3zV99K+3ayzJNRLuDKIIjaCx+xeoTP7zFOpqgUtoj kbKdXOmyJxq2d8e1Bu06IX/z+NZlEhDn3ZfOEI3JgEM6T49IfGPzFbH/oU9zLrWTrHm2 Nf2Uyg30Y5EKAUjGCn7eG59Y57x5mJ5ar4zjlDUC+fiHtWBIQGcO7HZ35LwYhjl42ryO sVag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:reply-to:mime-version:content-transfer-encoding; bh=QIGNGlP7xuh1YpQrt8nKCadP8+mm6b7v1eR+eryWOLs=; b=MnN0oIkpytMS2bcnP/KudaiUgviJkyPwip9hadf4VY1ylYinnpwoPAZqp8KbiZf7Zz QNBgz1BUU6nA1HaZGJ0Fxh4EaI1p3WYYvwK63tOrwRTAWWEKclH088sAEg3q/mEHEnRK CstDQIAdolADbtmHvJZLFFNSdGZeZZlMugg5hgY6cR/Z2/VFzz+MlWeNWTjUs5klIWj/ VvDys8oVwD/BApnwuEDJHesVvU0FuOLI0dk13O+I5BlhDpttpfdRroKUBsYpqJ/oQJD8 LCYDeBFTosApzLyYwjZ1IjrE+K64iVTXSMq8Gt1X3Mr1/oWerla2K8J2fTZAPgEAsIT0 bT3Q== X-Gm-Message-State: AOAM533uizBrCUkQsY5tArXq7hJ6cZ4zKP6W4SYaiI9rBkCMzfobspmO dsHuqw9onFGon+3xcuGBq7CVPw== X-Google-Smtp-Source: ABdhPJzrfFA8p9OPI0tVfxJ1kDUm7tGZtdxzBcqYdnZ10z8Jd6d1nStGkVs0BHGCW6muVZ0kf+1KWA== X-Received: by 2002:a5d:9409:: with SMTP id v9mr11897566ion.170.1630982222381; Mon, 06 Sep 2021 19:37:02 -0700 (PDT) Received: from ubuntu (public-nat-04.vpngate.v4.open.ad.jp. [219.100.37.236]) by smtp.gmail.com with ESMTPSA id g14sm5667135ila.28.2021.09.06.19.36.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Sep 2021 19:37:02 -0700 (PDT) Date: Tue, 7 Sep 2021 02:34:30 +0000 From: Glenn Washburn To: Patrick Steinhardt Cc: grub-devel@gnu.org, Daniel Kiper Subject: Re: [PATCH 3/3] cryptodisk: Move global variables into grub_cryptomount_args struct Message-ID: <20210907023430.2eaff8a1@ubuntu> In-Reply-To: References: Reply-To: development@efficientek.com X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2607:f8b0:4864:20::d2d; envelope-from=development@efficientek.com; helo=mail-io1-xd2d.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Sep 2021 02:37:07 -0000 On Mon, 30 Aug 2021 20:02:26 +0200 Patrick Steinhardt wrote: > On Thu, Aug 26, 2021 at 12:08:52AM -0500, Glenn Washburn wrote: > > Signed-off-by: Glenn Washburn > > --- > > 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? This struct is meant to be used for any data passed to the crypto backend from cryptomount. All of those members are affected by cryptomount options. So this struct isn't about anything in particular, just a common set of data passed to the crypto backends via cryptomount. So I don't think two structs would improve anything here. Am I missing something? Glenn