| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1656572413.pbaqjnrrcl.naveen@linux.ibm.com>
Date: Thu, 30 Jun 2022 13:35:08 +0530
From: "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
Sathvika Vasireddy <sv@...ux.ibm.com>,
Sathvika Vasireddy <sv@...ux.vnet.ibm.com>
Cc: "aik@...abs.ru" <aik@...abs.ru>,
"benh@...nel.crashing.org" <benh@...nel.crashing.org>,
Chen Zhongjin <chenzhongjin@...wei.com>,
"jpoimboe@...hat.com" <jpoimboe@...hat.com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Marc Zyngier <maz@...nel.org>,
"mbenes@...e.cz" <mbenes@...e.cz>,
"mingo@...hat.com" <mingo@...hat.com>,
"mpe@...erman.id.au" <mpe@...erman.id.au>,
"paulus@...ba.org" <paulus@...ba.org>,
"peterz@...radead.org" <peterz@...radead.org>,
"rostedt@...dmis.org" <rostedt@...dmis.org>
Subject: Re: [RFC PATCH v3 11/12] powerpc: Remove unreachable() from WARN_ON()
Christophe Leroy wrote:
> Hi Sathvika,
>
> Adding ARM people as they seem to face the same kind of problem (see
> https://patchwork.kernel.org/project/linux-kbuild/patch/20220623014917.199563-33-chenzhongjin@huawei.com/)
>
> Le 27/06/2022 à 17:35, Sathvika Vasireddy a écrit :
>>
>> On 25/06/22 12:16, Christophe Leroy wrote:
>>>
>>> Le 24/06/2022 à 20:32, Sathvika Vasireddy a écrit :
>>>> objtool is throwing *unannotated intra-function call*
>>>> warnings with a few instructions that are marked
>>>> unreachable. Remove unreachable() from WARN_ON()
>>>> to fix these warnings, as the codegen remains same
>>>> with and without unreachable() in WARN_ON().
>>> Did you try the two exemples described in commit 1e688dd2a3d6
>>> ("powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with
>>> asm goto") ?
>>>
>>> Without your patch:
>>>
>>> 00000640 <test>:
>>> 640: 81 23 00 84 lwz r9,132(r3)
>>> 644: 71 29 40 00 andi. r9,r9,16384
>>> 648: 40 82 00 0c bne 654 <test+0x14>
>>> 64c: 80 63 00 0c lwz r3,12(r3)
>>> 650: 4e 80 00 20 blr
>>> 654: 0f e0 00 00 twui r0,0
>>>
>>> 00000658 <test9w>:
>>> 658: 2c 04 00 00 cmpwi r4,0
>>> 65c: 41 82 00 0c beq 668 <test9w+0x10>
>>> 660: 7c 63 23 96 divwu r3,r3,r4
>>> 664: 4e 80 00 20 blr
>>> 668: 0f e0 00 00 twui r0,0
>>> 66c: 38 60 00 00 li r3,0
>>> 670: 4e 80 00 20 blr
>>>
>>>
>>> With your patch:
>>>
>>> 00000640 <test>:
>>> 640: 81 23 00 84 lwz r9,132(r3)
>>> 644: 71 29 40 00 andi. r9,r9,16384
>>> 648: 40 82 00 0c bne 654 <test+0x14>
>>> 64c: 80 63 00 0c lwz r3,12(r3)
>>> 650: 4e 80 00 20 blr
>>> 654: 0f e0 00 00 twui r0,0
>>> 658: 4b ff ff f4 b 64c <test+0xc> <==
>>>
>>> 0000065c <test9w>:
>>> 65c: 2c 04 00 00 cmpwi r4,0
>>> 660: 41 82 00 0c beq 66c <test9w+0x10>
>>> 664: 7c 63 23 96 divwu r3,r3,r4
>>> 668: 4e 80 00 20 blr
>>> 66c: 0f e0 00 00 twui r0,0
>>> 670: 38 60 00 00 li r3,0 <==
>>> 674: 4e 80 00 20 blr <==
>>> 678: 38 60 00 00 li r3,0
>>> 67c: 4e 80 00 20 blr
>>>
>> The builtin variant of unreachable (__builtin_unreachable()) works.
>>
>> How about using that instead of unreachable() ?
>>
>>
>
> In fact the problem comes from the macro annotate_unreachable() which is
> called by unreachable() before calling __build_unreachable().
>
> Seems like this macro adds (after the unconditional trap twui) a call to
> an empty function whose address is listed in section .discard.unreachable
>
> 1c78: 00 00 e0 0f twui r0,0
> 1c7c: 55 e7 ff 4b bl 3d0
> <qdisc_root_sleeping_lock.part.0>
>
>
> RELOCATION RECORDS FOR [.discard.unreachable]:
> OFFSET TYPE VALUE
> 0000000000000000 R_PPC64_REL32 .text+0x00000000000003d0
>
> The problem is that that function has size 0:
>
> 00000000000003d0 l F .text 0000000000000000
> qdisc_root_sleeping_lock.part.0
>
>
> And objtool is not prepared for a function with size 0.
annotate_unreachable() seems to have been introduced in commit
649ea4d5a624f0 ("objtool: Assume unannotated UD2 instructions are dead
ends").
Objtool considers 'ud2' instruction to be fatal, so BUG() has
__builtin_unreachable(), rather than unreachable(). See commit
bfb1a7c91fb775 ("x86/bug: Merge annotate_reachable() into _BUG_FLAGS()
asm"). For the same reason, __WARN_FLAGS() is annotated with
_ASM_REACHABLE so that objtool can differentiate warnings from a BUG().
On powerpc, we use trap variants for both and don't have a special
instruction for a BUG(). As such, for _WARN_FLAGS(), using
__builtin_unreachable() suffices to achieve optimal code generation from
the compiler. Objtool would consider subsequent instructions to be
reachable. For BUG(), we can continue to use unreachable() so that
objtool can differentiate these from traps used in warnings.
>
> The following changes to objtool seem to fix the problem, most warning
> are gone with that change.
>
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 63218f5799c2..37c0a268b7ea 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -77,6 +77,8 @@ static int symbol_by_offset(const void *key, const
> struct rb_node *node)
>
> if (*o < s->offset)
> return -1;
> + if (*o == s->offset && !s->len)
> + return 0;
> if (*o >= s->offset + s->len)
> return 1;
>
> @@ -400,7 +402,7 @@ static void elf_add_symbol(struct elf *elf, struct
> symbol *sym)
> * Don't store empty STT_NOTYPE symbols in the rbtree. They
> * can exist within a function, confusing the sorting.
> */
> - if (!sym->len)
> + if (sym->type == STT_NOTYPE && !sym->len)
> rb_erase(&sym->node, &sym->sec->symbol_tree);
> }
Is there a reason to do this, rather than change __WARN_FLAGS() to use
__builtin_unreachable()? Or, are you seeing an issue with unreachable()
elsewhere in the kernel?
- Naveen
Powered by blists - more mailing lists