From: Thomas Huth <thuth@redhat.com>

When compiling QEMU with --enable-ubsan there is a undefined behavior
warning when running the bios-tables-test for example:

 .../system/physmem.c:3243:13: runtime error: applying non-zero offset 262144 to null pointer
    #0 0x55ac1df5fbc4 in address_space_write_rom_internal .../system/physmem.c:3243:13

The problem is that buf is indeed NULL if the function is e.g. called
with type == FLUSH_CACHE. Add a check to fix the issue.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20250728172545.314178-1-thuth@redhat.com>
---
 system/physmem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/system/physmem.c b/system/physmem.c
index f498572fc82..311011156c7 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3231,8 +3231,10 @@ static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
             }
         }
         len -= l;
-        buf += l;
         addr += l;
+        if (buf) {
+            buf += l;
+        }
     }
     return MEMTX_OK;
 }
-- 
2.51.0

On 9/9/25 06:51, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> When compiling QEMU with --enable-ubsan there is a undefined behavior
> warning when running the bios-tables-test for example:
> 
>   .../system/physmem.c:3243:13: runtime error: applying non-zero offset 262144 to null pointer
>      #0 0x55ac1df5fbc4 in address_space_write_rom_internal .../system/physmem.c:3243:13
> 
> The problem is that buf is indeed NULL if the function is e.g. called
> with type == FLUSH_CACHE. Add a check to fix the issue.
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> Message-ID: <20250728172545.314178-1-thuth@redhat.com>
> ---
>   system/physmem.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/system/physmem.c b/system/physmem.c
> index f498572fc82..311011156c7 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -3231,8 +3231,10 @@ static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
>               }
>           }
>           len -= l;
> -        buf += l;
>           addr += l;
> +        if (buf) {
> +            buf += l;
> +        }
>       }
>       return MEMTX_OK;
>   }

This has caused a new warning from Coverity:

Line 3224:
CID 1621220: (#1 of 1): Dereference after null check (FORWARD_NULL)
10. var_deref_model: Passing null pointer buf to memcpy, which dereferences it. [Note: The 
source code implementation of the function has been overridden by a builtin model.]

Line 3235:
5. var_compare_op: Comparing buf to null implies that buf might be null.

I think the best solution would be to split this function, so that the flush cache 
operation doesn't have the ptr argument at all.

An easy short-term solution could be to move the increment of buf into the WRITE_DATA case 
of the switch.


r~