| Message ID | d5e3c8e66bad3725d7d48d0e45c4b7eb7c02631d.1620455671.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/legacy_serial: Fix UBSAN: array-index-out-of-bounds | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (7619d98e5041d5c25aba5428704dba6121237a9a) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 31 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch fixes a commit that hasn't been released yet |
On Sat, May 08, 2021 at 06:36:21AM +0000, Christophe Leroy wrote: > UBSAN complains when a pointer is calculated with invalid > 'legacy_serial_console' index, allthough the index is verified > before dereferencing the pointer. Addressing like this is UB already. You could just move this: > - if (legacy_serial_console < 0) > - return 0; to before > - struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console]; > - struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console]; and no other change is necessary. Segher
Segher Boessenkool <segher@kernel.crashing.org> writes: > On Sat, May 08, 2021 at 06:36:21AM +0000, Christophe Leroy wrote: >> UBSAN complains when a pointer is calculated with invalid >> 'legacy_serial_console' index, allthough the index is verified >> before dereferencing the pointer. > > Addressing like this is UB already. > > You could just move this: > >> - if (legacy_serial_console < 0) >> - return 0; > > to before > >> - struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console]; >> - struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console]; > > and no other change is necessary. Yeah I sent a v2 doing that, thanks. cheers
Le 11/05/2021 à 03:16, Michael Ellerman a écrit : > Segher Boessenkool <segher@kernel.crashing.org> writes: > >> On Sat, May 08, 2021 at 06:36:21AM +0000, Christophe Leroy wrote: >>> UBSAN complains when a pointer is calculated with invalid >>> 'legacy_serial_console' index, allthough the index is verified >>> before dereferencing the pointer. >> >> Addressing like this is UB already. >> >> You could just move this: >> >>> - if (legacy_serial_console < 0) >>> - return 0; >> >> to before >> >>> - struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console]; >>> - struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console]; >> >> and no other change is necessary. > > Yeah I sent a v2 doing that, thanks. > I wanted something looking similar to setup_legacy_serial_console(), but of course this also works. Christophe
On Sat, 8 May 2021 06:36:21 +0000 (UTC), Christophe Leroy wrote: > UBSAN complains when a pointer is calculated with invalid > 'legacy_serial_console' index, allthough the index is verified > before dereferencing the pointer. > > Fix it by checking 'legacy_serial_console' validity before > calculating pointers. Applied to powerpc/fixes. [1/1] powerpc/legacy_serial: Fix UBSAN: array-index-out-of-bounds https://git.kernel.org/powerpc/c/63970f3c37e75997ed86dbdfdc83df35f2152bb1 cheers
diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c index 8b2c1a8553a0..1c2e09e1d59b 100644 --- a/arch/powerpc/kernel/legacy_serial.c +++ b/arch/powerpc/kernel/legacy_serial.c @@ -354,15 +354,12 @@ static void __init setup_legacy_serial_console(int console) udbg_uart_setup(info->speed, info->clock); } -static int __init ioremap_legacy_serial_console(void) +static int __init do_ioremap_legacy_serial_console(int console) { - struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console]; - struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console]; + struct legacy_serial_info *info = &legacy_serial_infos[console]; + struct plat_serial8250_port *port = &legacy_serial_ports[console]; void __iomem *vaddr; - if (legacy_serial_console < 0) - return 0; - if (!info->early_addr) return 0; @@ -376,6 +373,13 @@ static int __init ioremap_legacy_serial_console(void) return 0; } + +static int __init ioremap_legacy_serial_console(void) +{ + if (legacy_serial_console < 0) + return 0; + return do_ioremap_legacy_serial_console(legacy_serial_console); +} early_initcall(ioremap_legacy_serial_console); /*
UBSAN complains when a pointer is calculated with invalid 'legacy_serial_console' index, allthough the index is verified before dereferencing the pointer. Fix it by checking 'legacy_serial_console' validity before calculating pointers. Fixes: 0bd3f9e953bd ("powerpc/legacy_serial: Use early_ioremap()") Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/kernel/legacy_serial.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)