Re: [PATCH] hw/pci-host/pnv_phb4: fix guest-triggerable abort on 8-byte config access

Hi,

On 5/8/26 11:05, Nikhil Kumar Singh wrote:
> A guest-triggerable assertion crash (DoS) exists in pnv_phb4_config_write()
> and pnv_phb4_config_read(). When a guest performs an 8-byte access to
> PHB_CONFIG_DATA (offset 0x130), QEMU aborts because the switch(size)
> statement in the config accessors only handles 1, 2, and 4-byte accesses,
> hitting g_assert_not_reached() in the default case.
> 
> Since guest input is untrusted, an invalid access size should not crash
> the host. Fix this by adding a size > 4 guard in pnv_phb4_reg_write() and
> pnv_phb4_reg_read() before the config accessor calls, and by replacing
> the g_assert_not_reached() in pnv_phb4_config_write() and
> pnv_phb4_config_read() with phb_error() to cover any other callers.
> Invalid reads now return ~0ull, maintaining PCI conventions.
> 
> Fixes: 4f9924c4d4cf ("ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3591
> Signed-off-by: Nikhil Kumar Singh <[email protected]>
> ---
>   hw/pci-host/pnv_phb4.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)


> @@ -507,6 +511,11 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>   
>       /* Special case outbound configuration data */
>       if ((off & 0xfffc) == PHB_CONFIG_DATA) {
> +        if (size > 4) {
> +            phb_error(phb, "invalid config write size %u at 0x%"PRIx64"\n",
> +                      size, off);
> +            return;
> +        }
>           pnv_phb4_config_write(phb, off & 0x3, size, val);

Alternatively MIN(size, 4).

But without looking much at this model, I'd expect these regions to
be mapped as the usual "pci-conf-idx" / "pci-data-idx" ones (FYI see
pci_host_conf_le_ops and pci_host_data_le_ops).

Anyway my 2 cents, since this involves more of "silence that DoS".

>           return;
>       }
> @@ -644,6 +653,11 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
>       uint64_t val;
>   
>       if ((off & 0xfffc) == PHB_CONFIG_DATA) {
> +        if (size > 4) {
> +            phb_error(phb, "invalid config read size %u at 0x%"PRIx64"\n",
> +                      size, off);
> +            return ~0ull;
> +        }
>           return pnv_phb4_config_read(phb, off & 0x3, size);
>       }
>