2021-05-08 06:37:05

by Christophe Leroy

Subject: [PATCH] powerpc/legacy_serial: Fix UBSAN: array-index-out-of-bounds

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 <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/legacy_serial.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

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);

/*
--
2.25.0

2021-05-10 21:37:20

by Segher Boessenkool

Subject: Re: [PATCH] powerpc/legacy_serial: Fix UBSAN: array-index-out-of-bounds

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

2021-05-11 01:16:56

by Michael Ellerman

Subject: Re: [PATCH] powerpc/legacy_serial: Fix UBSAN: array-index-out-of-bounds

Segher Boessenkool <[email protected]> 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

2021-05-11 04:55:34

by Christophe Leroy

Subject: Re: [PATCH] powerpc/legacy_serial: Fix UBSAN: array-index-out-of-bounds

Le 11/05/2021 à 03:16, Michael Ellerman a écrit :
> Segher Boessenkool <[email protected]> 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

2021-05-16 09:39:20

by Michael Ellerman

Subject: Re: [PATCH] powerpc/legacy_serial: Fix UBSAN: array-index-out-of-bounds

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