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

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

[Christophe Leroy]

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

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

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

[Segher Boessenkool]

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

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

[Michael Ellerman]

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

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

[Christophe Leroy]

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

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

[Michael Ellerman]

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