Re: [PATCH] hw/arm/aspeed: avoid sign mismatch on sscanf for uart property

On 8/2/26 18:28, Carlo Marcelo Arenas Belón wrote:
> using "%u" with sscanf() was likely meant to indicate that a
> negative value was unexpected, but with a signed variable it
> could result in undefined behaviour.
> 
> use "%d" and check for a negative input explicitly.
> 
> Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]>
> ---
>   hw/arm/aspeed.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index a48c442058..a9238e6217 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -327,7 +327,7 @@ static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp)
>       int uart_first = aspeed_uart_first(sc->uarts_base);
>       int uart_last = aspeed_uart_last(sc->uarts_base, sc->uarts_num);
>   
> -    if (sscanf(value, "uart%u", &val) != 1) {
> +    if (sscanf(value, "uart%d", &val) != 1 || val < 0) {
>           error_setg(errp, "Bad value for \"uart\" property");
>           return;
>       }

The following test :

     if (val < uart_first || val > uart_last) {

would have caught the bad value but it's better to return early.

Reviewed-by: Cédric Le Goater <[email protected]>

Thanks,

C.