hw/vfio/pci.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
When vfio_device_get_region_info() fails in vfio_pci_load_rom(),
the function returns without setting vdev->rom_read_failed to true,
and without allocating vdev->rom. This leaves vdev->rom as NULL.
Signed-off-by: Mario Casquero <mcasquer@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9c06b25e63..9df04fa2d7 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1027,7 +1027,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
}
}
-static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
+static bool vfio_pci_load_rom(VFIOPCIDevice *vdev)
{
VFIODevice *vbasedev = &vdev->vbasedev;
struct vfio_region_info *reg_info = NULL;
@@ -1041,7 +1041,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
if (ret != 0) {
error_report("vfio: Error getting ROM info: %s", strerror(-ret));
- return;
+ return false;
}
trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
@@ -1052,12 +1052,11 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
vdev->rom_offset = reg_info->offset;
if (!vdev->rom_size) {
- vdev->rom_read_failed = true;
error_report("vfio-pci: Cannot read device rom at %s", vbasedev->name);
error_printf("Device option ROM contents are probably invalid "
"(check dmesg).\nSkip option ROM probe with rombar=0, "
"or load from file with romfile=\n");
- return;
+ return false;
}
vdev->rom = g_malloc(size);
@@ -1113,6 +1112,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
data[6] = -csum;
}
}
+
+ return true;
}
/* "Raw" read of underlying config space. */
@@ -1146,7 +1147,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
/* Load the ROM lazily when the guest tries to read it */
if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
- vfio_pci_load_rom(vdev);
+ vdev->rom_read_failed = !vfio_pci_load_rom(vdev);
}
memcpy(&val, vdev->rom + addr,
--
2.54.0
Hi Mario,
On 2/6/26 09:08, Mario Casquero wrote:
> When vfio_device_get_region_info() fails in vfio_pci_load_rom(),
> the function returns without setting vdev->rom_read_failed to true,
> and without allocating vdev->rom. This leaves vdev->rom as NULL.
>
> Signed-off-by: Mario Casquero <mcasquer@redhat.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> ---
> hw/vfio/pci.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 9c06b25e63..9df04fa2d7 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1027,7 +1027,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
> }
> }
>
> -static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> +static bool vfio_pci_load_rom(VFIOPCIDevice *vdev)
Since you are modifying this method signature, a bit nicer would be to
have it take an Error* argument, and propagate it. Anyway my 2 cents.
Reviewed-by: Philippe Mathieu-Daudé <philmd@mailo.com>
> {
> VFIODevice *vbasedev = &vdev->vbasedev;
> struct vfio_region_info *reg_info = NULL;
> @@ -1041,7 +1041,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>
> if (ret != 0) {
> error_report("vfio: Error getting ROM info: %s", strerror(-ret));
> - return;
> + return false;
> }
>
> trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
> @@ -1052,12 +1052,11 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> vdev->rom_offset = reg_info->offset;
>
> if (!vdev->rom_size) {
> - vdev->rom_read_failed = true;
> error_report("vfio-pci: Cannot read device rom at %s", vbasedev->name);
> error_printf("Device option ROM contents are probably invalid "
> "(check dmesg).\nSkip option ROM probe with rombar=0, "
> "or load from file with romfile=\n");
> - return;
> + return false;
> }
>
> vdev->rom = g_malloc(size);
> @@ -1113,6 +1112,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> data[6] = -csum;
> }
> }
> +
> + return true;
> }
>
> /* "Raw" read of underlying config space. */
> @@ -1146,7 +1147,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
>
> /* Load the ROM lazily when the guest tries to read it */
> if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
> - vfio_pci_load_rom(vdev);
> + vdev->rom_read_failed = !vfio_pci_load_rom(vdev);
> }
>
> memcpy(&val, vdev->rom + addr,
On 6/2/26 09:08, Mario Casquero wrote:
> When vfio_device_get_region_info() fails in vfio_pci_load_rom(),
> the function returns without setting vdev->rom_read_failed to true,
> and without allocating vdev->rom. This leaves vdev->rom as NULL.
>
> Signed-off-by: Mario Casquero <mcasquer@redhat.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> ---
> hw/vfio/pci.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
Applied to
https://github.com/legoater/qemu vfio-next
Thanks,
C.
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 9c06b25e63..9df04fa2d7 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1027,7 +1027,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
> }
> }
>
> -static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> +static bool vfio_pci_load_rom(VFIOPCIDevice *vdev)
> {
> VFIODevice *vbasedev = &vdev->vbasedev;
> struct vfio_region_info *reg_info = NULL;
> @@ -1041,7 +1041,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>
> if (ret != 0) {
> error_report("vfio: Error getting ROM info: %s", strerror(-ret));
> - return;
> + return false;
> }
>
> trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
> @@ -1052,12 +1052,11 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> vdev->rom_offset = reg_info->offset;
>
> if (!vdev->rom_size) {
> - vdev->rom_read_failed = true;
> error_report("vfio-pci: Cannot read device rom at %s", vbasedev->name);
> error_printf("Device option ROM contents are probably invalid "
> "(check dmesg).\nSkip option ROM probe with rombar=0, "
> "or load from file with romfile=\n");
> - return;
> + return false;
> }
>
> vdev->rom = g_malloc(size);
> @@ -1113,6 +1112,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> data[6] = -csum;
> }
> }
> +
> + return true;
> }
>
> /* "Raw" read of underlying config space. */
> @@ -1146,7 +1147,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
>
> /* Load the ROM lazily when the guest tries to read it */
> if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
> - vfio_pci_load_rom(vdev);
> + vdev->rom_read_failed = !vfio_pci_load_rom(vdev);
> }
>
> memcpy(&val, vdev->rom + addr,
© 2016 - 2026 Red Hat, Inc.