Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 27 Sep 2021 16:45:43 -0700
From:   Josh Poimboeuf <jpoimboe@redhat.com>
To:     Sean Christopherson <seanjc@google.com>
Cc:     Dmitry Vyukov <dvyukov@google.com>, Marco Elver <elver@google.com>,
        syzbot <syzbot+d08efd12a2905a344291@syzkaller.appspotmail.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        syzkaller-bugs@googlegroups.com, viro@zeniv.linux.org.uk,
        the arch/x86 maintainers <x86@kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        kasan-dev <kasan-dev@googlegroups.com>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [syzbot] upstream test error: KFENCE: use-after-free in
 kvm_fastop_exception
Message-ID: <20210927234543.6waods7rraxseind@treble>
References: <000000000000d6b66705cb2fffd4@google.com>
 <CACT4Y+ZByJ71QfYHTByWaeCqZFxYfp8W8oyrK0baNaSJMDzoUw@mail.gmail.com>
 <CANpmjNMq=2zjDYJgGvHcsjnPNOpR=nj-gQ43hk2mJga0ES+wzQ@mail.gmail.com>
 <CACT4Y+Y1c-kRk83M-qiFY40its+bP3=oOJwsbSrip5AB4vBnYA@mail.gmail.com>
 <YUpr8Vu8xqCDwkE8@google.com>
 <CACT4Y+YuX3sVQ5eHYzDJOtenHhYQqRsQZWJ9nR0sgq3s64R=DA@mail.gmail.com>
 <YVHsV+o7Ez/+arUp@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <YVHsV+o7Ez/+arUp@google.com>
Precedence: bulk

On Mon, Sep 27, 2021 at 04:07:51PM +0000, Sean Christopherson wrote:
> I was asking about the exact location to confirm that the explosion is indeed
> from exception fixup, which is the "unwinder scenario get confused" I was thinking
> of.  Based on the disassembly from syzbot, that does indeed appear to be the case
> here, i.e. this
> 
>   2a:   4c 8b 21                mov    (%rcx),%r12
> 
> is from exception fixup from somewhere in __d_lookup (can't tell exactly what
> it's from, maybe KASAN?).
> 
> > Is there more info on this "the unwinder gets confused"? Bug filed
> > somewhere or an email thread? Is it on anybody's radar?
> 
> I don't know if there's a bug report or if this is on anyone's radar.  The issue
> I've encountered in the past, and what I'm pretty sure is being hit here, is that
> the ORC unwinder doesn't play nice with out-of-line fixup code, presumably because
> there are no tables for the fixup.  I believe kvm_fastop_exception() gets blamed
> because it's the first label that's found when searching back through the tables.

The ORC unwinder actually knows about .fixup, and unwinding through the
.fixup code worked here, as evidenced by the entire stacktrace getting
printed.  Otherwise there would have been a bunch of question marks in
the stack trace.

The problem reported here -- falsely printing kvm_fastop_exception -- is
actually in the arch-independent printing of symbol names, done by
__sprint_symbol().  Most .fixup code fragments are anonymous, in the
sense that they don't have symbols associated with them.  For x86, here
are the only defined symbols in .fixup:

  ffffffff81e02408 T kvm_fastop_exception
  ffffffff81e02728 t .E_read_words
  ffffffff81e0272b t .E_leading_bytes
  ffffffff81e0272d t .E_trailing_bytes
  ffffffff81e02734 t .E_write_words
  ffffffff81e02740 t .E_copy

There's a lot of anonymous .fixup code which happens to be placed in the
gap between "kvm_fastop_exception" and ".E_read_words".  The kernel
symbol printing code will go backwards from the given address and will
print the first symbol it finds.  So any anonymous code in that gap will
falsely be reported as kvm_fastop_exception().

I'm thinking the ideal way to fix this would be getting rid of the
.fixup section altogether, and instead place a function's corresponding
fixup code in a cold part of the original function, with the help of
asm_goto and cold label attributes.

That way, the original faulting function would be printed instead of an
obscure reference to an anonymous .fixup code fragment.  It would have
other benefits as well.  For example, not breaking livepatch...

I'll try to play around with it.

-- 
Josh