On 7/24/26 16:17, Alex Williamson wrote:
On Fri, 24 Jul 2026 11:24:13 +0200
Cédric Le Goater <[email protected]> wrote:
When vfio_pci_load_rom() fails, vdev->rom is NULL but the memcpy
still computes a source pointer from it, which is undefined behavior.
Guard the access and return a 0xff pattern instead, which is what
hardware returns for an absent ROM.
Signed-off-by: Cédric Le Goater <[email protected]>
---
hw/vfio/pci.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index
a3147d28665abd29fd804bd08bdffc3c7440033c..16d4f70ea58c9e5b174dd57950df55dc2ab91f6f
100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1176,8 +1176,13 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr,
unsigned size)
}
}
- memcpy(&val, vdev->rom + addr,
- (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
+ /* If ROM loading failed, return 0xff pattern */
+ if (vdev->rom_read_failed) {
+ memset(&val, 0xff, sizeof(val));
+ } else {
+ memcpy(&val, vdev->rom + addr,
+ (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
+ }
switch (size) {
case 1:
I thought we guaranteed that when (vdev->rom == NULL) that
(vdev->rom_size == 0), thus we end up with a harmless zero-sized memcpy
here. Also val is pre-initialized to ~0 so the memset() is redundant.
Does this actually change any behavior or is it only meant to sanitize
the undefined (NULL + addr) pointer that's never dereferenced? If the
latter, I'd drop the memset and note it explicitly as a sanitization,
not a fix. Thanks,
Right. the memcpy is a no-op because the size is 0 and val stays ~0.
I had that patch in my tree for a while, but Mario's recent fixes made
it useless. Let's drop it.
Thanks,
C.
