Re: [PATCH 4/5] pc-bios/s390-ccw: bounds-check zipl menu entry index before array write

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 7/27/26 7:50 AM, Christian Borntraeger wrote:
> From: Joshua Daley <jdaley@linux.ibm.com>
> 
> menu_get_zipl_boot_index() iterates NUL-separated strings from the
> zipl stage-2 boot-menu block, passes each to zipl_print_entry() which
> converts EBCDIC to ASCII and returns atoi(), then writes true into
> valid_entries[entry]. valid_entries is a MAX_BOOT_ENTRIES element
> stack array, but entry was never bounds-checked, so a crafted on-disk
> value could index arbitrarily beyond the array.
> 
> Fix this in two places:
> 
> - zipl_print_entry() now validates that the first significant character
>   (after an optional leading space) is a digit. Entries that fail this
>   check return -1 without printing.
> 
> - menu_get_zipl_boot_index() skips any entry whose index is outside
>   [0, MAX_BOOT_ENTRIES) before writing to valid_entries[].
> 
> Fixes: 7385e947fc65 ("pc-bios/s390-ccw: fix non-sequential boot entries (eckd)")
> Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>  pc-bios/s390-ccw/menu.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
> index eeaff78f87..b6a9a56d46 100644
> --- a/pc-bios/s390-ccw/menu.c
> +++ b/pc-bios/s390-ccw/menu.c
> @@ -176,18 +176,24 @@ int menu_get_boot_index(bool *valid_entries)
>      return boot_index;
>  }
>  
> -/* Returns the entry number that was printed */
> +/* 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];
> +    const char *p;
>  
>      ebcdic_to_ascii(data, buf, len);
>      buf[len] = '\n';
>      buf[len + 1] = '\0';
>  
> +    p = (buf[0] == ' ') ? buf + 1 : buf;
> +    if (!isdigit((unsigned char)*p)) {
> +        return -1;
> +    }
> +
>      printf("%s", buf);
>  
> -    return buf[0] == ' ' ? atoi(buf + 1) : atoi(buf);
> +    return atoi(p);
>  }
>  
>  int menu_get_zipl_boot_index(const char *menu_data)
> @@ -216,6 +222,9 @@ int menu_get_zipl_boot_index(const char *menu_data)
>          entry = zipl_print_entry(menu_data, len);
>          menu_data += len + 1;
>  
> +        if (entry < 0 || entry >= MAX_BOOT_ENTRIES) {
> +            continue;
> +        }
>          valid_entries[entry] = true;
>  
>          if (entry == 0) {