diff mbox

[2/2] multiboot: Limit number of multiboot modules

Message ID 1255268921-5403-2-git-send-email-adam@os.inf.tu-dresden.de
State Under Review
Headers show

Commit Message

Adam Lackorzynski Oct. 11, 2009, 1:48 p.m. UTC 
From: Adam Lackorzynski <adam@os.inf.tu-dresden.de>

Add size checks to avoid overwriting the multiboot structure
when too many modules are loaded.

Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
---
 hw/pc.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

Comments

Kevin Wolf Oct. 12, 2009, 10:43 a.m. UTC | #1 
Am 11.10.2009 15:48, schrieb adam@os.inf.tu-dresden.de:
> From: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> 
> Add size checks to avoid overwriting the multiboot structure
> when too many modules are loaded.
> 
> Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>

Acked-by: Kevin Wolf <kwolf@redhat.com>

Looks correct, too, and is definitely an improvement over the current
code. But can't we make it more dynamic in a second step? I don't like
arbitrary fixed limits.

Kevin
Adam Lackorzynski Oct. 12, 2009, 4:40 p.m. UTC | #2 
On Mon Oct 12, 2009 at 12:43:38 +0200, Kevin Wolf wrote:
> Am 11.10.2009 15:48, schrieb adam@os.inf.tu-dresden.de:
> > From: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> > 
> > Add size checks to avoid overwriting the multiboot structure
> > when too many modules are loaded.
> > 
> > Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> 
> Acked-by: Kevin Wolf <kwolf@redhat.com>
> 
> Looks correct, too, and is definitely an improvement over the current
> code. But can't we make it more dynamic in a second step? I don't like
> arbitrary fixed limits.

Yes, definitely. I'm looking into this.



Adam
diff mbox

Patch

diff --git a/hw/pc.c b/hw/pc.c
index e34ad9c..b190d22 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -702,6 +702,10 @@  static int load_multiboot(void *fw_cfg,
         int mb_mod_count = 0;
 
         do {
+            if (mb_mod_info + 16 > mb_mod_cmdline) {
+                printf("WARNING: Too many modules loaded, aborting.\n");
+                break;
+            }
             next_initrd = strchr(initrd_filename, ',');
             if (next_initrd)
                 *next_initrd = '\0';
@@ -712,8 +716,11 @@  static int load_multiboot(void *fw_cfg,
                     initrd_filename);
             stl_p(bootinfo + mb_mod_info + 8, mb_bootinfo + mb_mod_cmdline); /* string */
             mb_mod_cmdline += strlen(initrd_filename) + 1;
-            if (mb_mod_cmdline > sizeof(bootinfo))
+            if (mb_mod_cmdline > sizeof(bootinfo)) {
                 mb_mod_cmdline = sizeof(bootinfo);
+                printf("WARNING: Too many module cmdlines loaded, aborting.\n");
+                break;
+            }
             if ((next_space = strchr(initrd_filename, ' ')))
                 *next_space = '\0';
 #ifdef DEBUG_MULTIBOOT