From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kA61I-0003kj-Hg for mharc-grub-devel@gnu.org; Mon, 24 Aug 2020 02:31:52 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:34796) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kA61G-0003kV-6t for grub-devel@gnu.org; Mon, 24 Aug 2020 02:31:51 -0400 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:53715) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kA61E-0007Hk-0G for grub-devel@gnu.org; Mon, 24 Aug 2020 02:31:49 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id D6EB85C00C9; Mon, 24 Aug 2020 02:31:46 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Mon, 24 Aug 2020 02:31:46 -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=fm1; bh=o8A2gKVTPsYamSVhr8xchfmEwgU YO7pXBJPW00VSyno=; b=BvihOxnjxle3SZfQo7+zc9SYT7Lkg74trU6O8h8WlKD aLkphg9/izOnyoJVJDhyDB4DukHLOPoraIbbXIn4nPi1QcSmVXbg6eJSyGeRmX+3 oA7gUFEXxRskUiJhtR+uGn0YiayMK+B62L4V/2lyAy7N1SxGleKp7DLEJVOqPtLt 28Rsn/J4wSwA/2Oq1zg4V2gqTqj+49o95vd2qYxy05U6tmcgPCr8SdXxloDXuX8P T4SLmPjvXp0bCuH0R7UZLTTU3uv9Wp2oFcYPIoRkMgKC4ClNTPAH2KP5OxSC2PBj LRJzdTjCux/5d1QJxzfshTBB6ZHzNzR6V7OFZwg1AEg== 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=o8A2gK VTPsYamSVhr8xchfmEwgUYO7pXBJPW00VSyno=; b=oqvie4QKDgtRkFvntvnXaR AiD7lUDkHgyZ++b9KG5pPBs/LnHLDtTxDUH7JtU1tM/EAOw4SFwgA4fVXYrMUQa4 0MBn1ZxsMyxrvJ1i811jXy3Yc57q3EPA4cWyMoAX81TWeIncogw3nQ0lFnevFee/ LEHf//Brnn+l35P6Y7wuceyejaBFPqed1+8n7RU+qJOGsOSYwNeRla9Hz/IhBEUl 0xqeyq2+2aqvMtnKgw9oVu2+jgNk/lKwuZ2ddIGY3n3ESEhBOrMMg90GQp86b17o STp8Ck/NAReSEhFMJmQoaZQG1uaO+I1Mm4ETDkdHTvR+z0pVxwKwDXvojWS3pCVg == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedruddujedgkeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucfkphepjeejrdduuddrhedvrdejieenucevlhhushhtvghrufhiiigvpedtnecurfgrrh grmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Received: from vm-mail.pks.im (x4d0b344c.dyn.telefonica.de [77.11.52.76]) by mail.messagingengine.com (Postfix) with ESMTPA id A619C30600A9; Mon, 24 Aug 2020 02:31:45 -0400 (EDT) Received: from localhost (tanuki [10.192.0.23]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 81b064b7 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 24 Aug 2020 06:31:41 +0000 (UTC) Date: Mon, 24 Aug 2020 08:31:47 +0200 From: Patrick Steinhardt To: Glenn Washburn Cc: grub-devel@gnu.org, Denis 'GNUtoo' Carikli , Daniel Kiper Subject: Re: [PATCH 0/9] Cryptodisk fixes for v2.06 Message-ID: <20200824063147.GB953@tanuki.pks.im> References: <20200824012220.0fb938b7@crass-HP-ZBook-15-G2> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Pd0ReVV5GZGQvF3a" Content-Disposition: inline In-Reply-To: <20200824012220.0fb938b7@crass-HP-ZBook-15-G2> Received-SPF: pass client-ip=66.111.4.27; envelope-from=ps@pks.im; helo=out3-smtp.messagingengine.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/08/24 02:31:46 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] 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_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, 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: Mon, 24 Aug 2020 06:31:51 -0000 --Pd0ReVV5GZGQvF3a Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Aug 24, 2020 at 01:22:20AM -0500, Glenn Washburn wrote: > On Sun, 23 Aug 2020 12:59:47 +0200 > Patrick Steinhardt wrote: >=20 > > Hi, > >=20 > > I've sifted through the mailing list contents of the last few months > > to cherry-pick cryptodisk bugfixes which I think should be included > > in the v2.06 release. I've found the following 9 patches from Glenn > > and me which should probably be included, separated them out from > > their respective patch series and made them play nice with each other. > >=20 > > This patch series shouldn't be applied as-is, but my intention is > > instead to bundle all fixes which apply to v2.06 in a single thread to > > make discussion easier and help us keep track of what needs to be > > done. I've got some comments which I've sent to the original threads > > already and added notes below. > >=20 > > - luks2: grub_cryptodisk_t->total_length is the max number of device > > native sectors > >=20 > > I'm not sure if this fix is correct, mostly because I think that > > `grub_disk_get_size` is buggy already: it returns sectors for > > partitions and the total size for disks. So I do think we need > > another patch to fix that function, too. >=20 > Its not clear to me what "total size" means here. However, > `grub_disk_get_size` returns sectors for non-partition disks as well. > Note, that it returns the total number of grub native sector-sized > sectors (ie 512 byte sectors). I do think that the _size suffix is > misleading and should be named `grub_disk_get_sectors` or something > similar. Oh, yeah. I've misunderstood the translation from disk sector size to GRUB-native sector size. I heavily agree with you that there should be some code cleanup after v2.06 which does s/grub_disk_get_size/grub_disk_get_sectors/ for this and other related code. It's just too easy to shoot yourself in the foot here if you're not knowing a 100% what you're doing. > Is there something I can do to clear up the uncertainty around this > fix? I suspect part of the confusion lies in the fact that the > total_length field is actually in cryptodisk sector-sized sectors. We > know this because in `grub_cryptodisk_open` in cryptodisk.c the > total_length field is being set unmodified to the total_sectors field of > a `grub_disk_t` and `grub_disk_get_size` assume that total_sectors > needs to be converted from disk native to grub native sector-sized > sectors. No, I've read it again and will add my Reviewed-by for this patch. > > - cryptodisk: Properly handle non-512 byte sized sectors > >=20 > > Should we pick this for v2.06? It definitely fixes things, but > > also feels a bit like feature-enablement. >=20 > I see you fixed the bug in my patch where IV_PLAIN IVs were not being > calculated, thanks. Considering grubs slow release cycle, I think its > better to include this or at least make a note that non-512-byte > sectors are currently not supported and have cryptomount fail with an > appropriate error message if it detects trying to open a LUKS2 device > with sector size not equal to 512. As it is, I think LUKS2 support > will have people thinking that non-default sector sizes are supported. I should've added a note in here about my fixup. And let's do include it then. > > I've added my Reviewed-by to those patches which look obviously > > correct to me. > >=20 > > Glenn, please let me know if this somehow interferes with your work or > > if you'd like to handle upstreaming of those fixes yourself. >=20 > I would like to get these patches in a quickly as possible and I > suspect this patchset is probably that route, so I'm pleased to have > these patches in here. =20 >=20 > As a reminder, my CRYPTOMOUNT-TESTS patchset can test the full patch > set (not individual patches because non-512-byte sector support > requires multiple patches in this patchset). I'll have a look at that patchset soonish. Patrick --Pd0ReVV5GZGQvF3a Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAl9DXtIACgkQVbJhu7ck PpQsQBAAnLXoiWVzTX9ex3aOtegZWadPXrZA/CDKr4/yTbq0QQ2Vao/8RdTtlrvh KBd64dLDR6Pw5eQZ3SljgYSFI2+wSPb/4S6He2V9f+J5m/kpBVn2jiAdxOVVz03O RdgGTzbZ/rj9+33wbx06OG57f0PZRVmY47KFMMqpp7m/swXt1sNndi3DKm5gZQjL MdoRktxC55lfkX3UGjDnS2SGtPQ8vf76CAIkA6rR1MnL4daTtyVEOpVD8cKYenQ+ 2pSpZ+dTdLtWYJ4NG9Jo5uFmprxcAPvA/F5EHibyuTPgV+tzBduSv9//gIh+mERh Oriz4241Ckue9nP2FcfaYw0tB7V++K02N3br4jZgQiTM3H3kAgA8DSMpXmQ0i7+f 0BU8eQUJarI/OlQOgsJJkuo6Iyb315457+18KBbS6cq8exLX1wLkHumsISW6GvJc QWTz4kswtSBVFWM3hKC5SfKa/Kp3qbeZBVVK3HtxBgSr0KEtoqWlT6dSRn1kWnF1 c97kBPEuH0ANdM5AdJkKcNdZAlOOTpuhQdduUHj22VFUqQHsCeKIkRfEHoTn6dsY U5N1D3I7S2ycMH/WUrqB4C3QjzcYFXir75IfW8XhD7FsfFegStgYfa/4YBsEeTj5 662/GfalaUdvZzYK9pQvrBBdCX/c/jrISuvqDev5gDnYs6Rsts0= =FTSl -----END PGP SIGNATURE----- --Pd0ReVV5GZGQvF3a--