Hi Heinrich, On Sat, 29 Oct 2022 at 20:47, Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > > > On 10/30/22 02:43, Simon Glass wrote: > > Hi Heinrich, > > > > On Sat, 22 Oct 2022 at 03:49, Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > >> > >> We use the parameter file in console function to choose from an array after > >> checking against MAX_FILES but we never check if the value of file is > >> negative. > >> > >> Running ./u-boot -T -l and issuing the poweroff command has resulted in > >> crashes because > >> > >> os_exit() results in std::ostream::flush() calling U-Boot's fflush with > >> file being a pointer which when converted to int may be represented by a > >> negative number. > >> > >> This shows that checking against MAX_FILES is not enough we have to ensure > >> that the file argument is always positive. > >> > >> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > >> --- > >> common/console.c | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > > > > How about changing the 'file' parameter to a uint? It seems that this > > is what it is supposed to be, from your checks. > > As we support only 3 values the variable type should have been defined > as an enum. > > Changing the parameter type would have meant to look at all callers and > check if the value of file is stored in a variable and replace that > variable type too. > > I just went for the least invasive change.
Well I don't really mind. Reviewed-by: Simon Glass <s...@chromium.org>
