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