Re: [PATCH] ppc/pnv: ADU fix possible buffer overrun with invalid size

[Cédric Le Goater]

On 8/6/24 17:13, Nicholas Piggin wrote:
> The ADU LPC transfer-size field is 7 bits, but the supported sizes for
> LPC access via ADU appear to be 1, 2, 4, 8. The data buffer could
> overrun if firmware set an invalid size field, so add checks to reject
> them with a message.
>
> Reported-by: Cédric Le Goater <clg@redhat.com>
> Resolves: Coverity CID 1558830
> Fixes: 24bd283bccb33 ("ppc/pnv: Implement ADU access to LPC space")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/ppc/pnv_adu.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/hw/ppc/pnv_adu.c b/hw/ppc/pnv_adu.c
> index 81b7d6e526..f636dedf79 100644
> --- a/hw/ppc/pnv_adu.c
> +++ b/hw/ppc/pnv_adu.c
> @@ -116,6 +116,12 @@ static void pnv_adu_xscom_write(void *opaque, hwaddr addr, 
> uint64_t val,
>               uint32_t lpc_size = lpc_cmd_size(adu);
>               uint64_t data = 0;
>   
> +            if (!is_power_of_2(lpc_size) || lpc_size > sizeof(data)) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "ADU: Unsupported LPC access "
> +                                               "size:%" PRId32 "\n", lpc_size);
> +                break;
> +            }
> +
>               pnv_lpc_opb_read(adu->lpc, lpc_addr, (void *)&data, lpc_size);
>   
>               /*
> @@ -135,6 +141,12 @@ static void pnv_adu_xscom_write(void *opaque, hwaddr addr, 
> uint64_t val,
>               uint32_t lpc_size = lpc_cmd_size(adu);
>               uint64_t data;
>   
> +            if (!is_power_of_2(lpc_size) || lpc_size > sizeof(data)) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "ADU: Unsupported LPC access "
> +                                               "size:%" PRId32 "\n", lpc_size);
> +                break;
> +            }
> +
>               data = cpu_to_be64(val) >> ((lpc_addr & 7) * 8); /* See above */
>               pnv_lpc_opb_write(adu->lpc, lpc_addr, (void *)&data, lpc_size);
>           }