Messages in this thread | | | From | Jann Horn <> | Date | Wed, 18 Dec 2019 23:33:54 +0100 | Subject | Re: [PATCH v6 4/4] x86/kasan: Print original address on #GP |
| |
On Wed, Dec 11, 2019 at 6:37 PM Borislav Petkov <bp@alien8.de> wrote: > On Mon, Dec 09, 2019 at 03:31:20PM +0100, Jann Horn wrote: > > arch/x86/kernel/traps.c | 12 ++++++++++- > > arch/x86/mm/kasan_init_64.c | 21 ------------------- > > include/linux/kasan.h | 6 ++++++ > > mm/kasan/report.c | 40 +++++++++++++++++++++++++++++++++++++ > > 4 files changed, 57 insertions(+), 22 deletions(-) > > I need a KASAN person ACK here, I'd guess.
Right, I got a Reviewed-by from Dmitry for v2, but cleared that when I made changes to the patch later - I'll ask Dmitry for a fresh ack on the v7 patch.
[...] > > - die(desc, regs, error_code); > > + flags = oops_begin(); > > + sig = SIGSEGV; > > + __die_header(desc, regs, error_code); > > + if (hint == GP_NON_CANONICAL) > > + kasan_non_canonical_hook(gp_addr); > > + if (__die_body(desc, regs, error_code)) > > + sig = 0; > > + oops_end(flags, regs, sig); > > Instead of opencoding it like this, can we add a > > die_addr(desc, regs, error_code, gp_addr); > > to arch/x86/kernel/dumpstack.c and call it from here: > > if (hint != GP_NON_CANONICAL) > gp_addr = 0; > > die_addr(desc, regs, error_code, gp_addr);
Okay, so I'll make __die_header() and __die_body() static, introduce and hook up die_addr() in patch 3/4, and then in patch 4/4 insert the call to the KASAN hook.
> This way you won't need to pass down to die_addr() the hint too - you > code into gp_addr whether it was non-canonical or not. > > The > > + if (addr < KASAN_SHADOW_OFFSET) > + return; > > check in kasan_non_canonical_hook() would then catch it when addr == 0.
I'll add an explicit check for nonzero address before calling kasan_non_canonical_hook() so that the semantics are a bit more cleanly split.
| |