ppc/spapr: Fix ubsan warning with unaligned pointer access

[PATCH] ppc/spapr: Fix ubsan warning with unaligned pointer access

Posted by Daniel Hoffman 1 year, 11 months ago

Found while running QTest with UBsan. Unaligned pointers appear to be
valid, so moving the read to an explicit memcpy to an intermediate.
---
 hw/ppc/vof.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index e3b430a81f4..609a51c645d 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -646,7 +646,10 @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base)
     mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen);
     g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
     if (sc == 2) {
-        mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) * ac));
+        /* Pointer may be unaligned */
+        uint64_t mem0_end_copy;
+        memcpy(&mem0_end_copy, mem0_reg + sizeof(uint32_t) * ac, sizeof(mem0_end_copy));
+        mem0_end = be64_to_cpu(mem0_end_copy);
     } else {
         mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) * ac));
     }
-- 
2.40.1

Re: [PATCH] ppc/spapr: Fix ubsan warning with unaligned pointer access

Posted by Richard Henderson 1 year, 11 months ago

On 12/16/23 16:14, Daniel Hoffman wrote:
> Found while running QTest with UBsan. Unaligned pointers appear to be
> valid, so moving the read to an explicit memcpy to an intermediate.
> ---
>   hw/ppc/vof.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index e3b430a81f4..609a51c645d 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -646,7 +646,10 @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base)
>       mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen);
>       g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
>       if (sc == 2) {
> -        mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) * ac));
> +        /* Pointer may be unaligned */
> +        uint64_t mem0_end_copy;
> +        memcpy(&mem0_end_copy, mem0_reg + sizeof(uint32_t) * ac, sizeof(mem0_end_copy));
> +        mem0_end = be64_to_cpu(mem0_end_copy);

mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);


r~

Re: [PATCH] ppc/spapr: Fix ubsan warning with unaligned pointer access

Posted by Alexey Kardashevskiy 1 year, 11 months ago


On 20/12/2023 12:45, Richard Henderson wrote:
> On 12/16/23 16:14, Daniel Hoffman wrote:
>> Found while running QTest with UBsan. Unaligned pointers appear to be
>> valid, so moving the read to an explicit memcpy to an intermediate.
>> ---
>>   hw/ppc/vof.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
>> index e3b430a81f4..609a51c645d 100644
>> --- a/hw/ppc/vof.c
>> +++ b/hw/ppc/vof.c
>> @@ -646,7 +646,10 @@ static void vof_dt_memory_available(void *fdt, 
>> GArray *claimed, uint64_t base)
>>       mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen);
>>       g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
>>       if (sc == 2) {
>> -        mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + 
>> sizeof(uint32_t) * ac));
>> +        /* Pointer may be unaligned */
>> +        uint64_t mem0_end_copy;
>> +        memcpy(&mem0_end_copy, mem0_reg + sizeof(uint32_t) * ac, 
>> sizeof(mem0_end_copy));
>> +        mem0_end = be64_to_cpu(mem0_end_copy);
> 
> mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);


+1 for ldq_be_p(). Or read lo&hi 32bit chunks and combine. Thanks,

> 
> 
> r~

-- 
Alexey