PANIC: double fault in fixup_bad_iret

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit: 7b4cb0a4 Add linux-next specific files for 20200525
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=15dc34ba100000
kernel config: https://syzkaller.appspot.com/x/.config?x=47b0740d89299c10
dashboard link: https://syzkaller.appspot.com/bug?extid=dc1fa714cb070b184db5
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14678626100000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1017ef06100000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+dc1fa7...@syzkaller.appspotmail.com

traps: PANIC: double fault, error_code: 0x0
double fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 7280 Comm: syz-executor776 Not tainted 5.7.0-rc7-next-20200525-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:fixup_bad_iret+0x24/0x170 arch/x86/kernel/traps.c:665
Code: eb cb 0f 1f 40 00 41 55 49 bd 00 00 00 00 00 fc ff df 41 54 55 48 89 fd 48 c7 c7 80 8a 25 88 53 48 81 ec 40 01 00 00 48 89 e3 <48> c7 04 24 b3 8a b5 41 48 c7 44 24 08 bf d3 49 89 48 c1 eb 03 48
RSP: 0018:fffffe0000001fb8 EFLAGS: 00010086
RAX: fffffffffffffff7 RBX: fffffe0000001fb8 RCX: ffffffff87e00d57
RDX: 0000000000000000 RSI: ffffffff87e009c8 RDI: ffffffff88258a80
RBP: fffffe0000002120 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000001f65880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: fffffe0000001fa8 CR3: 00000000a02aa000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<ENTRY_TRAMPOLINE>
error_entry+0xb8/0xc0 arch/x86/entry/entry_64.S:1375
RIP: 0010:native_irq_return_iret+0x0/0x2
Code: 5a 41 59 41 58 58 59 5a 5e 5f 48 83 c4 08 e9 10 00 00 00 90 0f 1f 44 00 00 66 2e 0f 1f 84 00 00 00 00 00 f6 44 24 20 04 75 02 <48> cf 57 0f 01 f8 0f 1f 00 65 48 8b 3c 25 00 b0 01 00 48 89 07 48
RSP: 0018:fffffe00000021d8 EFLAGS: 00010046 ORIG_RAX: 0000000000000000
RAX: fffffffffffffff7 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000020000100
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
RIP: 0033:0x3bfd19e0df38d197
Code: Bad RIP value.
RSP: 002b:00007fffaaa547c8 EFLAGS: 00000346 </ENTRY_TRAMPOLINE>
Modules linked in:
---[ end trace d6561a908e3835a1 ]---
RIP: 0010:fixup_bad_iret+0x24/0x170 arch/x86/kernel/traps.c:665
Code: eb cb 0f 1f 40 00 41 55 49 bd 00 00 00 00 00 fc ff df 41 54 55 48 89 fd 48 c7 c7 80 8a 25 88 53 48 81 ec 40 01 00 00 48 89 e3 <48> c7 04 24 b3 8a b5 41 48 c7 44 24 08 bf d3 49 89 48 c1 eb 03 48
RSP: 0018:fffffe0000001fb8 EFLAGS: 00010086
RAX: fffffffffffffff7 RBX: fffffe0000001fb8 RCX: ffffffff87e00d57
RDX: 0000000000000000 RSI: ffffffff87e009c8 RDI: ffffffff88258a80
RBP: fffffe0000002120 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000001f65880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: fffffe0000001fa8 CR3: 00000000a02aa000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Dmitry Vyukov]

On Fri, May 29, 2020 at 3:14 PM syzbot
<syzbot+dc1fa7...@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 7b4cb0a4 Add linux-next specific files for 20200525
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=15dc34ba100000
> kernel config: https://syzkaller.appspot.com/x/.config?x=47b0740d89299c10
> dashboard link: https://syzkaller.appspot.com/bug?extid=dc1fa714cb070b184db5
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14678626100000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1017ef06100000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+dc1fa7...@syzkaller.appspotmail.com

From the reproducer it seems to be either x86 related or ptrace related.

> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/000000000000d2474c05a6c938fe%40google.com.

[Thomas Gleixner]

Dmitry,

Dmitry Vyukov <dvy...@google.com> writes:
> On Fri, May 29, 2020 at 3:14 PM syzbot
> <syzbot+dc1fa7...@syzkaller.appspotmail.com> wrote:
>>
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: 7b4cb0a4 Add linux-next specific files for 20200525
>> git tree: linux-next
>> console output: https://syzkaller.appspot.com/x/log.txt?x=15dc34ba100000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=47b0740d89299c10
>> dashboard link: https://syzkaller.appspot.com/bug?extid=dc1fa714cb070b184db5
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14678626100000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1017ef06100000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+dc1fa7...@syzkaller.appspotmail.com
>
> From the reproducer it seems to be either x86 related or ptrace
> related.
>

>> RIP: 0010:fixup_bad_iret+0x24/0x170 arch/x86/kernel/traps.c:665

as a quick assumption that's related to KASAN in fixup_bad_iret() which
is a frightenly bad idea. I'm about to verify.

Thanks,

tglx

[Thomas Gleixner]

Exactly as I assumed. With KASAN off, no problem, with KASAN on, insta
crash.

This function needs to be excluded from KASAN or any other of those
magic function. I need to walk the dogs first and will look into fixing
it later.

Thanks,

tglx

[Peter Zijlstra]

Like with KCSAN, we should blanket kill KASAN/UBSAN and friends (at the
very least in arch/x86/) until they get that function attribute stuff
sorted.

[Peter Zijlstra]

On Fri, May 29, 2020 at 06:07:11PM +0200, Peter Zijlstra wrote:

> Like with KCSAN, we should blanket kill KASAN/UBSAN and friends (at the
> very least in arch/x86/) until they get that function attribute stuff
> sorted.

Something like so.

---
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 00e378de8bc0..a90d32b87d7e 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -1,6 +1,14 @@
# SPDX-License-Identifier: GPL-2.0
# Unified Makefile for i386 and x86_64

+#
+# Until such a time that __no_kasan and __no_ubsan work as expected (and are
+# made part of noinstr), don't sanitize anything.
+#
+KASAN_SANITIZE := n
+UBSAN_SANITIZE := n
+KCOV_INSTRUMENT := n
+
# select defconfig based on actual architecture
ifeq ($(ARCH),x86)
ifeq ($(shell uname -m),x86_64)

[Thomas Gleixner]

Peter Zijlstra <pet...@infradead.org> writes:
> On Fri, May 29, 2020 at 06:07:11PM +0200, Peter Zijlstra wrote:
>> Like with KCSAN, we should blanket kill KASAN/UBSAN and friends (at the
>> very least in arch/x86/) until they get that function attribute stuff
>> sorted.
>
> Something like so.

We have noinstr in kernel/rcu as well but that at least has a halfways
correct state, but still....

Thanks,

tglx

[Dmitry Vyukov]

+kasan-dev
+Marco, please send a fix for this

[Marco Elver]

I think Peter wanted to send a patch to add __no_kcsan to noinstr:
https://lkml.kernel.org/r/20200529170...@hirez.programming.kicks-ass.net

In the same patch we can add __no_sanitize_address to noinstr. But:

- We're missing a definition for __no_sanitize_undefined and
__no_sanitize_coverage.

- Could optionally add __no_{kasan,ubsan,kcov}, to be consistent with
__no_kcsan, although I'd just keep __no_sanitize for the unambiguous
names (__no_kcsan is special because __no_sanitize_thread and TSAN
instrumentation is just an implementation detail of KCSAN, which !=
KTSAN).

- We still need the above blanket no-instrument for x86 because of
GCC. We could guard it with "ifdef CONFIG_CC_IS_GCC".

Not sure what the best strategy is to minimize patch conflicts. For
now I could send just the patches to add missing definitions. If you'd
like me to send all patches (including modifying 'noinstr'), let me
know.

Thanks,
-- Marco

[Peter Zijlstra]

On Mon, Jun 01, 2020 at 02:40:31PM +0200, Marco Elver wrote:
> I think Peter wanted to send a patch to add __no_kcsan to noinstr:
> https://lkml.kernel.org/r/20200529170...@hirez.programming.kicks-ass.net
>
> In the same patch we can add __no_sanitize_address to noinstr. But:
>
> - We're missing a definition for __no_sanitize_undefined and
> __no_sanitize_coverage.

Do those function attributes actually work? Because the last time I
played with some of that I didn't.

Specifically: unmarked __always_inline functions must not generate
instrumentation when they're inlined into a __no_*san function.

(and that fails to build on some GCC versions, and I think fails to
actually work on the rest of them, but I'd have to double check)

> - We still need the above blanket no-instrument for x86 because of
> GCC. We could guard it with "ifdef CONFIG_CC_IS_GCC".

Right; so all of GCC is broken vs that function attribute stuff? Any
plans of getting that fixed? Do we have GCC that care?

Does the GCC plugin approach sound like a viable alternative
implementation of all this?

Anyway, we can make it:

KASAN := SANITIZER_HAS_FUNCTION_ATTRIBUTES

or something, and only make that 'y' when the compiler is sane.

> Not sure what the best strategy is to minimize patch conflicts. For
> now I could send just the patches to add missing definitions. If you'd
> like me to send all patches (including modifying 'noinstr'), let me
> know.

If you're going to do patches anyway, might as well do that :-)

[Marco Elver]

You were a bit faster with the other patches ;-) I was still
experimenting the the patches, but let me briefly respond here.

On Tue, 2 Jun 2020 at 11:41, Peter Zijlstra <pet...@infradead.org> wrote:
>
> On Mon, Jun 01, 2020 at 02:40:31PM +0200, Marco Elver wrote:
> > I think Peter wanted to send a patch to add __no_kcsan to noinstr:
> > https://lkml.kernel.org/r/20200529170...@hirez.programming.kicks-ass.net
> >
> > In the same patch we can add __no_sanitize_address to noinstr. But:
> >
> > - We're missing a definition for __no_sanitize_undefined and
> > __no_sanitize_coverage.
>
> Do those function attributes actually work? Because the last time I
> played with some of that I didn't.

__no_sanitize_coverage won't work, because neither compiler has an
attribute to disable coverage instrumentation. I'll try and add this
to compilers, but KCOV_INSTRUMENT := n is in the right places right
now it seems. More on that in the patch adding this.

> Specifically: unmarked __always_inline functions must not generate
> instrumentation when they're inlined into a __no_*san function.
>
> (and that fails to build on some GCC versions, and I think fails to
> actually work on the rest of them, but I'd have to double check)

We'll probably need to bump the required compiler version if anybody
still attempts to use these old compilers with sanitizers. The precise
versions of compilers and what mixes with what is a bit of a
nightmare. For now I'd just say, let's add the attributes, and see
where that gets us. Surely it won't be more broken than before. ;-)

> > - We still need the above blanket no-instrument for x86 because of
> > GCC. We could guard it with "ifdef CONFIG_CC_IS_GCC".
>
> Right; so all of GCC is broken vs that function attribute stuff? Any
> plans of getting that fixed? Do we have GCC that care?
>
> Does the GCC plugin approach sound like a viable alternative
> implementation of all this?

I don't think it's realistic to maintain a GCC plugin like that
indefinitely. We can investigate, but it's not a quick fix.

> Anyway, we can make it:
>
> KASAN := SANITIZER_HAS_FUNCTION_ATTRIBUTES
>
> or something, and only make that 'y' when the compiler is sane.

We have all attributes except __no_sanitize_coverage. GCC <= 7 has
problems with __always_inline, so we may just have to bump the
required compiler or emit a warning.

> > Not sure what the best strategy is to minimize patch conflicts. For
> > now I could send just the patches to add missing definitions. If you'd
> > like me to send all patches (including modifying 'noinstr'), let me
> > know.
>
> If you're going to do patches anyway, might as well do that :-)

I was stuck on trying to find ways to emulate __no_sanitize_coverage
(with no success), and then agonizing which patches to send in which
sequence. ;-) You made that decision by sending the KCSAN noinstr
series first, so let me respond to that with what I think we can add
for KASAN and UBSAN at least.

Thanks,
-- Marco

[Peter Zijlstra]

On Tue, Jun 02, 2020 at 07:51:40PM +0200, Marco Elver wrote:

> We have all attributes except __no_sanitize_coverage. GCC <= 7 has
> problems with __always_inline, so we may just have to bump the
> required compiler or emit a warning.

GCC <= 7 will hard fail the compile with those function attributes.
Bumping the min GCC version for KASAN/UBSAN to avoid that might be best.

> > > Not sure what the best strategy is to minimize patch conflicts. For
> > > now I could send just the patches to add missing definitions. If you'd
> > > like me to send all patches (including modifying 'noinstr'), let me
> > > know.
> >
> > If you're going to do patches anyway, might as well do that :-)
>
> I was stuck on trying to find ways to emulate __no_sanitize_coverage
> (with no success), and then agonizing which patches to send in which
> sequence. ;-) You made that decision by sending the KCSAN noinstr
> series first, so let me respond to that with what I think we can add
> for KASAN and UBSAN at least.

Excellent, thanks!