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_error()
> > statements using the wrong index in constructing the error string.
> >
> > Signed-off-by: Glenn Washburn <developm...@efficientek.com>
> > ---
> > 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 = 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
> > %"PRIuGRUB_SIZE, i);
> > + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest
> > %"PRIuGRUB_SIZE, j);
> >
> > if ((d->keyslots & (1 << idx)))
> > break;
> > }
> > if (j == size)
> > - return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot
> > %"PRIuGRUB_SIZE);
> > + return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot
> > %"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 = 0; j < size; j++)
> > + for (i = j, j = 0; j < size; j++)
>
> Should not it be "i = j = 0" instead of "i = j, j = 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 == 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);
>
> s/PRIuGRUB_SIZE, i/PRIuGRUB_SIZE, j/?
>
> Daniel
... 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 <p...@pks.im>
Patrick
signature.asc
Description: PGP signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
