[PATCH] autoboot: fix illegal memory access when stop key and delay key are empty

[Andy.Wu at sony.com]

> Both indices cannot be negative. So it is understandable that u_int was chosen.
Yes, u_int is understandable that the length is never be negative, but define the length parameter as "int" also usable.
Some example in current u-boot source code:

$ grep -rn length common/ | grep "int "
common/usb_storage.c:363:static int us_one_transfer(struct us_data *us, int pipe, char *buf, int length)
common/lcd.c:52:int lcd_line_length;
common/lcd.c:66:        int line_length;
common/lcd.c:143:__weak int lcd_get_size(int *line_length)
common/lcd.c:283:       int line_length;
common/usb.c:275:                       void *data, int len, int *actual_length, int timeout)
common/usb.c:600:                            unsigned char *buffer, int length)
common/usb.c:751:static void usb_try_string_workarounds(unsigned char *buf, int *length)
common/usb.c:753:       int newlength, oldlength = *length;
common/kgdb.c:319:      int length;
common/cli_hush.c:318:  int length;
common/usb_hub.c:613:   int i, length;

> You could avoid the subtraction instead of changing the type:
> 
> -for (i = 0; i < presskey_max - 1; i++)
> +for (i = 0; i + 1 < presskey_max; i++)
This style seems not typically way for for loop, how do you think?

Best Regards
Andy Wu

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Heinrich
> Schuchardt
> Sent: Friday, January 15, 2021 8:19 PM
> To: Mo, Yuezhang <Yuezhang.Mo at sony.com>; u-boot at lists.denx.de
> Cc: sjg at chromium.org; hs at denx.de
> Subject: Re: [PATCH] autoboot: fix illegal memory access when stop key and
> delay key are empty
> 
> On 15.01.21 04:11, Yuezhang.Mo at sony.com wrote:
> > If both stop key and delay key are empty, the length of these keys is
> > 0. The subtraction operation will cause the u_int type variable to
> > overflow, will cause illegal memory access in key input loop.
> >
> > This commit fixes this bug by using int type instead of u_init.
> > ---
> >  common/autoboot.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/autoboot.c b/common/autoboot.c index
> > e628baffb8..61fb09f910 100644
> > --- a/common/autoboot.c
> > +++ b/common/autoboot.c
> > @@ -156,9 +156,9 @@ static int passwd_abort_key(uint64_t etime)
> >  	};
> >
> >  	char presskey[MAX_DELAY_STOP_STR];
> > -	u_int presskey_len = 0;
> > -	u_int presskey_max = 0;
> > -	u_int i;
> > +	int presskey_len = 0;
> > +	int presskey_max = 0;
> 
> Both indices cannot be negative. So it is understandable that u_int was chosen.
> You could avoid the subtraction instead of changing the type:
> 
> -for (i = 0; i < presskey_max - 1; i++)
> +for (i = 0; i + 1 < presskey_max; i++)
> 
> Acked-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> 
> > +	int i;
> >
> >  #  ifdef CONFIG_AUTOBOOT_DELAY_STR
> >  	if (delaykey[0].str == NULL)
> >