Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 2 Mar 2022 11:09:49 +0100
From:   Corentin Labbe <clabbe.montjoie@gmail.com>
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>,
        "Russell King (Oracle)" <linux@armlinux.org.uk>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: boot flooded with unwind: Index not found
Message-ID: <Yh9CbcrfDvN2Z9Y9@Red>
References: <Yh5ASXVoWoMj7/Rr@Red>
 <Yh5AlfprVAZvJDJA@shell.armlinux.org.uk>
 <CAMj1kXGRTM99F_Q29Q4G2Q4L6WSHn2YY+_QZCXQGmw=yWPe1mQ@mail.gmail.com>
 <CAMj1kXEy6n3zZ8Z51kP=tTuOU0xCXLLfC-b6BMpdsjMoM7zGBg@mail.gmail.com>
 <Yh8tWdiWPgZLyQtx@Red>
 <CAMj1kXGRtdftpoqmd7HBonBBS67jO=YWzoESPAagGfQBZUDQWg@mail.gmail.com>
 <Yh8w7ldudhmbYv4N@Red>
 <CAMj1kXHri2_tnYhu2gE9xTUOxLY9v1=zODCo1BGfjFTKukiedA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAMj1kXHri2_tnYhu2gE9xTUOxLY9v1=zODCo1BGfjFTKukiedA@mail.gmail.com>
Precedence: bulk

Le Wed, Mar 02, 2022 at 10:45:46AM +0100, Ard Biesheuvel a ?crit :
> On Wed, 2 Mar 2022 at 09:55, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
> >
> > Le Wed, Mar 02, 2022 at 09:44:52AM +0100, Ard Biesheuvel a ?crit :
> > > On Wed, 2 Mar 2022 at 09:40, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
> > > >
> > > > Le Tue, Mar 01, 2022 at 05:52:30PM +0100, Ard Biesheuvel a ?crit :
> > > > > On Tue, 1 Mar 2022 at 17:37, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, 1 Mar 2022 at 16:52, Russell King (Oracle)
> > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > >
> > > > > > > On Tue, Mar 01, 2022 at 04:48:25PM +0100, Corentin Labbe wrote:
> > > > > > > > Hello
> > > > > > > >
> > > > > > > > I booted today linux-next (20220301) and my boot is flooded with:
> > > > > > > > [    0.000000] unwind: Index not found c0f0c440
> > > > > > > > [    0.000000] unwind: Index not found 00000000
> > > > > > > > [    0.000000] unwind: Index not found c0f0c440
> > > > > > > > [    0.000000] unwind: Index not found 00000000
> > > > > > > >
> > > > > > > > This happen on a sun8i-a83t-bananapi-m3
> > > > > > >
> > > > > > > Have you enabled vmapped stacks?
> > > > > > >
> > > > > >
> > > > > > This is probably related to
> > > > > >
> > > > > > 538b9265c063 ARM: unwind: track location of LR value in stack frame
> > > > > >
> > > > > > which removes a kernel_text_address() check on frame->pc as it is
> > > > > > essentially redundant, given that we won't find unwind data otherwise.
> > > > > > Unfortunately, I failed to realise that the other check carries a
> > > > > > pr_warn(), which may apparently fire spuriously in some cases.
> > > > > >
> > > > > > The 0x0 value can easily be filtered out, but i would be interesting
> > > > > > where the other value originates from. We might be able to solve this
> > > > > > with a simple .nounwind directive in a asm routine somewhere.
> > > > > >
> > > > > > I'll prepare a patch that disregards the 0x0 value - could you check
> > > > > > in the mean time what the address 0xcf0c440 coincides with in your
> > > > > > build?
> > > > >
> > > > > Something like the below should restore the previous behavior, while
> > > > > taking the kernel_text_address() check out of the hot path.
> > > > >
> > > > > --- a/arch/arm/kernel/unwind.c
> > > > > +++ b/arch/arm/kernel/unwind.c
> > > > > @@ -400,7 +400,8 @@ int unwind_frame(struct stackframe *frame)
> > > > >
> > > > >         idx = unwind_find_idx(frame->pc);
> > > > >         if (!idx) {
> > > > > -               pr_warn("unwind: Index not found %08lx\n", frame->pc);
> > > > > +               if (frame->pc && kernel_text_address(frame->pc))
> > > > > +                       pr_warn("unwind: Index not found %08lx\n", frame->pc);
> > > > >                 return -URC_FAILURE;
> > > > >         }
> > > >
> > > > Hello
> > > >
> > > > This is a more detailed trace from my follow up after your patch:
> > >
> > > So the log below is from a kernel that has the above patch applied?
> > > Could you please share the .config?
> > >
> >
> > Yes this is a kernel with above patch applied (this board do not boot without it).
> 
> It's not entirely clear to me how (or whether) the recent changes to
> unwind.c cause this issue, but one thing that stands out in the
> current code is the unguarded dereference of a value pulled of the
> stack as a memory address.
> 
> It is worth noting that the only unwind entries in vmlinux that load
> SP from the stack directly (as opposed to unwinding it by moving from
> the frame pointer or by addition/subtraction) are the
> __irq_svc/__pabt_svc/__dabt_svc entry routines, and given that the
> bogus address 60000013 looks suspiciously like a PSR value (which is
> stored in the vicinity of SP on the exception stack), my suspicion is
> that some unwinder annotations are out of sync with the actual code.
> 
> So while the below does not fix the root cause, i.e., that the
> unwinder unwinds SP incorrectly causing us to dereference a bogus
> pointer, it should avoid the subsequent crash. Please give it a go.
> 
> --- a/arch/arm/kernel/unwind.c
> +++ b/arch/arm/kernel/unwind.c
> @@ -27,6 +27,7 @@
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/uaccess.h>
>  #include <linux/list.h>
> 
>  #include <asm/sections.h>
> @@ -236,10 +237,11 @@ static int unwind_pop_register(struct
> unwind_ctrl_block *ctrl,
>                 if (*vsp >= (unsigned long *)ctrl->sp_high)
>                         return -URC_FAILURE;
> 
> -       /* Use READ_ONCE_NOCHECK here to avoid this memory access
> -        * from being tracked by KASAN.
> +       /* Use get_kernel_nofault() here to avoid this memory access
> +        * from causing a fatal fault, and from being tracked by KASAN.
>          */
> -       ctrl->vrs[reg] = READ_ONCE_NOCHECK(*(*vsp));
> +       if (get_kernel_nofault(ctrl->vrs[reg], *vsp))
> +               return -URC_FAILURE;
>         if (reg == 14)
>                 ctrl->lr_addr = *vsp;
>         (*vsp)++;

The crash disappeared (but the suspicious RCU usage is still here).