Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <f74665d1-4d28-01a2-5694-b06fde202d39@ghiti.fr> <20230928231239.2144579-2-twuufnxlz@gmail.com>
In-Reply-To: <20230928231239.2144579-2-twuufnxlz@gmail.com>
From:   Alexandre Ghiti <alexghiti@rivosinc.com>
Date:   Fri, 29 Sep 2023 10:25:59 +0200
Message-ID: <CAHVXubhG2c=ShFHF4hMMFm8=BYDTyDUduOizujbUWQBW+55wTw@mail.gmail.com>
Subject: Re: [PATCH] riscv: fix out of bounds in walk_stackframe
To:     Edward AD <twuufnxlz@gmail.com>
Cc:     alex@ghiti.fr, aou@eecs.berkeley.edu, conor@kernel.org,
        gregkh@linuxfoundation.org, guoren@kernel.org,
        jirislaby@kernel.org, linux-kernel@vger.kernel.org,
        linux-riscv@lists.infradead.org, linux-serial@vger.kernel.org,
        liushixin2@huawei.com, palmer@dabbelt.com,
        paul.walmsley@sifive.com,
        syzbot+8d2757d62d403b2d9275@syzkaller.appspotmail.com,
        syzkaller-bugs@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Edward,

On Fri, Sep 29, 2023 at 1:12=E2=80=AFAM Edward AD <twuufnxlz@gmail.com> wro=
te:
>
> Add vmalloc and kernel addresses check to prevent invalid access.
>
> Closes: https://lore.kernel.org/all/20230926105949.1025995-2-twuufnxlz@gm=
ail.com/
> Fixes: 5d8544e2d007 ("RISC-V: Generic library routines and assembly")
> Reported-and-test-by: syzbot+8d2757d62d403b2d9275@syzkaller.appspotmail.c=
om
> Link: https://lore.kernel.org/all/0000000000000170df0605ccf91a@google.com=
/T/
> Signed-off-by: Edward AD <twuufnxlz@gmail.com>
> ---
>  arch/riscv/kernel/stacktrace.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrac=
e.c
> index 64a9c093aef9..031a4a35c1d0 100644
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c
> @@ -54,6 +54,9 @@ void notrace walk_stackframe(struct task_struct *task, =
struct pt_regs *regs,
>                         break;
>                 /* Unwind stack frame */
>                 frame =3D (struct stackframe *)fp - 1;
> +               if ((is_vmalloc_addr(frame) && !pfn_valid(page_to_pfn(vma=
lloc_to_page(frame)))) ||
> +                    !virt_addr_valid(frame))
> +                       break;
>                 sp =3D fp;
>                 if (regs && (regs->epc =3D=3D pc) && (frame->fp & 0x7)) {
>                         fp =3D frame->ra;
> --
> 2.25.1
>

I'm still not convinced this will fix the kasan out-of-bounds
accesses, the page can be valid but the read can happen at an offset
not initialized and trigger such errors right? I still think there is
something weird about the stack frame, as to me this should not happen
(but admittedly I don't know much about that).