| 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
| ||
|
Date: Thu, 18 Jul 2019 16:02:46 -0700
From: Nick Desaulniers <ndesaulniers@...gle.com>
To: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Arnd Bergmann <arnd@...db.de>, Jann Horn <jannh@...gle.com>,
Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH 20/22] objtool: Fix seg fault on bad switch table entry
On Mon, Jul 15, 2019 at 10:29 AM Josh Poimboeuf <jpoimboe@...hat.com> wrote:
>
> On Mon, Jul 15, 2019 at 10:24:24AM -0700, Nick Desaulniers wrote:
> > On Sun, Jul 14, 2019 at 5:37 PM Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> > >
> > > In one rare case, Clang generated the following code:
> > >
> > > 5ca: 83 e0 21 and $0x21,%eax
> > > 5cd: b9 04 00 00 00 mov $0x4,%ecx
> > > 5d2: ff 24 c5 00 00 00 00 jmpq *0x0(,%rax,8)
> > > 5d5: R_X86_64_32S .rodata+0x38
> > >
> > > which uses the corresponding jump table relocations:
> > >
> > > 000000000038 000200000001 R_X86_64_64 0000000000000000 .text + 834
> > > 000000000040 000200000001 R_X86_64_64 0000000000000000 .text + 5d9
> > > 000000000048 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000050 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000058 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000060 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000068 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000070 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000078 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000080 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000088 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000090 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000098 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000a0 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000a8 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000b0 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000b8 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000c0 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000c8 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000d0 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000d8 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000e0 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000e8 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000f0 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 0000000000f8 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000100 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000108 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000110 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000118 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000120 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000128 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000130 000200000001 R_X86_64_64 0000000000000000 .text + b96
> > > 000000000138 000200000001 R_X86_64_64 0000000000000000 .text + 82f
> > > 000000000140 000200000001 R_X86_64_64 0000000000000000 .text + 828
> > >
> > > Since %eax was masked with 0x21, only the first two and the last two
> > > entries are possible.
> > >
> > > Objtool doesn't actually emulate all the code, so it isn't smart enough
> > > to know that all the middle entries aren't reachable. They point to the
> > > NOP padding area after the end of the function, so objtool seg faulted
> > > when it tried to dereference a NULL insn->func.
> > >
> > > After this fix, objtool still gives an "unreachable" error because it
> > > stops reading the jump table when it encounters the bad addresses:
> > >
> > > /home/jpoimboe/objtool-tests/adm1275.o: warning: objtool: adm1275_probe()+0x828: unreachable instruction
> > >
> > > While the above code is technically correct, it's very wasteful of
> > > memory -- it uses 34 jump table entries when only 4 are needed. It's
> > > also not possible for objtool to validate this type of switch table
> > > because the unused entries point outside the function and objtool has no
> > > way of determining if that's intentional. Hopefully the Clang folks can
> > > fix it.
> >
> > So this came from
> > drivers/hwmon/pmbus/adm1275.c ?
$ grep switch drivers/hwmon/pmbus/adm1275.c | wc -l
8
$ grep switch drivers/hwmon/pmbus/adm1275.c
switch (reg) {
switch (reg) {
switch (reg) {
switch (data->id) {
switch (config & ADM1075_IRANGE_MASK) {
switch (config & (ADM1275_VRANGE | ADM1272_IRANGE)) {
switch (config & ADM1293_VIN_SEL_MASK) {
switch (config & ADM1293_IRANGE_MASK) {
Looking specifically at the definition of adm1275_probe, I see:
...
switch (data->id) {
...
switch (config & ADM1075_IRANGE_MASK) {
...
switch (config & (ADM1275_VRANGE | ADM1272_IRANGE)) {
...
switch (config & ADM1293_VIN_SEL_MASK) {
...
switch (config & ADM1293_IRANGE_MASK) {
So I assume that the two level switch statement is somehow related.
Maybe the two level switch is transformed into a one level switch with
duplicated case labels? Let me play around in <strikethrough>my
sandbox</strikethrough>godbolt and see if I can reproduce with that
pattern.
--
Thanks,
~Nick Desaulniers
Powered by blists - more mailing lists