All of lore.kernel.org
 help / color / mirror / Atom feed
* WARNING: CPU: 0 PID: 1 at arch/powerpc/lib/feature-fixups.c:109 do_feature_fixups+0xb0/0xf0
@ 2021-05-06 19:31 Paul Menzel
  2021-05-06 19:32 ` UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56 Paul Menzel
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Menzel @ 2021-05-06 19:31 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev

Dear Linux folks,


On the POWER8 system IBM S822LC, Linux 5.13+, built with USSAN, logs the 
warning below.

```
[    0.030091] 
================================================================================
[    0.030295] UBSAN: array-index-out-of-bounds in 
arch/powerpc/kernel/legacy_serial.c:359:56
[    0.030325] index -1 is out of range for type 'legacy_serial_info [8]'
[    0.030350] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
[    0.030360] Call Trace:
[    0.030363] [c000000024f1bad0] [c0000000009f4330] 
dump_stack+0xc4/0x114 (unreliable)
[    0.030386] [c000000024f1bb20] [c0000000009efed0] 
ubsan_epilogue+0x18/0x78
[    0.030400] [c000000024f1bb80] [c0000000009efafc] 
__ubsan_handle_out_of_bounds+0xac/0xd0
[    0.030414] [c000000024f1bc20] [c000000001711588] 
ioremap_legacy_serial_console+0x54/0x144
[    0.030430] [c000000024f1bc70] [c0000000000123c0] 
do_one_initcall+0x60/0x2c0
[    0.030444] [c000000024f1bd40] [c000000001704bc4] 
kernel_init_freeable+0x19c/0x25c
[    0.030458] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
[    0.030471] [c000000024f1be10] [c00000000000d6ec] 
ret_from_kernel_thread+0x5c/0x70
[    0.030484] 
================================================================================
[    0.030641] 
================================================================================
[    0.030668] UBSAN: array-index-out-of-bounds in 
arch/powerpc/kernel/legacy_serial.c:360:58
[    0.030697] index -1 is out of range for type 'plat_serial8250_port [9]'
[    0.030721] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
[    0.030730] Call Trace:
[    0.030733] [c000000024f1bad0] [c0000000009f4330] 
dump_stack+0xc4/0x114 (unreliable)
[    0.030749] [c000000024f1bb20] [c0000000009efed0] 
ubsan_epilogue+0x18/0x78
[    0.030762] [c000000024f1bb80] [c0000000009efafc] 
__ubsan_handle_out_of_bounds+0xac/0xd0
[    0.030775] [c000000024f1bc20] [c0000000017115a0] 
ioremap_legacy_serial_console+0x6c/0x144
[    0.030790] [c000000024f1bc70] [c0000000000123c0] 
do_one_initcall+0x60/0x2c0
[    0.030802] [c000000024f1bd40] [c000000001704bc4] 
kernel_init_freeable+0x19c/0x25c
[    0.030816] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
[    0.030829] [c000000024f1be10] [c00000000000d6ec] 
ret_from_kernel_thread+0x5c/0x70
[    0.030842] 
================================================================================
```


Kind regards,

Paul


^ permalink raw reply	[flat|nested] 7+ messages in thread

* UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
  2021-05-06 19:31 WARNING: CPU: 0 PID: 1 at arch/powerpc/lib/feature-fixups.c:109 do_feature_fixups+0xb0/0xf0 Paul Menzel
@ 2021-05-06 19:32 ` Paul Menzel
  2021-05-07  8:31   ` Christophe Leroy
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Menzel @ 2021-05-06 19:32 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev

[corrected subject]

Am 06.05.21 um 21:31 schrieb Paul Menzel:
> Dear Linux folks,
> 
> 
> On the POWER8 system IBM S822LC, Linux 5.13+, built with USSAN, logs the 
> warning below.
> 
> ```
> [    0.030091] 
> ================================================================================ 
> 
> [    0.030295] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
> [    0.030325] index -1 is out of range for type 'legacy_serial_info [8]'
> [    0.030350] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
> [    0.030360] Call Trace:
> [    0.030363] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
> [    0.030386] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
> [    0.030400] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
> [    0.030414] [c000000024f1bc20] [c000000001711588] ioremap_legacy_serial_console+0x54/0x144
> [    0.030430] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
> [    0.030444] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
> [    0.030458] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
> [    0.030471] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
> [    0.030484] 
> ================================================================================ 
> 
> [    0.030641] 
> ================================================================================ 
> 
> [    0.030668] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:360:58
> [    0.030697] index -1 is out of range for type 'plat_serial8250_port [9]'
> [    0.030721] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
> [    0.030730] Call Trace:
> [    0.030733] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
> [    0.030749] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
> [    0.030762] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
> [    0.030775] [c000000024f1bc20] [c0000000017115a0] ioremap_legacy_serial_console+0x6c/0x144
> [    0.030790] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
> [    0.030802] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
> [    0.030816] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
> [    0.030829] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
> [    0.030842] 
> ================================================================================ 
> ```
> 
> 
> Kind regards,
> 
> Paul

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
  2021-05-06 19:32 ` UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56 Paul Menzel
@ 2021-05-07  8:31   ` Christophe Leroy
  2021-05-07  8:42     ` Paul Menzel
  2021-05-07 20:59     ` Segher Boessenkool
  0 siblings, 2 replies; 7+ messages in thread
From: Christophe Leroy @ 2021-05-07  8:31 UTC (permalink / raw)
  To: Paul Menzel, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linuxppc-dev

Hi Paul,

Le 06/05/2021 à 21:32, Paul Menzel a écrit :
> [corrected subject]
> 
> Am 06.05.21 um 21:31 schrieb Paul Menzel:
>> Dear Linux folks,
>>
>>
>> On the POWER8 system IBM S822LC, Linux 5.13+, built with USSAN, logs the warning below.
>>
>> ```
>> [    0.030091] ================================================================================
>> [    0.030295] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
>> [    0.030325] index -1 is out of range for type 'legacy_serial_info [8]'
>> [    0.030350] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>> [    0.030360] Call Trace:
>> [    0.030363] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>> [    0.030386] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>> [    0.030400] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>> [    0.030414] [c000000024f1bc20] [c000000001711588] ioremap_legacy_serial_console+0x54/0x144
>> [    0.030430] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>> [    0.030444] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>> [    0.030458] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>> [    0.030471] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>> [    0.030484] ================================================================================
>> [    0.030641] ================================================================================
>> [    0.030668] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:360:58
>> [    0.030697] index -1 is out of range for type 'plat_serial8250_port [9]'
>> [    0.030721] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>> [    0.030730] Call Trace:
>> [    0.030733] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>> [    0.030749] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>> [    0.030762] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>> [    0.030775] [c000000024f1bc20] [c0000000017115a0] ioremap_legacy_serial_console+0x6c/0x144
>> [    0.030790] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>> [    0.030802] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>> [    0.030816] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>> [    0.030829] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>> [    0.030842] ================================================================================ ```
>>
>>

The function is as follows, so when legacy_serial_console == -1 as in your situation, the pointers 
are just not used.

static int __init ioremap_legacy_serial_console(void)
{
	struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console];
	struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console];
	void __iomem *vaddr;

	if (legacy_serial_console < 0)
		return 0;

...
}

When I look into the generated code (UBSAN not selected), we see the verification and the bail-out 
is done prior to any calculation based on legacy_serial_console.

00000000 <ioremap_legacy_serial_console>:
    0:	94 21 ff e0 	stwu    r1,-32(r1)
    4:	3d 20 00 00 	lis     r9,0
			6: R_PPC_ADDR16_HA	.data
    8:	7c 08 02 a6 	mflr    r0
    c:	bf 81 00 10 	stmw    r28,16(r1)
   10:	3b 80 00 00 	li      r28,0
   14:	83 a9 00 00 	lwz     r29,0(r9)
			16: R_PPC_ADDR16_LO	.data
   18:	90 01 00 24 	stw     r0,36(r1)
   1c:	2c 1d 00 00 	cmpwi   r29,0
   20:	41 80 00 80 	blt     a0 <ioremap_legacy_serial_console+0xa0>


So, is it normal that UBSAN reports an error here ?

Thanks
Christophe

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
  2021-05-07  8:31   ` Christophe Leroy
@ 2021-05-07  8:42     ` Paul Menzel
  2021-05-07  8:59       ` Christophe Leroy
  2021-05-07 20:59     ` Segher Boessenkool
  1 sibling, 1 reply; 7+ messages in thread
From: Paul Menzel @ 2021-05-07  8:42 UTC (permalink / raw)
  To: Christophe Leroy, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras
  Cc: Andrey Ryabinin, linuxppc-dev

[+Andrey]

Dear Christophe,


Am 07.05.21 um 10:31 schrieb Christophe Leroy:

> Le 06/05/2021 à 21:32, Paul Menzel a écrit :
>> [corrected subject]
>>
>> Am 06.05.21 um 21:31 schrieb Paul Menzel:

>>> On the POWER8 system IBM S822LC, Linux 5.13+, built with USSAN, logs 
>>> the warning below.
>>>
>>> ```
>>> [    0.030091] 
>>> ================================================================================
>>> [    0.030295] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
>>> [    0.030325] index -1 is out of range for type 'legacy_serial_info [8]'
>>> [    0.030350] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>>> [    0.030360] Call Trace:
>>> [    0.030363] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>>> [    0.030386] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>>> [    0.030400] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>>> [    0.030414] [c000000024f1bc20] [c000000001711588] ioremap_legacy_serial_console+0x54/0x144
>>> [    0.030430] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>>> [    0.030444] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>>> [    0.030458] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>>> [    0.030471] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>>> [    0.030484] ================================================================================ 
>>>
>>> [    0.030641] ================================================================================
>>> [    0.030668] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:360:58
>>> [    0.030697] index -1 is out of range for type 'plat_serial8250_port [9]'
>>> [    0.030721] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>>> [    0.030730] Call Trace:
>>> [    0.030733] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>>> [    0.030749] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>>> [    0.030762] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>>> [    0.030775] [c000000024f1bc20] [c0000000017115a0] ioremap_legacy_serial_console+0x6c/0x144
>>> [    0.030790] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>>> [    0.030802] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>>> [    0.030816] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>>> [    0.030829] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>>> [    0.030842] ================================================================================
>>> ```
> 
> The function is as follows, so when legacy_serial_console == -1 as in 
> your situation, the pointers are just not used.
> 
> static int __init ioremap_legacy_serial_console(void)
> {
>      struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console];
>      struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console];
>      void __iomem *vaddr;
> 
>      if (legacy_serial_console < 0)
>          return 0;
> 
> ...
> }
> 
> When I look into the generated code (UBSAN not selected), we see the 
> verification and the bail-out is done prior to any calculation based on 
> legacy_serial_console.
> 
> 00000000 <ioremap_legacy_serial_console>:
>    0:    94 21 ff e0     stwu    r1,-32(r1)
>    4:    3d 20 00 00     lis     r9,0
>              6: R_PPC_ADDR16_HA    .data
>    8:    7c 08 02 a6     mflr    r0
>    c:    bf 81 00 10     stmw    r28,16(r1)
>   10:    3b 80 00 00     li      r28,0
>   14:    83 a9 00 00     lwz     r29,0(r9)
>              16: R_PPC_ADDR16_LO    .data
>   18:    90 01 00 24     stw     r0,36(r1)
>   1c:    2c 1d 00 00     cmpwi   r29,0
>   20:    41 80 00 80     blt     a0 <ioremap_legacy_serial_console+0xa0>
> 
> So, is it normal that UBSAN reports an error here ?

If it’s useful, I could disassemble the code here. But please tell me how.

Sorry, I do not know. I just selected the option, and saw the error. 
Maybe Andrey has an idea.


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
  2021-05-07  8:42     ` Paul Menzel
@ 2021-05-07  8:59       ` Christophe Leroy
  2021-05-07 17:52         ` Paul Menzel
  0 siblings, 1 reply; 7+ messages in thread
From: Christophe Leroy @ 2021-05-07  8:59 UTC (permalink / raw)
  To: Paul Menzel, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
  Cc: Andrey Ryabinin, linuxppc-dev



Le 07/05/2021 à 10:42, Paul Menzel a écrit :
> [+Andrey]
> 
> Dear Christophe,
> 
> 
> Am 07.05.21 um 10:31 schrieb Christophe Leroy:
> 
>> Le 06/05/2021 à 21:32, Paul Menzel a écrit :
>>> [corrected subject]
>>>
>>> Am 06.05.21 um 21:31 schrieb Paul Menzel:
> 
>>>> On the POWER8 system IBM S822LC, Linux 5.13+, built with USSAN, logs the warning below.
>>>>
>>>> ```
>>>> [    0.030091] ================================================================================
>>>> [    0.030295] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
>>>> [    0.030325] index -1 is out of range for type 'legacy_serial_info [8]'
>>>> [    0.030350] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>>>> [    0.030360] Call Trace:
>>>> [    0.030363] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>>>> [    0.030386] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>>>> [    0.030400] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>>>> [    0.030414] [c000000024f1bc20] [c000000001711588] ioremap_legacy_serial_console+0x54/0x144
>>>> [    0.030430] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>>>> [    0.030444] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>>>> [    0.030458] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>>>> [    0.030471] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>>>> [    0.030484] ================================================================================
>>>> [    0.030641] ================================================================================
>>>> [    0.030668] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:360:58
>>>> [    0.030697] index -1 is out of range for type 'plat_serial8250_port [9]'
>>>> [    0.030721] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>>>> [    0.030730] Call Trace:
>>>> [    0.030733] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>>>> [    0.030749] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>>>> [    0.030762] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>>>> [    0.030775] [c000000024f1bc20] [c0000000017115a0] ioremap_legacy_serial_console+0x6c/0x144
>>>> [    0.030790] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>>>> [    0.030802] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>>>> [    0.030816] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>>>> [    0.030829] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>>>> [    0.030842] ================================================================================
>>>> ```
>>
>> The function is as follows, so when legacy_serial_console == -1 as in your situation, the pointers 
>> are just not used.
>>
>> static int __init ioremap_legacy_serial_console(void)
>> {
>>      struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console];
>>      struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console];
>>      void __iomem *vaddr;
>>
>>      if (legacy_serial_console < 0)
>>          return 0;
>>
>> ...
>> }
>>
>> When I look into the generated code (UBSAN not selected), we see the verification and the bail-out 
>> is done prior to any calculation based on legacy_serial_console.
>>
>> 00000000 <ioremap_legacy_serial_console>:
>>    0:    94 21 ff e0     stwu    r1,-32(r1)
>>    4:    3d 20 00 00     lis     r9,0
>>              6: R_PPC_ADDR16_HA    .data
>>    8:    7c 08 02 a6     mflr    r0
>>    c:    bf 81 00 10     stmw    r28,16(r1)
>>   10:    3b 80 00 00     li      r28,0
>>   14:    83 a9 00 00     lwz     r29,0(r9)
>>              16: R_PPC_ADDR16_LO    .data
>>   18:    90 01 00 24     stw     r0,36(r1)
>>   1c:    2c 1d 00 00     cmpwi   r29,0
>>   20:    41 80 00 80     blt     a0 <ioremap_legacy_serial_console+0xa0>
>>
>> So, is it normal that UBSAN reports an error here ?
> 
> If it’s useful, I could disassemble the code here. But please tell me how.
> 
> Sorry, I do not know. I just selected the option, and saw the error. Maybe Andrey has an idea.
> 

No need for you to disassemble, I just wanted to show that without UBSAN there is no problem with 
the index as it is used only after boundary checking. (But if you want to do so, if is just an 
'objdump -dr legacy_serial.o')

Now, with UBSAN, I see that UBSAN does the verification of the index earlier than expected. So what 
to do here, we can modify the code, but that modification would just be to make UBSAN happy as there 
is no problem in itself.

Christophe

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
  2021-05-07  8:59       ` Christophe Leroy
@ 2021-05-07 17:52         ` Paul Menzel
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Menzel @ 2021-05-07 17:52 UTC (permalink / raw)
  To: Christophe Leroy, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras
  Cc: Andrey Ryabinin, linuxppc-dev

Dear Christophe,


Am 07.05.21 um 10:59 schrieb Christophe Leroy:

> Le 07/05/2021 à 10:42, Paul Menzel a écrit :
>> [+Andrey]

>> Am 07.05.21 um 10:31 schrieb Christophe Leroy:
>>
>>> Le 06/05/2021 à 21:32, Paul Menzel a écrit :
>>>> [corrected subject]
>>>>
>>>> Am 06.05.21 um 21:31 schrieb Paul Menzel:
>>
>>>>> On the POWER8 system IBM S822LC, Linux 5.13+, built with USSAN, 
>>>>> logs the warning below.
>>>>>
>>>>> ```
>>>>> [    0.030091] ================================================================================
>>>>> [    0.030295] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
>>>>> [    0.030325] index -1 is out of range for type 'legacy_serial_info [8]'
>>>>> [    0.030350] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>>>>> [    0.030360] Call Trace:
>>>>> [    0.030363] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>>>>> [    0.030386] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>>>>> [    0.030400] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>>>>> [    0.030414] [c000000024f1bc20] [c000000001711588] ioremap_legacy_serial_console+0x54/0x144
>>>>> [    0.030430] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>>>>> [    0.030444] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>>>>> [    0.030458] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>>>>> [    0.030471] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>>>>> [    0.030484] ================================================================================
>>>>> [    0.030641] ================================================================================
>>>>> [    0.030668] UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:360:58
>>>>> [    0.030697] index -1 is out of range for type 'plat_serial8250_port [9]'
>>>>> [    0.030721] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0+ #2
>>>>> [    0.030730] Call Trace:
>>>>> [    0.030733] [c000000024f1bad0] [c0000000009f4330] dump_stack+0xc4/0x114 (unreliable)
>>>>> [    0.030749] [c000000024f1bb20] [c0000000009efed0] ubsan_epilogue+0x18/0x78
>>>>> [    0.030762] [c000000024f1bb80] [c0000000009efafc] __ubsan_handle_out_of_bounds+0xac/0xd0
>>>>> [    0.030775] [c000000024f1bc20] [c0000000017115a0] ioremap_legacy_serial_console+0x6c/0x144
>>>>> [    0.030790] [c000000024f1bc70] [c0000000000123c0] do_one_initcall+0x60/0x2c0
>>>>> [    0.030802] [c000000024f1bd40] [c000000001704bc4] kernel_init_freeable+0x19c/0x25c
>>>>> [    0.030816] [c000000024f1bda0] [c000000000012a2c] kernel_init+0x2c/0x180
>>>>> [    0.030829] [c000000024f1be10] [c00000000000d6ec] ret_from_kernel_thread+0x5c/0x70
>>>>> [    0.030842] ================================================================================
>>>>> ```
>>>
>>> The function is as follows, so when legacy_serial_console == -1 as in 
>>> your situation, the pointers are just not used.
>>>
>>> static int __init ioremap_legacy_serial_console(void)
>>> {
>>>      struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console];
>>>      struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console];
>>>      void __iomem *vaddr;
>>>
>>>      if (legacy_serial_console < 0)
>>>          return 0;
>>>
>>> ...
>>> }
>>>
>>> When I look into the generated code (UBSAN not selected), we see the 
>>> verification and the bail-out is done prior to any calculation based 
>>> on legacy_serial_console.
>>>
>>> 00000000 <ioremap_legacy_serial_console>:
>>>    0:    94 21 ff e0     stwu    r1,-32(r1)
>>>    4:    3d 20 00 00     lis     r9,0
>>>              6: R_PPC_ADDR16_HA    .data
>>>    8:    7c 08 02 a6     mflr    r0
>>>    c:    bf 81 00 10     stmw    r28,16(r1)
>>>   10:    3b 80 00 00     li      r28,0
>>>   14:    83 a9 00 00     lwz     r29,0(r9)
>>>              16: R_PPC_ADDR16_LO    .data
>>>   18:    90 01 00 24     stw     r0,36(r1)
>>>   1c:    2c 1d 00 00     cmpwi   r29,0
>>>   20:    41 80 00 80     blt     a0 <ioremap_legacy_serial_console+0xa0>
>>>
>>> So, is it normal that UBSAN reports an error here ?
>>
>> If it’s useful, I could disassemble the code here. But please tell me 
>> how.
>>
>> Sorry, I do not know. I just selected the option, and saw the error. 
>> Maybe Andrey has an idea.
> 
> No need for you to disassemble, I just wanted to show that without UBSAN 
> there is no problem with the index as it is used only after boundary 
> checking. (But if you want to do so, if is just an 'objdump -dr 
> legacy_serial.o')

Thank you for the hint.

> Now, with UBSAN, I see that UBSAN does the verification of the index 
> earlier than expected. So what to do here, we can modify the code, but 
> that modification would just be to make UBSAN happy as there is no 
> problem in itself.

In #gcc@irc.freenode.net I was told by zid (they weren’t so happy with 
the wording), but maybe you understand it:

> It's not legal C to generate pointers to things other than 0,
> objects, or 1 past the end of an object, not just dereference them,
> so technically that's not legal per the C spec.
> 
> In practice it won't matter until it's dereferenced of course unless
> you're doing something weird, let's say.. instrumenting the code

Kind regards,

Paul

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56
  2021-05-07  8:31   ` Christophe Leroy
  2021-05-07  8:42     ` Paul Menzel
@ 2021-05-07 20:59     ` Segher Boessenkool
  1 sibling, 0 replies; 7+ messages in thread
From: Segher Boessenkool @ 2021-05-07 20:59 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Paul Menzel, linuxppc-dev, Paul Mackerras

On Fri, May 07, 2021 at 10:31:42AM +0200, Christophe Leroy wrote:
> The function is as follows, so when legacy_serial_console == -1 as in your 
> situation, the pointers are just not used.

And it is still undefined behaviour.  C11 6.5.6/8 has
  If both the pointer operand and the result point to elements of the
  same array object, or one past the last element of the array object,
  the evaluation shall not produce an overflow; otherwise, the behavior
  is undefined.
(this is for adding an integer to a pointer).

> When I look into the generated code (UBSAN not selected), we see the 
> verification and the bail-out is done prior to any calculation based on 
> legacy_serial_console.

Yes, you got lucky.  Generating the code you wanted is one of the things
the compiler is allowed to do for UB.

> So, is it normal that UBSAN reports an error here ?

Yes.  It detected undefined behaviour just fine, it did exactly its
job :-)


Segher

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-05-07 21:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-06 19:31 WARNING: CPU: 0 PID: 1 at arch/powerpc/lib/feature-fixups.c:109 do_feature_fixups+0xb0/0xf0 Paul Menzel
2021-05-06 19:32 ` UBSAN: array-index-out-of-bounds in arch/powerpc/kernel/legacy_serial.c:359:56 Paul Menzel
2021-05-07  8:31   ` Christophe Leroy
2021-05-07  8:42     ` Paul Menzel
2021-05-07  8:59       ` Christophe Leroy
2021-05-07 17:52         ` Paul Menzel
2021-05-07 20:59     ` Segher Boessenkool

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.