[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANpmjNNzTGN1xa5Egf2e+twd9n0LgEVUS_sG9nOCzb50NPTKpg@mail.gmail.com>
Date: Thu, 4 Mar 2021 13:02:49 +0100
From: Marco Elver <elver@...gle.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: Alexander Potapenko <glider@...gle.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>,
Michael Ellerman <mpe@...erman.id.au>,
Dmitry Vyukov <dvyukov@...gle.com>,
LKML <linux-kernel@...r.kernel.org>,
linuxppc-dev@...ts.ozlabs.org,
kasan-dev <kasan-dev@...glegroups.com>
Subject: Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
On Thu, 4 Mar 2021 at 13:00, Christophe Leroy
<christophe.leroy@...roup.eu> wrote:
>
>
>
> Le 04/03/2021 à 12:48, Christophe Leroy a écrit :
> >
> >
> > Le 04/03/2021 à 12:31, Marco Elver a écrit :
> >> On Thu, 4 Mar 2021 at 12:23, Christophe Leroy
> >> <christophe.leroy@...roup.eu> wrote:
> >>> Le 03/03/2021 à 11:56, Marco Elver a écrit :
> >>>>
> >>>> Somewhat tangentially, I also note that e.g. show_regs(regs) (which
> >>>> was printed along the KFENCE report above) didn't include the top
> >>>> frame in the "Call Trace", so this assumption is definitely not
> >>>> isolated to KFENCE.
> >>>>
> >>>
> >>> Now, I have tested PPC64 (with the patch I sent yesterday to modify save_stack_trace_regs()
> >>> applied), and I get many failures. Any idea ?
> >>>
> >>> [ 17.653751][ T58] ==================================================================
> >>> [ 17.654379][ T58] BUG: KFENCE: invalid free in .kfence_guarded_free+0x2e4/0x530
> >>> [ 17.654379][ T58]
> >>> [ 17.654831][ T58] Invalid free of 0xc00000003c9c0000 (in kfence-#77):
> >>> [ 17.655358][ T58] .kfence_guarded_free+0x2e4/0x530
> >>> [ 17.655775][ T58] .__slab_free+0x320/0x5a0
> >>> [ 17.656039][ T58] .test_double_free+0xe0/0x198
> >>> [ 17.656308][ T58] .kunit_try_run_case+0x80/0x110
> >>> [ 17.656523][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> >>> [ 17.657161][ T58] .kthread+0x18c/0x1a0
> >>> [ 17.659148][ T58] .ret_from_kernel_thread+0x58/0x70
> >>> [ 17.659869][ T58]
> >>> [ 17.663954][ T58] kfence-#77 [0xc00000003c9c0000-0xc00000003c9c001f, size=32, cache=kmalloc-32]
> >>> allocated by task 58:
> >>> [ 17.666113][ T58] .__kfence_alloc+0x1bc/0x510
> >>> [ 17.667069][ T58] .__kmalloc+0x280/0x4f0
> >>> [ 17.667452][ T58] .test_alloc+0x19c/0x430
> >>> [ 17.667732][ T58] .test_double_free+0x88/0x198
> >>> [ 17.667971][ T58] .kunit_try_run_case+0x80/0x110
> >>> [ 17.668283][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> >>> [ 17.668553][ T58] .kthread+0x18c/0x1a0
> >>> [ 17.669315][ T58] .ret_from_kernel_thread+0x58/0x70
> >>> [ 17.669711][ T58]
> >>> [ 17.669711][ T58] freed by task 58:
> >>> [ 17.670116][ T58] .kfence_guarded_free+0x3d0/0x530
> >>> [ 17.670421][ T58] .__slab_free+0x320/0x5a0
> >>> [ 17.670603][ T58] .test_double_free+0xb4/0x198
> >>> [ 17.670827][ T58] .kunit_try_run_case+0x80/0x110
> >>> [ 17.671073][ T58] .kunit_generic_run_threadfn_adapter+0x38/0x50
> >>> [ 17.671410][ T58] .kthread+0x18c/0x1a0
> >>> [ 17.671618][ T58] .ret_from_kernel_thread+0x58/0x70
> >>> [ 17.671972][ T58]
> >>> [ 17.672638][ T58] CPU: 0 PID: 58 Comm: kunit_try_catch Tainted: G B
> >>> 5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
> >>> [ 17.673768][ T58] ==================================================================
> >>> [ 17.677031][ T58] # test_double_free: EXPECTATION FAILED at mm/kfence/kfence_test.c:380
> >>> [ 17.677031][ T58] Expected report_matches(&expect) to be true, but is false
> >>> [ 17.684397][ T1] not ok 7 - test_double_free
> >>> [ 17.686463][ T59] # test_double_free-memcache: setup_test_cache: size=32, ctor=0x0
> >>> [ 17.688403][ T59] # test_double_free-memcache: test_alloc: size=32, gfp=cc0, policy=any,
> >>> cache=1
> >>
> >> Looks like something is prepending '.' to function names. We expect
> >> the function name to appear as-is, e.g. "kfence_guarded_free",
> >> "test_double_free", etc.
> >>
> >> Is there something special on ppc64, where the '.' is some convention?
> >>
> >
> > I think so, see https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
> >
> > Also see commit https://github.com/linuxppc/linux/commit/02424d896
> >
>
> But I'm wondering, if the dot is the problem, how so is the following one ok ?
>
> [ 79.574457][ T75] # test_krealloc: test_alloc: size=32, gfp=cc0, policy=any, cache=0
> [ 79.682728][ T75] ==================================================================
> [ 79.684017][ T75] BUG: KFENCE: use-after-free read in .test_krealloc+0x4fc/0x5b8
> [ 79.684017][ T75]
> [ 79.684955][ T75] Use-after-free read at 0xc00000003d060000 (in kfence-#130):
> [ 79.687581][ T75] .test_krealloc+0x4fc/0x5b8
> [ 79.688216][ T75] .test_krealloc+0x4e4/0x5b8
> [ 79.688824][ T75] .kunit_try_run_case+0x80/0x110
> [ 79.689737][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.690335][ T75] .kthread+0x18c/0x1a0
> [ 79.691092][ T75] .ret_from_kernel_thread+0x58/0x70
> [ 79.692081][ T75]
> [ 79.692671][ T75] kfence-#130 [0xc00000003d060000-0xc00000003d06001f, size=32,
> cache=kmalloc-32] allocated by task 75:
> [ 79.700977][ T75] .__kfence_alloc+0x1bc/0x510
> [ 79.701812][ T75] .__kmalloc+0x280/0x4f0
> [ 79.702695][ T75] .test_alloc+0x19c/0x430
> [ 79.703051][ T75] .test_krealloc+0xa8/0x5b8
> [ 79.703276][ T75] .kunit_try_run_case+0x80/0x110
> [ 79.703693][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.704223][ T75] .kthread+0x18c/0x1a0
> [ 79.704586][ T75] .ret_from_kernel_thread+0x58/0x70
> [ 79.704968][ T75]
> [ 79.704968][ T75] freed by task 75:
> [ 79.705756][ T75] .kfence_guarded_free+0x3d0/0x530
> [ 79.706754][ T75] .__slab_free+0x320/0x5a0
> [ 79.708575][ T75] .krealloc+0xe8/0x180
> [ 79.708970][ T75] .test_krealloc+0x1c8/0x5b8
> [ 79.709606][ T75] .kunit_try_run_case+0x80/0x110
> [ 79.710204][ T75] .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.710639][ T75] .kthread+0x18c/0x1a0
> [ 79.710996][ T75] .ret_from_kernel_thread+0x58/0x70
> [ 79.711349][ T75]
> [ 79.717435][ T75] CPU: 0 PID: 75 Comm: kunit_try_catch Tainted: G B
> 5.12.0-rc1-01540-g0783285cc1b8-dirty #4685
> [ 79.718124][ T75] NIP: c000000000468a40 LR: c000000000468a28 CTR: 0000000000000000
> [ 79.727741][ T75] REGS: c000000007dd3830 TRAP: 0300 Tainted: G B
> (5.12.0-rc1-01540-g0783285cc1b8-dirty)
> [ 79.733377][ T75] MSR: 8000000002009032 <SF,VEC,EE,ME,IR,DR,RI> CR: 28000440 XER: 00000000
> [ 79.738770][ T75] CFAR: c000000000888c7c DAR: c00000003d060000 DSISR: 40000000 IRQMASK: 0
> [ 79.738770][ T75] GPR00: c000000000468a28 c000000007dd3ad0 c000000001eaad00 c0000000073c3988
> [ 79.738770][ T75] GPR04: c000000007dd3b60 0000000000000001 0000000000000000 c00000003d060000
> [ 79.738770][ T75] GPR08: 00000000000002c8 0000000000000001 c0000000011bb410 c00000003fe903d8
> [ 79.738770][ T75] GPR12: 0000000028000440 c0000000020f0000 c0000000001a6460 c00000000724bb80
> [ 79.738770][ T75] GPR16: 0000000000000000 c00000000731749f c0000000011bb278 c00000000731749f
> [ 79.738770][ T75] GPR20: 00000001000002c1 0000000000000000 c0000000011bb278 c0000000011bb3b8
> [ 79.738770][ T75] GPR24: c0000000073174a0 c0000000011aa7b8 c000000001e35328 c00000000208ad00
> [ 79.738770][ T75] GPR28: 0000000000000000 c0000000011bb0b8 c0000000073c3988 c000000007dd3ad0
> [ 79.751744][ T75] NIP [c000000000468a40] .test_krealloc+0x4fc/0x5b8
> [ 79.752243][ T75] LR [c000000000468a28] .test_krealloc+0x4e4/0x5b8
> [ 79.752699][ T75] Call Trace:
> [ 79.753027][ T75] [c000000007dd3ad0] [c000000000468a28] .test_krealloc+0x4e4/0x5b8 (unreliable)
> [ 79.753878][ T75] [c000000007dd3c40] [c0000000008886d0] .kunit_try_run_case+0x80/0x110
> [ 79.754641][ T75] [c000000007dd3cd0] [c00000000088a808]
> .kunit_generic_run_threadfn_adapter+0x38/0x50
> [ 79.755494][ T75] [c000000007dd3d50] [c0000000001a65ec] .kthread+0x18c/0x1a0
> [ 79.757254][ T75] [c000000007dd3e10] [c00000000000dd68] .ret_from_kernel_thread+0x58/0x70
> [ 79.775521][ T75] Instruction dump:
> [ 79.776890][ T75] 68a50001 9b9f00c8 fbdf0090 fbbf00a0 fb5f00b8 484201cd 60000000 e8ff0080
> [ 79.783146][ T75] 3d42ff31 390002c8 394a0710 39200001 <88e70000> 38a00000 fb9f00a8 e8fbe80e
> [ 79.787563][ T75] ==================================================================
> [ 79.804667][ T1] ok 24 - test_krealloc
This one is using pt_regs, and therefore isn't trying to determine how
many entries we can skip in the stack trace to avoid showing
internals. I'll reply with a potential solution you can test shortly.
Thanks,
-- Marco
Powered by blists - more mailing lists