[PATCH] arm64: restore get_current() optimisation

[mark.rutland@arm.com (Mark Rutland)]

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.