On Wed, Oct 20, 2021 at 11:19:13AM -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 | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c > index 48f8a7907..90e83e4d4 100644 > --- a/grub-core/kern/dl.c > +++ b/grub-core/kern/dl.c > @@ -457,14 +457,16 @@ 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)) > return GRUB_ERR_NONE; > - return grub_error (GRUB_ERR_BAD_MODULE, "incompatible license"); > + return grub_error (GRUB_ERR_BAD_MODULE, > + "incompatible license in module %s: %s", mod->name, > + (char *) e + s->sh_offset);
The Covertiy complains with this: *** CID 361072: Null pointer dereferences (FORWARD_NULL) /grub-core/kern/dl.c: 467 in grub_dl_check_license() 461 { 462 Elf_Shdr *s = grub_dl_find_section (e, ".module_license"); 463 if (s && (grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv3") == 0 464 || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv3+") == 0 465 || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=GPLv2+") == 0)) 466 return GRUB_ERR_NONE; >>> CID 361072: Null pointer dereferences (FORWARD_NULL) >>> Dereferencing null pointer "s". 467 return grub_error (GRUB_ERR_BAD_MODULE, "incompatible license in module %s: %s", 468 mod->name, (char *) e + s->sh_offset); 469 } I think you should rework the if conditional and split it into two parts. First you should check if s is not NULL. If it is then print an error, e.g. "no license section in %s module". Then second if checking for licenses. Additionally, I think we trust to widely input from the ELF files here. I would limit all comparisons and messages to first e.g. 64 characters from the "(char *) e + s->sh_offset". This begs for second patch. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel