[PULL 09/15] pc-bios/s390-ccw: bound zipl menu strlen and replace VLA in zipl_print_entry

From: Joshua Daley <[email protected]>

menu_get_zipl_boot_index() calls strlen() on a pointer into the middle
of _s2 with no upper bound, so a stage-2 image whose blocks contain no
NUL bytes causes strlen() to walk beyond _s2. The resulting length
is then used to size a stack VLA in zipl_print_entry(), risking a stack
overflow.

Fix by:

- Implementing strnlen(), a bounded version of strlen(). s390-ccw uses
  libc from SLOF, which includes strlen() but does not have an
  implementation of strnlen(), so we must implement our own.

- Adding a menu_data_end parameter to menu_get_zipl_boot_index() and
  replacing both strlen() calls with strnlen() bounded by the remaining
  buffer space. The loop guard also checks that the pointer has not
  reached menu_data_end. The function returns 0 (boot default) if
  somehow menu_data reaches menu_data_end before printing any entries.

- Replacing the VLA char buf[len + 2] in zipl_print_entry() with a fixed
  ZIPL_ENTRY_MAX + 2 (82-byte) buffer and truncating len before use.

- Passing s2_end (_s2 + sizeof(_s2)) as menu_data_end at the one call
  site in eckd_get_boot_menu_index(), so the bound is exactly the end of
  the buffer.

Fixes: f7178910845a ("s390-ccw: print zipl boot menu")
Cc: [email protected]
Signed-off-by: Joshua Daley <[email protected]>
Reviewed-by: Matthew Rosato <[email protected]>
Link: https://lore.kernel.org/qemu-devel/[email protected]
[[email protected]: Per list, add strnlen rationale to commit message
 and added cc stable]
Signed-off-by: Eric Farman <[email protected]>
---
 pc-bios/s390-ccw/bootmap.c  |  4 +++-
 pc-bios/s390-ccw/helper.h   | 10 ++++++++++
 pc-bios/s390-ccw/menu.c     | 29 +++++++++++++++++++++++------
 pc-bios/s390-ccw/s390-ccw.h |  2 +-
 4 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index ed6e8cbbc7..81512265ce 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -61,6 +61,7 @@ static uint8_t _s2[MAX_SECTOR_SIZE * 3] __attribute__((__aligned__(PAGE_SIZE)));
 static void *s2_prev_blk = _s2;
 static void *s2_cur_blk = _s2 + MAX_SECTOR_SIZE;
 static void *s2_next_blk = _s2 + MAX_SECTOR_SIZE * 2;
+static void *s2_end = _s2 + sizeof(_s2);
 
 static inline int verify_boot_info(BootInfo *bip)
 {
@@ -308,7 +309,8 @@ static int eckd_get_boot_menu_index(block_number_t s1b_block_nr)
                 }
             }
 
-            return menu_get_zipl_boot_index(s2_cur_blk + banner_offset);
+            return menu_get_zipl_boot_index(s2_cur_blk + banner_offset,
+                                            s2_end);
         }
 
         prev_block_nr = cur_block_nr;
diff --git a/pc-bios/s390-ccw/helper.h b/pc-bios/s390-ccw/helper.h
index 8e3dfcb6d6..d9b7da444a 100644
--- a/pc-bios/s390-ccw/helper.h
+++ b/pc-bios/s390-ccw/helper.h
@@ -45,4 +45,14 @@ static inline void sleep(unsigned int seconds)
     }
 }
 
+static inline size_t strnlen(const char *s, size_t maxlen)
+{
+    size_t len = 0;
+
+    while (len < maxlen && s[len]) {
+        len++;
+    }
+    return len;
+}
+
 #endif
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index b6a9a56d46..9b81154b0e 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -16,6 +16,7 @@
 #include "s390-ccw.h"
 #include "sclp.h"
 #include "s390-time.h"
+#include "helper.h"
 
 #define KEYCODE_NO_INP '\0'
 #define KEYCODE_ESCAPE '\033'
@@ -26,6 +27,9 @@
 #define ZIPL_TIMEOUT_OFFSET 138
 #define ZIPL_FLAG_OFFSET    140
 
+/* Max printable chars for a zipl boot menu entry */
+#define ZIPL_ENTRY_MAX 80
+
 #define TOD_CLOCK_MILLISECOND   0x3e8000
 
 #define LOW_CORE_EXTERNAL_INT_ADDR   0x86
@@ -179,9 +183,13 @@ int menu_get_boot_index(bool *valid_entries)
 /* Returns the entry number that was printed, or -1 on invalid entry */
 static int zipl_print_entry(const char *data, size_t len)
 {
-    char buf[len + 2];
+    char buf[ZIPL_ENTRY_MAX + 2];
     const char *p;
 
+    if (len > ZIPL_ENTRY_MAX) {
+        len = ZIPL_ENTRY_MAX;
+    }
+
     ebcdic_to_ascii(data, buf, len);
     buf[len] = '\n';
     buf[len + 1] = '\0';
@@ -196,7 +204,7 @@ static int zipl_print_entry(const char *data, size_t len)
     return atoi(p);
 }
 
-int menu_get_zipl_boot_index(const char *menu_data)
+int menu_get_zipl_boot_index(const char *menu_data, const char *menu_data_end)
 {
     size_t len;
     int entry;
@@ -212,13 +220,22 @@ int menu_get_zipl_boot_index(const char *menu_data)
         timeout = zipl_timeout * 1000;
     }
 
-    /* Print banner */
+    if (menu_data >= menu_data_end) {
+        return 0; /* Boot default */
+    }
+
+    /* Skip banner */
+    len = strnlen(menu_data, menu_data_end - menu_data);
+    menu_data += len + 1;
+    if (menu_data >= menu_data_end || !(*menu_data)) {
+        return 0; /* No entries, boot default */
+    }
+
     puts("s390-ccw zIPL Boot Menu\n");
-    menu_data += strlen(menu_data) + 1;
 
     /* Print entries */
-    while (*menu_data) {
-        len = strlen(menu_data);
+    while (menu_data < menu_data_end && *menu_data) {
+        len = strnlen(menu_data, menu_data_end - menu_data);
         entry = zipl_print_entry(menu_data, len);
         menu_data += len + 1;
 
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 1e1f71775e..f6030a6071 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -76,7 +76,7 @@ void jump_to_low_kernel(void);
 
 /* menu.c */
 void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
-int menu_get_zipl_boot_index(const char *menu_data);
+int menu_get_zipl_boot_index(const char *menu_data, const char *menu_data_end);
 bool menu_is_enabled_zipl(void);
 int menu_get_enum_boot_index(bool *valid_entries);
 bool menu_is_enabled_enum(void);
-- 
2.55.0