[PATCH] loader/multiboot: do not add modules before successful download

[PATCH] loader/multiboot: do not add modules before successful download

[Valentin Gehrke]

Multiboot modules that could not be read successfully (e.g. via network)
should not be added to the list of modules to forward to the operating
system that is to be booted subsequently.

This patch is necessary because even if a grub.cfg checks whether or not
a module was successfully downloaded, an attempt to retry a failed
download is futile because the corrupted module will be forwarded either
way.

Signed-off-by: Valentin Gehrke <valentin.gehrke@kernkonzept.com>
---
 grub-core/loader/multiboot.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 94be512c4..36b27a906 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -420,13 +420,6 @@ grub_cmd_module (grub_command_t cmd __attribute__
((unused)), target = 0;
     }
 
-  err = GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
-  if (err)
-    {
-      grub_file_close (file);
-      return err;
-    }
-
   if (size && grub_file_read (file, module, size) != size)
     {
       grub_file_close (file);
@@ -437,7 +430,8 @@ grub_cmd_module (grub_command_t cmd __attribute__
((unused)), }
 
   grub_file_close (file);
-  return GRUB_ERR_NONE;
+
+  return GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv +
1); }
 
 static grub_command_t cmd_multiboot, cmd_module;
-- 
2.45.2

---
Valentin Gehrke
Software Engineer
Kernkonzept GmbH at Dresden, Germany,
HRB 31129, CEO Dr.-Ing. Michael Hohmuth

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] loader/multiboot: do not add modules before successful download

[Daniel Kiper]

On Fri, Aug 23, 2024 at 08:20:53PM +0200, Valentin Gehrke wrote:
> Multiboot modules that could not be read successfully (e.g. via network)
> should not be added to the list of modules to forward to the operating
> system that is to be booted subsequently.
>
> This patch is necessary because even if a grub.cfg checks whether or not
> a module was successfully downloaded, an attempt to retry a failed
> download is futile because the corrupted module will be forwarded either
> way.
>
> Signed-off-by: Valentin Gehrke <valentin.gehrke@kernkonzept.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] loader/multiboot: do not add modules before successful download

[Daniel Kiper]

On Fri, Aug 23, 2024 at 08:20:53PM +0200, Valentin Gehrke wrote:
> Multiboot modules that could not be read successfully (e.g. via network)
> should not be added to the list of modules to forward to the operating
> system that is to be booted subsequently.
>
> This patch is necessary because even if a grub.cfg checks whether or not
> a module was successfully downloaded, an attempt to retry a failed
> download is futile because the corrupted module will be forwarded either
> way.
>
> Signed-off-by: Valentin Gehrke <valentin.gehrke@kernkonzept.com>

Sorry, this patch does not apply to the upstream.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] loader/multiboot: do not add modules before successful download

[Valentin Gehrke]

> Sorry, this patch does not apply to the upstream.
>
> Daniel

Oops. My bad. There was some unfortunate line wrapping in the diff
in the @@ lines caused by my email client. This time it should work.
I rebased the commit onto current master, just to be sure.

From: Valentin Gehrke <valentin.gehrke@kernkonzept.com>
Date: Fri, 23 Aug 2024 16:41:23 +0200
Subject: [PATCH] loader/multiboot: do not add modules before successful
 download

Multiboot modules that could not be read successfully (e.g. via network)
should not be added to the list of modules to forward to the operating
system that is to be booted subsequently.

This patch is necessary because even if a grub.cfg checks whether or not
a module was successfully downloaded, an attempt to retry a failed
download is futile because the corrupted module will be forwarded either
way.

Signed-off-by: Valentin Gehrke <valentin.gehrke@kernkonzept.com>
---
 grub-core/loader/multiboot.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 94be512c4..36b27a906 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -420,13 +420,6 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
       target = 0;
     }
 
-  err = GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
-  if (err)
-    {
-      grub_file_close (file);
-      return err;
-    }
-
   if (size && grub_file_read (file, module, size) != size)
     {
       grub_file_close (file);
@@ -437,7 +430,8 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
     }
 
   grub_file_close (file);
-  return GRUB_ERR_NONE;
+
+  return GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
 }
 
 static grub_command_t cmd_multiboot, cmd_module;
-- 
2.46.0

---
Valentin Gehrke
Software Engineer
Kernkonzept GmbH at Dresden, Germany,
HRB 31129, CEO Dr.-Ing. Michael Hohmuth

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] loader/multiboot: do not add modules before successful download

[Valentin Gehrke]

Hey Daniel,

did you have time yet to give the patch in my last email
a try? I just want to make sure the email didn't get
overlooked.

Best regards,
Valentin.

On Tue, 10 Sep 2024 16:10:47 +0200
Valentin Gehrke <valentin.gehrke@kernkonzept.com> wrote:

> > Sorry, this patch does not apply to the upstream.
> >
> > Daniel  
> 
> Oops. My bad. There was some unfortunate line wrapping in the diff
> in the @@ lines caused by my email client. This time it should work.
> I rebased the commit onto current master, just to be sure.
> 
> From: Valentin Gehrke <valentin.gehrke@kernkonzept.com>
> Date: Fri, 23 Aug 2024 16:41:23 +0200
> Subject: [PATCH] loader/multiboot: do not add modules before
> successful download
> 
> Multiboot modules that could not be read successfully (e.g. via
> network) should not be added to the list of modules to forward to the
> operating system that is to be booted subsequently.
> 
> This patch is necessary because even if a grub.cfg checks whether or
> not a module was successfully downloaded, an attempt to retry a failed
> download is futile because the corrupted module will be forwarded
> either way.
> 
> Signed-off-by: Valentin Gehrke <valentin.gehrke@kernkonzept.com>


---
 grub-core/loader/multiboot.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 94be512c4..36b27a906 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -420,13 +420,6 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
       target = 0;
     }
 
-  err = GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
-  if (err)
-    {
-      grub_file_close (file);
-      return err;
-    }
-
   if (size && grub_file_read (file, module, size) != size)
     {
       grub_file_close (file);
@@ -437,7 +430,8 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
     }
 
   grub_file_close (file);
-  return GRUB_ERR_NONE;
+
+  return GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
 }
 
 static grub_command_t cmd_multiboot, cmd_module;
-- 
2.46.0

---
Valentin Gehrke
Software Engineer
Kernkonzept GmbH at Dresden, Germany,
HRB 31129, CEO Dr.-Ing. Michael Hohmuth

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] loader/multiboot: do not add modules before successful download

[Daniel Kiper]

Hey,

On Fri, Oct 25, 2024 at 01:59:25PM +0200, Valentin Gehrke wrote:
> Hey Daniel,
>
> did you have time yet to give the patch in my last email
> a try? I just want to make sure the email didn't get
> overlooked.

Sorry, it looks it fell through the cracks.

Anyway, I think I was going to ask you to use "git send-email" to send
the patch. So, please do. You can add Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
to the patch...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] loader/multiboot: do not add modules before successful download

[Valentin Gehrke]

Multiboot modules that could not be read successfully (e.g. via network)
should not be added to the list of modules to forward to the operating
system that is to be booted subsequently.

This patch is necessary because even if a grub.cfg checks whether or not
a module was successfully downloaded, it is futile to retry a failed
download as the corrupted module will be forwarded either way.

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 grub-core/loader/multiboot.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 94be512c4..36b27a906 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -420,13 +420,6 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
       target = 0;
     }
 
-  err = GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
-  if (err)
-    {
-      grub_file_close (file);
-      return err;
-    }
-
   if (size && grub_file_read (file, module, size) != size)
     {
       grub_file_close (file);
@@ -437,7 +430,8 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
     }
 
   grub_file_close (file);
-  return GRUB_ERR_NONE;
+
+  return GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
 }
 
 static grub_command_t cmd_multiboot, cmd_module;
-- 
2.46.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] loader/multiboot: do not add modules before successful download

[Valentin Gehrke]

Multiboot modules that could not be read successfully (e.g. via network)
should not be added to the list of modules to forward to the operating
system that is to be booted subsequently.

This patch is necessary because even if a grub.cfg checks whether or not
a module was successfully downloaded, it is futile to retry a failed
download as the corrupted module will be forwarded either way.

Signed-off-by: Valentin Gehrke <valentin.gehrke@kernkonzept.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 grub-core/loader/multiboot.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 94be512c4..36b27a906 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -420,13 +420,6 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
       target = 0;
     }
 
-  err = GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
-  if (err)
-    {
-      grub_file_close (file);
-      return err;
-    }
-
   if (size && grub_file_read (file, module, size) != size)
     {
       grub_file_close (file);
@@ -437,7 +430,8 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
     }
 
   grub_file_close (file);
-  return GRUB_ERR_NONE;
+
+  return GRUB_MULTIBOOT (add_module) (target, size, argc - 1, argv + 1);
 }
 
 static grub_command_t cmd_multiboot, cmd_module;
-- 
2.46.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel