| Message ID | 20231217001441.146344-1-dhoff749@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ppc/spapr: Fix ubsan warning with unaligned pointer access | expand |
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~
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~
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)); }