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().
- 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. For robustness, the function returns 0
(boot default) if somehow menu_data is initially >= menu_data_end.
- 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")
Signed-off-by: Joshua Daley <[email protected]>
---
pc-bios/s390-ccw/bootmap.c | 4 +++-
pc-bios/s390-ccw/helper.h | 10 ++++++++++
pc-bios/s390-ccw/menu.c | 23 ++++++++++++++++++-----
pc-bios/s390-ccw/s390-ccw.h | 2 +-
4 files changed, 32 insertions(+), 7 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..af96a83de9 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,18 @@ int menu_get_zipl_boot_index(const char *menu_data)
timeout = zipl_timeout * 1000;
}
+ if (menu_data >= menu_data_end) {
+ return 0;
+ }
+
/* Print banner */
puts("s390-ccw zIPL Boot Menu\n");
- menu_data += strlen(menu_data) + 1;
+ len = strnlen(menu_data, menu_data_end - menu_data);
+ menu_data += len + 1;
