On Sun, 14 Nov 2021 10:56:15 +0100 Patrick Steinhardt <p...@pks.im> wrote:
> On Tue, Oct 12, 2021 at 06:26:28PM -0500, Glenn Washburn wrote: > > Signed-off-by: Glenn Washburn <developm...@efficientek.com> > > --- > > grub-core/disk/cryptodisk.c | 26 +++++++++----------------- > > grub-core/disk/geli.c | 9 ++++----- > > grub-core/disk/luks.c | 11 +++++------ > > grub-core/disk/luks2.c | 6 +++--- > > include/grub/cryptodisk.h | 10 ++++++++-- > > 5 files changed, 29 insertions(+), 33 deletions(-) > > > > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > > index a5f7b860c..5b38606ed 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); > > if (grub_errno) > > return grub_errno; > > if (!dev) > > @@ -1049,7 +1046,7 @@ grub_cryptodisk_scan_device_real (const char *name, > > > > grub_cryptodisk_insert (dev, name, source); > > > > - have_it = 1; > > + cargs->found_uuid = 1; > > > > goto cleanup; > > } > > @@ -1091,7 +1088,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); > > If I didn't get this wrong, then all scan implementations > unconditionally dereference the `grub_cryptomount_args_t` pointer. > So why does this work, and shouldn't we pass down a struct which has the > `search_uuid` and `check_boot` parameters properly set up? I'm not sure that this does work, good catch. I don't have a setup to test GRUB utils in conjunction with cryptodisk. If you do (or anyone else does), testing would be greatly appreciated. Regardless, I believe this is an issue. My reading of the previous code is that search_uuid and check_boot are not explicitly initialized here. The only reasonable conclusion I come to is that they are initialized to zero by default by the compiler. So I'll create a cargs struct with all fields initalized to zero and pass that in. Does this sound good? Glenn _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel