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

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

[Nicholas Piggin]

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>
---
 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);
         }
-- 
2.45.2

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);
>           }