On Mon, Oct 25, 2021 at 06:17:03PM -0400, Robbie Harwood wrote: > Before performing the license check, resolve the module name so that it > can be printed if the license check fails. Prior to this change, grub > would only indicate that the check had been failed, but not by what > module. This made it difficult to track down either the problem module, > or debug the false positive further. > > Signed-off-by: Robbie Harwood <rharw...@redhat.com> > --- > grub-core/kern/dl.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c > index 48f8a7907..363bacc43 100644 > --- a/grub-core/kern/dl.c > +++ b/grub-core/kern/dl.c > @@ -457,14 +457,21 @@ grub_dl_find_section (Elf_Ehdr *e, const char *name) > Be sure to understand your license obligations. > */ > static grub_err_t > -grub_dl_check_license (Elf_Ehdr *e) > +grub_dl_check_license (grub_dl_t mod, Elf_Ehdr *e) > { > Elf_Shdr *s = grub_dl_find_section (e, ".module_license"); > - if (s && (grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv3") == 0 > - || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv3+") == 0 > - || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv2+") == 0)) > + if (!s)
s == NULL please... > + return grub_error (GRUB_ERR_BAD_MODULE, > + "no license section in module %.64s", mod->name); > + > + if (grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv3") == 0 > + || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv3+") == 0 > + || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv2+") == 0) Could use grub_strncmp() instead of grub_strcmp() here? > return GRUB_ERR_NONE; > - return grub_error (GRUB_ERR_BAD_MODULE, "incompatible license"); > + > + return grub_error (GRUB_ERR_BAD_MODULE, > + "incompatible license in module %.64s: %.64s", mod->name, > + (char *) e + s->sh_offset); > } > > static grub_err_t > @@ -641,8 +648,8 @@ grub_dl_load_core_noinit (void *addr, grub_size_t size) > constitutes linking) and GRUB core being licensed under GPLv3+. > Be sure to understand your license obligations. > */ > - if (grub_dl_check_license (e) > - || grub_dl_resolve_name (mod, e) > + if (grub_dl_resolve_name (mod, e) > + || grub_dl_check_license (mod, e) I think you should split this patch into two. One which improves error messages and another which imposes length restrictions on the grub_strcmp() and grub_error() in the grub_dl_check_license(). And I think you should use 63 instead of 64 (63 + NUL gives us 64 :-)). Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel