zipl/boot: Fix undefined behaviour logic in menu

The code within 'menu_param()' previously assumed that a read from sclp
will never fail.

If 'sclp_param()' fails then 'endptr' is never initialised and
'loadparm' is compared with 'endptr' which is undefined behvaiour.

If a sclp read fails, an undefined 'endptr' is never accessed, and upon
a failed read, will return a new error code 'SCLP_ERROR' instead of
returning 'NUMBER_FOUND' which is incorrect logic wise.

Remove compare conditions and assignments of 0 in 'value', as 'value'
is initialised with 0 ('DEFAULT_MENU_ENTRY') and cannot be non zero,
only in the case where a number is found and we go to boot.

Logic:

Check if we got a number and boot from it.

If 'PRINT_PROMPT', break out to menu print logic.

If an 'SCLP_ERROR' occurs, print an error message and boot the default
since 'value' is initialised with 'DEFAULT_MENU_ENTRY'.

If 'NOTHING_FOUND', check if the menu is disabled. If disabled, go to
default boot. Otherwise break out to print logic.

Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Richie Buturla <richie@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>

zipl/boot/menu.c

@@ -102,6 +102,7 @@ enum param_result {
 	NUMBER_FOUND  = 0,
 	PRINT_PROMPT  = 1,
 	NOTHING_FOUND = 2,
+	SCLP_ERROR    = 3,
 };
 
 /*
@@ -114,6 +115,7 @@ enum param_result {
  *     0 - found number to boot, stored in value
  *     1 - print prompt
  *     2 - nothing found
+ *     3 - sclp error
  */
 static enum param_result menu_param(unsigned long *value)
 {
@@ -121,8 +123,12 @@ static enum param_result menu_param(unsigned long *value)
 	char *endptr;
 	int i;
 
-	if (!sclp_param(loadparm))
+	/* try to fetch loadparms from sclp into 'loadparm' */
-		*value = ebcdic_strtoul(loadparm, &endptr, 10);
+	if (sclp_param(loadparm) != 0)
+		return SCLP_ERROR;
+
+	/* parse number from loadparm */
+	*value = ebcdic_strtoul(loadparm, &endptr, 10);
 
 	/* got number, done */
 	if (endptr != loadparm)
@@ -134,39 +140,44 @@ static enum param_result menu_param(unsigned long *value)
 	while ((i < PARAM_SIZE) && ecbdic_isspace(loadparm[i]))
 		i++;
 
-	if (!strncmp(&loadparm[i], "PROMPT", 6)) {
+	if (!strncmp(&loadparm[i], "PROMPT", 6))
-		*value = 0;
 		return PRINT_PROMPT;
-	}
 
 	return NOTHING_FOUND;
 }
 
 int menu(void)
 {
-	unsigned long value = 0;
+	enum { DEFAULT_MENU_ENTRY = 0 };
+	unsigned long value = DEFAULT_MENU_ENTRY;
 	char *cmd_line_extra;
 	char endstring[15];
-	int rc;
 
 	cmd_line_extra = (char *)COMMAND_LINE_EXTRA;
 	memset(cmd_line_extra, 0, COMMAND_LINE_EXTRA_SIZE);
 
-	rc = sclp_setup(SCLP_INIT);
+	if (sclp_setup(SCLP_INIT) != 0) {
-	if (rc)
 		/* sclp setup failed boot default */
 		goto boot;
+	}
 
-	rc = menu_param(&value);
+	switch (menu_param(&value)) {
-	if (rc == NUMBER_FOUND) {
+	case NUMBER_FOUND:
 		/* got number from loadparm, boot it */
 		goto boot;
-	} else if (rc == PRINT_PROMPT && value == 0) {
+	case PRINT_PROMPT:
-		/* keyword "prompt", show menu */
+		/* print menu */
-	} else if (__stage2_params.flag == 0) {
+		break;
-		/* menu disabled, boot default */
+	case SCLP_ERROR:
-		value = 0;
+		/* failed to read from sclp, boot default */
+		printf("SCLP_ERROR\n");
 		goto boot;
+	case NOTHING_FOUND:
+		if (__stage2_params.flag == 0) {
+			/* menu disabled, boot default */
+			goto boot;
+		}
+		break;
 	}
 
 	/* print banner */