[PATCH] arm64: restore get_current() optimisation
Mark Rutland
mark.rutland at arm.com
Thu Mar 2 09:11:01 PST 2017
On Thu, Mar 02, 2017 at 04:12:08PM +0000, Robin Murphy wrote:
> On 02/03/17 15:30, Jon Hunter wrote:
> > On 02/03/17 12:35, Mark Rutland wrote:
> >> On Thu, Mar 02, 2017 at 11:35:06AM +0000, Jon Hunter wrote:
> >>> [ 184.523390] Unable to handle kernel paging request at virtual address ffff8001bb7a2800
>
> Notably, this is x4 + x23, where I'd bet on x4 being the address of
> "cache", and x23 being the index, except that apparently the top half of
> a pointer has somehow got in there instead - the stack contents at b9c8
> and b9e8 also stand out in that regard.
Indeed.
This could be indicative of an uninitialise reg value being passed in
from above, assuming map->reg_stride_order is 0.
> I'm wondering if the removal of volatile means we get some stack
> access hoisted before an earlier swizzling of current, the effect of
> which only makes itself known way down the line.
I don't see how that can happen, as current is never swizzled from the
PoV of the thread.
We switch it in assembly, in cpu_switch_to(), along with the other regs.
It's also initialsied in assembly, so at no point should C code be able
to observe a stale value.
> The KASAN version below is also interesting in that the
> reasonable-looking duff address is x0 + x1, but neither of those looks
> like anything sane on their own.
This is just an edge-case of KASAN. Anything that's outside of a mapped
area can also fall outside of the mapped shadow for that area.
> >>> [ 184.582802] sp : ffff8000b964b970
> >>> [ 184.586108] x29: ffff8000b964b970 x28: ffff8000b9584800
> >>> [ 184.591412] x27: ffff8000b964bcc8 x26: ffff8000b9461000
> >>> [ 184.596716] x25: 0000000000000000 x24: 0000000000000000
> >>> [ 184.602019] x23: 00000000ffff8000 x22: ffff8000b964ba1c
> >>> [ 184.607322] x21: ffff8000b964ba1c x20: 00000000ffff8000
> >>> [ 184.612626] x19: ffff8000bb7dc400 x18: 0000000000000000
> >>> [ 184.617928] x17: 0000000000000001 x16: ffff0000081f79e8
> >>> [ 184.623230] x15: 0000000000497000 x14: 0000000000000000
> >>> [ 184.628532] x13: 0000000000000001 x12: 0000000005cc6000
> >>> [ 184.633835] x11: 0000000000000000 x10: ffff8000bc16bf00
> >>> [ 184.639138] x9 : 0000000000000000 x8 : 0000000000000000
> >>> [ 184.644441] x7 : ffff8000bff68908 x6 : 0000000000000000
> >>> [ 184.649742] x5 : ffff000008fc9f00 x4 : ffff8000bb7aa800
> >>> [ 184.655044] x3 : 0000000000000002 x2 : ffff8000b964ba1c
> >>> [ 184.660347] x1 : 000000003fffe000 x0 : 0000000000000000
> >> If the commit in question is resulting in get_current() behaving differently,
> >> it *might* be possible to detect with the hack below. I haven't seen it blow up
> >> on my test systems.
> >
> > Unfortunately, that did not catch it :-(
Just to check, did it still blow up with that patch applied, or did it
function without any warning?
Thanks,
Mark.
More information about the linux-arm-kernel
mailing list