Re: [PATCH] Test for riscv fixes

[Alexandre Ghiti]

Hi Edward,

On Sat, Sep 30, 2023 at 1:06 AM Edward AD <twuufnxlz@xxxxxxxxx> wrote:
>
> Hi Alexandre,
>
> On Fri, 29 Sep 2023 10:25:59 +0200 Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> wrote:
> > 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).
> The added check can confirm that the physical page is invalid (whether it is a
> vmalloc allocated page or a slab allocated page), and exit the for loop when it is invalid.

Yes, but to me this is not what happens in the bug report you link:

| BUG: KASAN: out-of-bounds in walk_stackframe+0x130/0x2f2
arch/riscv/kernel/stacktrace.c:59
| Read of size 8 at addr ff20000006d37c38 by task swapper/1/0

So the read at address ff20000006d37c38 is not "normal" according to
KASAN (you can see there is no trap, meaning the physical mapping
exists).

| The buggy address belongs to the virtual mapping at
|  [ff20000006d30000, ff20000006d39000) created by:
| kernel_clone+0x118/0x896 kernel/fork.c:2909

The virtual address is legitimate since the vma exists ^

| The buggy address belongs to the physical page:
| page:ff1c00000250dbc0 refcount:1 mapcount:0 mapping:0000000000000000
index:0x0 pfn:0x9436f

And the physical page also exists ^

So I insist, checking that a physical mapping exists to exit the loop
is not enough, to me, the error here is that the backtrace goes "too
far" at an address where nothing was written before and then KASAN
complains about that, again, we don't take any page fault here so it's
not a problem of existing physical mapping.

>
> Perhaps we can trust the test results of syzbot.
>
> Thanks,
> edward