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;
/* 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.53.0
