| Message ID | f0b1f299d793c4302ee1b4c6a9c99057f924d4f4.1740168326.git.sanastasio@raptorengineering.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable UBSAN on ppc | expand |
On 21/02/2025 8:08 pm, Shawn Anastasio wrote: > Fix two misaligned reads from the FDT in the opal setup code to avoid > tripping UBSAN failures. Without this change, UBSAN-enabled builds on > PPC will fail on boot before the serial console is even initialized. > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> > --- > xen/arch/ppc/opal.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/ppc/opal.c b/xen/arch/ppc/opal.c > index 1183b7d5ef..3d0e4daf27 100644 > --- a/xen/arch/ppc/opal.c > +++ b/xen/arch/ppc/opal.c > @@ -34,8 +34,9 @@ static void opal_putchar(char c) > void __init boot_opal_init(const void *fdt) > { > int opal_node; > - const __be64 *opal_base; > - const __be64 *opal_entry; > + const __be64 *opal_base_p; > + const __be64 *opal_entry_p; > + __be64 opal_base, opal_entry; > > if ( fdt_check_header(fdt) < 0 ) > { > @@ -54,17 +55,20 @@ void __init boot_opal_init(const void *fdt) > die(); > } > > - opal_base = fdt_getprop(fdt, opal_node, "opal-base-address", NULL); > - opal_entry = fdt_getprop(fdt, opal_node, "opal-entry-address", NULL); > - if ( !opal_base || !opal_entry ) > + opal_base_p = fdt_getprop(fdt, opal_node, "opal-base-address", NULL); > + opal_entry_p = fdt_getprop(fdt, opal_node, "opal-entry-address", NULL); > + if ( !opal_base_p || !opal_entry_p ) > { > early_printk("Failed to get opal-base-address/opal-entry-address " > "property from DT!\n"); > die(); > } > > - opal.base = be64_to_cpu(*opal_base); > - opal.entry = be64_to_cpu(*opal_entry); > + memcpy(&opal_base, opal_base_p, sizeof(opal_base)); > + memcpy(&opal_entry, opal_entry_p, sizeof(opal_entry)); > + > + opal.base = be64_to_cpu(opal_base); > + opal.entry = be64_to_cpu(opal_entry); Thanks for getting to the bottom of this. However, you can use get_unaligned_*() and friends which is probably a bit nicer, and doesn't involve the extra local variables. ~Andrew
On 21/02/2025 8:10 pm, Andrew Cooper wrote: > On 21/02/2025 8:08 pm, Shawn Anastasio wrote: >> Fix two misaligned reads from the FDT in the opal setup code to avoid >> tripping UBSAN failures. Without this change, UBSAN-enabled builds on >> PPC will fail on boot before the serial console is even initialized. >> >> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> >> --- >> xen/arch/ppc/opal.c | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/ppc/opal.c b/xen/arch/ppc/opal.c >> index 1183b7d5ef..3d0e4daf27 100644 >> --- a/xen/arch/ppc/opal.c >> +++ b/xen/arch/ppc/opal.c >> @@ -34,8 +34,9 @@ static void opal_putchar(char c) >> void __init boot_opal_init(const void *fdt) >> { >> int opal_node; >> - const __be64 *opal_base; >> - const __be64 *opal_entry; >> + const __be64 *opal_base_p; >> + const __be64 *opal_entry_p; >> + __be64 opal_base, opal_entry; >> >> if ( fdt_check_header(fdt) < 0 ) >> { >> @@ -54,17 +55,20 @@ void __init boot_opal_init(const void *fdt) >> die(); >> } >> >> - opal_base = fdt_getprop(fdt, opal_node, "opal-base-address", NULL); >> - opal_entry = fdt_getprop(fdt, opal_node, "opal-entry-address", NULL); >> - if ( !opal_base || !opal_entry ) >> + opal_base_p = fdt_getprop(fdt, opal_node, "opal-base-address", NULL); >> + opal_entry_p = fdt_getprop(fdt, opal_node, "opal-entry-address", NULL); >> + if ( !opal_base_p || !opal_entry_p ) >> { >> early_printk("Failed to get opal-base-address/opal-entry-address " >> "property from DT!\n"); >> die(); >> } >> >> - opal.base = be64_to_cpu(*opal_base); >> - opal.entry = be64_to_cpu(*opal_entry); >> + memcpy(&opal_base, opal_base_p, sizeof(opal_base)); >> + memcpy(&opal_entry, opal_entry_p, sizeof(opal_entry)); >> + >> + opal.base = be64_to_cpu(opal_base); >> + opal.entry = be64_to_cpu(opal_entry); > Thanks for getting to the bottom of this. > > However, you can use get_unaligned_*() and friends which is probably a > bit nicer, and doesn't involve the extra local variables. Sorry, the other thing to say is that if PPC is generally fine with unaligned accesses, then you might want to follow what x86 does. We use -fno-sanitize=alignment generally, because there's a whole pile of things which are misaigned and spec'd that way for backwards compatibility. ~Andrew
diff --git a/xen/arch/ppc/opal.c b/xen/arch/ppc/opal.c index 1183b7d5ef..3d0e4daf27 100644 --- a/xen/arch/ppc/opal.c +++ b/xen/arch/ppc/opal.c @@ -34,8 +34,9 @@ static void opal_putchar(char c) void __init boot_opal_init(const void *fdt) { int opal_node; - const __be64 *opal_base; - const __be64 *opal_entry; + const __be64 *opal_base_p; + const __be64 *opal_entry_p; + __be64 opal_base, opal_entry; if ( fdt_check_header(fdt) < 0 ) { @@ -54,17 +55,20 @@ void __init boot_opal_init(const void *fdt) die(); } - opal_base = fdt_getprop(fdt, opal_node, "opal-base-address", NULL); - opal_entry = fdt_getprop(fdt, opal_node, "opal-entry-address", NULL); - if ( !opal_base || !opal_entry ) + opal_base_p = fdt_getprop(fdt, opal_node, "opal-base-address", NULL); + opal_entry_p = fdt_getprop(fdt, opal_node, "opal-entry-address", NULL); + if ( !opal_base_p || !opal_entry_p ) { early_printk("Failed to get opal-base-address/opal-entry-address " "property from DT!\n"); die(); } - opal.base = be64_to_cpu(*opal_base); - opal.entry = be64_to_cpu(*opal_entry); + memcpy(&opal_base, opal_base_p, sizeof(opal_base)); + memcpy(&opal_entry, opal_entry_p, sizeof(opal_entry)); + + opal.base = be64_to_cpu(opal_base); + opal.entry = be64_to_cpu(opal_entry); early_printk_init(opal_putchar);
Fix two misaligned reads from the FDT in the opal setup code to avoid tripping UBSAN failures. Without this change, UBSAN-enabled builds on PPC will fail on boot before the serial console is even initialized. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> --- xen/arch/ppc/opal.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)