From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kW1Eg-0003AN-Pq for mharc-grub-devel@gnu.org; Fri, 23 Oct 2020 13:52:18 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39714) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kW1Ef-00038R-13 for grub-devel@gnu.org; Fri, 23 Oct 2020 13:52:17 -0400 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:39537) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kW1Ea-0003jl-Ai for grub-devel@gnu.org; Fri, 23 Oct 2020 13:52:16 -0400 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 52B39B1E; Fri, 23 Oct 2020 13:52:07 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 23 Oct 2020 13:52:07 -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=+loM+1RXj8rDCQ/9H08JqDq5Z17 cebZkrNseqejv6cg=; b=VcS5dyACokSZCLmd6DjDDQeLYERt7iM8H2u0SI7Gs0W ExjWV5dPb4aB1QJYG+0kb+ingtK/3WEE5LwHAaq+oKgaqOptYOZYkb178EZy4SK9 w802WyiBF/iEw1xy0dokiM9FvVBTjLXUPnWVijEwNvfEkdFmZUGzK/iKp5yxRO+m /VlxRtd1or3d9mTecXu7G+I59kUGrzoOoaR+OLNYm/eHTPWXJgZZbCPrLcrpnFqy OfqOfvIFpRa2et28NWIhzMnivecPba5gUa5uhHF8T0GeGZcFlmmVo/UfTj3qP9mf 8t0HWRoeOJmA6HjiORiiBU5YpXmOIgKcDKqhqjAX0Ow== 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=fm1; bh=+loM+1 RXj8rDCQ/9H08JqDq5Z17cebZkrNseqejv6cg=; b=DnSQN0BU9UZ60JN/drfYI7 anyNiDESPjunT3mIt+aIC6ZG4UqLRNevcSP2nEuV+IBtdZkc/xV67sz+AaIFDkz8 Je7OGcDjXo7Uvuqmy3HPMLhaS7FrDWr4zG7XjOC8G/tDXUqmuu7yfUWG47HsmYqk conz9TitPL9syqDwlNZX+vR+4c62qxllqTPT3VmKca5YlxG1eI9hAHhciECJr+JF H31/CKyc5Uk9DFDUa9UkRKjWR8OacjJFHeGqU8fJCZdt/hiQ6IMpWKjdVh6M9FCm HMoGcl4N0aooFKPMRDqNyAuFqSbWMtrqUYCdilvGA5Ucr9ymX8IKP45q7Dx5qyLA == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrkedtgdduudelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgefhtdefueffheekgfffudelffejtdfhvdejkedthfehvdelgfetgfdvtedthfen ucfkphepkeelrddugedrgeeirddvtddtnecuvehluhhsthgvrhfuihiivgeptdenucfrrg hrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: from vm-mail.pks.im (dynamic-089-014-046-200.89.14.pool.telefonica.de [89.14.46.200]) by mail.messagingengine.com (Postfix) with ESMTPA id DA7B43064686; Fri, 23 Oct 2020 13:52:05 -0400 (EDT) Received: from localhost (tanuki [10.192.0.23]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 3e51ddee (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Oct 2020 17:52:04 +0000 (UTC) Date: Fri, 23 Oct 2020 19:52:11 +0200 From: Patrick Steinhardt To: The development of GNU GRUB Cc: Glenn Washburn Subject: Re: [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_error() messages. Message-ID: <20201023175211.GB810@tanuki> References: <20201009100122.GH2088@tanuki> <80dd653c989818e86f219cbfc83be9115c06ccd2.1603148099.git.development@efficientek.com> <20201023120818.iqss6nawibucw63x@tomti.i.net-space.pl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="RASg3xLB4tUQ4RcS" Content-Disposition: inline In-Reply-To: <20201023120818.iqss6nawibucw63x@tomti.i.net-space.pl> Received-SPF: pass client-ip=64.147.123.20; envelope-from=ps@pks.im; helo=wout4-smtp.messagingengine.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/10/23 13:46:38 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.001, RCVD_IN_MSPIKE_WL=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: Fri, 23 Oct 2020 17:52:17 -0000 --RASg3xLB4tUQ4RcS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 23, 2020 at 02:08:18PM +0200, Daniel Kiper wrote: > On Mon, Oct 19, 2020 at 06:09:49PM -0500, Glenn Washburn wrote: > > When looping over the digests and segments, the loop variable is j, but= the > > variable i is used to index in the the digests and segments json array.= The > > variable i is the keyslot index. Similarly, there are several grub_erro= r() > > statements using the wrong index in constructing the error string. > > > > Signed-off-by: Glenn Washburn > > --- > > grub-core/disk/luks2.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c > > index 31d7166fc..2241e0312 100644 > > --- a/grub-core/disk/luks2.c > > +++ b/grub-core/disk/luks2.c > > @@ -275,34 +275,34 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_= luks2_digest_t *d, grub_luks2_s > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not get digests"); > > for (j =3D 0; j < size; j++) > > { > > - if (grub_json_getchild (&digest, &digests, i) || > > + if (grub_json_getchild (&digest, &digests, j) || > > grub_json_getchild (&digest, &digest, 0) || > > luks2_parse_digest (d, &digest)) > > - return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest %"P= RIuGRUB_SIZE, i); > > + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest %"P= RIuGRUB_SIZE, j); > > > > if ((d->keyslots & (1 << idx))) > > break; > > } > > if (j =3D=3D size) > > - return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keysl= ot %"PRIuGRUB_SIZE); > > + return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keysl= ot %"PRIuGRUB_SIZE, i); > > > > /* Get segment that matches the digest. */ > > if (grub_json_getvalue (&segments, root, "segments") || > > grub_json_getsize (&size, &segments)) > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not get segments"= ); > > - for (j =3D 0; j < size; j++) > > + for (i =3D j, j =3D 0; j < size; j++) >=20 > Should not it be "i =3D j =3D 0" instead of "i =3D j, j =3D 0"? The intent is to save the digest index here... > > { > > - if (grub_json_getchild (&segment, &segments, i) || > > + if (grub_json_getchild (&segment, &segments, j) || > > grub_json_getuint64 (&idx, &segment, NULL) || > > grub_json_getchild (&segment, &segment, 0) || > > luks2_parse_segment (s, &segment)) > > - return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment %"= PRIuGRUB_SIZE, i); > > + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment %"= PRIuGRUB_SIZE, j); > > > > if ((d->segments & (1 << idx))) > > break; > > } > > if (j =3D=3D size) > > - return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest= %"PRIuGRUB_SIZE); > > + return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest= %"PRIuGRUB_SIZE, i); >=20 > s/PRIuGRUB_SIZE, i/PRIuGRUB_SIZE, j/? >=20 > Daniel =2E.. which then gets used here. It initially confused me as well, but it should be correct. The problem is the awkward naming, but the patch following this one improves the situation. Reviewed-by: Patrick Steinhardt Patrick --RASg3xLB4tUQ4RcS Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAl+TGEoACgkQVbJhu7ck PpTvzg//ezXb3Jtgjxax070/4NrpIAj/ph1OTRbGEZg+U9+XOs3NELgDVV4P8qjn uDtoXmldFRsL8VLByOyMfXpcWSmHc7WlwTTDuOOtjv8FQGE9SnHXg2axQfh8fwtM +4aQVXhz0l98HiYibu77GqSUXdvhns3Qb1PkgvkXPC086TKxiBh7QXpsHDEYaL8a S/vMTLVrR7aO87fJLXOWQGTnDwFflt4Wjcc5qxKqqVOU+smkQcRM+hy5aYFJxUU8 lNmAWW+bIkqVdgRWWdHE750L1bMge8WIAXIRXi26qDgahB5K7xKMk0PDesLzWaaW oe6LDk/QWv3EXW1JmwhjCgt/2AeWt3d9cUn9nRK5RNsM8v0tm/x6z63VzxTOU2uS A60s9NyNIEPFY310Ey2slS9rN0lH8DAAAXYTDueN8SWZm44aO/G6y4CCEuNLyfr7 BSN4nCTzBeEJxWvTzhlHYgECynB1h4YIy4iRLDk0G1sFdL1aaammIWtKWZpNkG/5 LyOe5f/gsLeH8w4iwu2DPf8Dgpxr1hGwhjaTQLIYNWV7oOc9r3LSjM0FGoJW5HT8 1H5aAE1Q6y19828FgpmSR6rbuzIWiTeKHDfqPx0KS24eWsSaQIa0ZkX/TVQctOH2 RtedoSLu/gcxNq6ssc7PmwupwfmpDsUUu70g8tl0WL6taZnRDwY= =wte4 -----END PGP SIGNATURE----- --RASg3xLB4tUQ4RcS--