From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mPNUS-0002lx-1S for mharc-grub-devel@gnu.org; Sun, 12 Sep 2021 07:17:40 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:57636) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mPNUP-0002lm-4E for grub-devel@gnu.org; Sun, 12 Sep 2021 07:17:38 -0400 Received: from wout1-smtp.messagingengine.com ([64.147.123.24]:50221) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mPNUN-0005C7-8N for grub-devel@gnu.org; Sun, 12 Sep 2021 07:17:36 -0400 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 438DB3200124; Sun, 12 Sep 2021 07:17:33 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Sun, 12 Sep 2021 07:17:33 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm2; bh=cssXgM7+zhSYkBgIxJY/N9ckQN7 +m+YqA7lr6ZXQUb4=; b=VgCLZthumYibfLQLhTYFgF9imEGoQwlv9MGiTBihkKu 9sNZNjNOFnCJmZFyMNeZDNvMxLMGnS2T9W/u/FafCV9udOye0j+3A7G9VcVpUV+K TMSLkoLfJyRelWTK/7jS7iRTw8xrmdqX96WaANPl0ftds7SDjz9FL+xcu3jIidML hcmmmXQsFDHezBWIOq96CCSJ5X689pL24l8QR08odF6CvxHhOLXYEAGTLr2Y9/SS BPoOjgzblTthdsttt1fbNcmJkOtKoCf3Trl4DuBnXpYrLn0xCw1lbQqYo8Zw03CP XTe2cVfxlgGVkG8iGWHxuYyh8Fgp8OO1XX8rmbkejcw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=cssXgM 7+zhSYkBgIxJY/N9ckQN7+m+YqA7lr6ZXQUb4=; b=hdfX6lkykOtgE1CMap/RNr Ael9jEdi1s6tbHCFVJUTLzT6F0R0vOVCFk+7dg9okTgM0KG+lq02CO0rMV5Dkaln p8p7RiWPLog2Wfxaou60QkyR6WneA3V1A1WuREviY0WjbZzOqta0esjE0oDmBNvW HdmJOlfKucS7rJAN3z/PY3U0UAjU8yaGqyYbYcxrjoW42zHhhNjm0OL8EI0x9S1K 5cHGS8umQEQY/FEkBYJdYpOAzQoNkkw9hWDXk9tx4bobXD9bw5WUBXzOPSSb2kCm 7q088vySFUiYBvFYaiZwtlFA+L+AtQ+/+/V6sUXjrlx1M6kANh97gXfzypx6oJPQ == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudeghedgfeeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 12 Sep 2021 07:17:31 -0400 (EDT) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 49dada70 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sun, 12 Sep 2021 11:17:29 +0000 (UTC) Date: Sun, 12 Sep 2021 13:17:29 +0200 From: Patrick Steinhardt To: Glenn Washburn Cc: grub-devel@gnu.org, Daniel Kiper Subject: Re: [PATCH 3/3] cryptodisk: Move global variables into grub_cryptomount_args struct Message-ID: References: <20210907023430.2eaff8a1@ubuntu> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="l42tHVUJeBwJOD5U" Content-Disposition: inline In-Reply-To: <20210907023430.2eaff8a1@ubuntu> Received-SPF: pass client-ip=64.147.123.24; envelope-from=ps@pks.im; helo=wout1-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_PASS=-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: Sun, 12 Sep 2021 11:17:38 -0000 --l42tHVUJeBwJOD5U Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 07, 2021 at 02:34:30AM +0000, Glenn Washburn wrote: > On Mon, 30 Aug 2021 20:02:26 +0200 > Patrick Steinhardt wrote: >=20 > > 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(-) > > >=20 > > > 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) > > > =20 > > > #endif > > > =20 > > > -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,=20 > > > FOR_CRYPTODISK_DEVS (cr) > > > { > > > - dev =3D cr->scan (source, search_uuid, check_boot); > > > + dev =3D 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,=20 > > > grub_cryptodisk_insert (dev, name, source); > > > =20 > > > - have_it =3D 1; > > > + cargs->found_uuid =3D 1; > > > =20 > > > goto cleanup; > > > } > > > @@ -1093,7 +1090,7 @@ grub_cryptodisk_cheat_mount (const char > > > *sourcedev, const char *cheat)=20 > > > FOR_CRYPTODISK_DEVS (cr) > > > { > > > - dev =3D cr->scan (source, search_uuid, check_boot); > > > + dev =3D cr->scan (source, NULL, 0); > > > if (grub_errno) > > > return grub_errno; > > > if (!dev) > > > @@ -1137,7 +1134,7 @@ grub_cryptodisk_scan_device (const char *name, > > > =20 > > > if (err) > > > grub_print_error (); > > > - return have_it && search_uuid ? 1 : 0; > > > + return (cargs->found_uuid && cargs->search_uuid) ? 1 : 0; > > > } > > > =20 > > > static grub_err_t > > > @@ -1155,7 +1152,6 @@ grub_cmd_cryptomount (grub_extcmd_context_t > > > ctxt, int argc, char **args) cargs.key_len =3D > > > grub_strlen(state[3].arg); } > > > =20 > > > - have_it =3D 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; > > > } > > > =20 > > > - check_boot =3D state[2].set; > > > - search_uuid =3D args[0]; > > > + cargs.check_boot =3D state[2].set; > > > + cargs.search_uuid =3D args[0]; > > > grub_device_iterate (&grub_cryptodisk_scan_device, &cargs); > > > - search_uuid =3D NULL; > > > =20 > > > - 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 =3D=3D 0 && state[2].set)) /* -a|-b= */ > > > { > > > - search_uuid =3D NULL; > > > - check_boot =3D state[2].set; > > > + cargs.check_boot =3D state[2].set; > > > grub_device_iterate (&grub_cryptodisk_scan_device, &cargs); > > > - search_uuid =3D NULL; > > > return GRUB_ERR_NONE; > > > } > > > else > > > @@ -1194,8 +1187,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t > > > ctxt, int argc, char **args) char *disklast =3D NULL; > > > grub_size_t len; > > > =20 > > > - search_uuid =3D NULL; > > > - check_boot =3D state[2].set; > > > + cargs.check_boot =3D state[2].set; > > > diskname =3D args[0]; > > > len =3D grub_strlen (diskname); > > > if (len && diskname[0] =3D=3D '(' && diskname[len - 1] =3D=3D = ')') > > > 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 > > > =20 > > > 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; > > > }; > >=20 > > 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? >=20 > 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? >=20 > Glenn I'm mostly wondering about lifetimes of these parameters. They are used in different phases of the cryptomount: some are used only at the time of discovery, while others are used at decryption time, where it's not immediately clear which parameters are used when without having a look at the cryptodisk implementations. That's why I was thinking it might make more sense to split them up by those phases such that this becomes explicit. Patrick --l42tHVUJeBwJOD5U Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmE94cgACgkQVbJhu7ck PpRunRAAigvOGpI3xSJf6rhRL9wX9Nd4dzeQV5fOfB8DqJ1AuQ1gIY4Z05m05Ss0 8GRdlp3sNKK1svo0goh7b4+G1U+mFxAaEame4IWtg+SQf61Gh6x2jtj+LqDvs8hz K61mXNhWOXFSrD/f1zinRbExWj92LoJgyRJriLIZBKY+TWThwtyQuIWVv8DvnONN O+MIc+Oj2HPYKtM3i2QYj6AYVwvmstOSQuWfp1Y6ljb6qna+xYtmyqe3Lvk9JmN4 Y5y1hYAMQBZMQ4T441z0bZLvlshaaeECsXP8BgTOlAfrL7iqIlgNawtMQstb5Mzi 82Bn0KMnFJ4CrQxlTsJkfaZ/zbDxT6hZcp7qH+yXczIaYox3FiLgyBiHlIhEoIPs Co/tfZtpxZrBtfInC9ej3o66Avx5IFt7zVSP48IND1gtzAFUc2jyCrKadBvV1mOz 2ywxDXW34V6jn0cRpI5++qCrVgxdAwk7Gh97eAQbjgKumRB7q+T2FrOHp9Jjtyxx vFBWNHMNRAvWy42/HpCEKSj/9qdi+X/p5QWsCf9Iquvem2AzYjHLFWeB1cClHAGz Eeb7PHLPwUWV8FpEH5RZgyAUrm8bUHAXHuidrGRJey15pE9zUjhLY8goRwG8zzNz kdmCy0z2eEMbc4Ld6Ej0yzPwV3R356qYGMIAM1P2UuDZ5JbpaF4= =M9fy -----END PGP SIGNATURE----- --l42tHVUJeBwJOD5U--