[PATCH] Print module name on license check failure

[PATCH] Print module name on license check failure

[Robbie Harwood]

At the very least, this will make it easier to track down the problem
module - or, if something else has gone wrong, provide more information
for debugging.

Signed-off-by: Robbie Harwood <rharwood@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);
 }
 
 static grub_err_t
@@ -641,8 +643,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)
       || grub_dl_resolve_dependencies (mod, e)
       || grub_dl_load_segments (mod, e)
       || grub_dl_resolve_symbols (mod, e)
-- 
2.33.0

Re: [PATCH] Print module name on license check failure

[Daniel Kiper]

On Thu, Oct 14, 2021 at 02:26:35PM -0400, Robbie Harwood wrote:
> At the very least, this will make it easier to track down the problem
> module - or, if something else has gone wrong, provide more information
> for debugging.
>
> Signed-off-by: Robbie Harwood <rharwood@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);
>  }
>
>  static grub_err_t
> @@ -641,8 +643,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 explain in the commit message why you change the
order of these functions calls. Otherwise LGTM.

Daniel