[llvm-dev] [compiler-rt] Use of ESR context in AArch64 sigframe

[Evgenii Stepanov via llvm-dev]

On Thu, Mar 8, 2018 at 9:11 AM, Will Deacon <will.deacon at arm.com> wrote:
> Thanks for the replies. Some comments below.
>
> On Wed, Mar 07, 2018 at 11:40:41AM -0800, Evgenii Stepanov wrote:
>> On Wed, Mar 7, 2018 at 7:48 AM, Dmitry Vyukov <dvyukov at google.com> wrote:
>> > On Wed, Mar 7, 2018 at 4:39 PM, Andrey Ryabinin <aryabinin at virtuozzo.com> wrote:
>> >> On 03/06/2018 08:58 PM, Will Deacon wrote:
>> >>> Hi all,
>> >>>
>> >>> As part of some recent work to harden the Kernel Address Space Layout
>> >>> Randomisation (KASLR) implementation in arm64 Linux, I've proposed a
>> >>> patch for the kernel which omits the ESR context from the signal frame
>> >>> if the faulting virtual address is outside the range of addresses which
>> >>> can be mapped by userspace.
>> >>>
>> >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/563837.html
>> >>>
>> >>> Looking around, it seems that AddressSanitizer is using this information
>> >>> in compiler-rt in order to distinguish the faulting access type between
>> >>> READ, WRITE or UNKNOWN. With this change, all attempted accesses to kernel
>> >>> memory from userspace will be reported as UNKNOWN.
>> >>>
>> >>> Is this likely to cause a problem?
>> >>
>> >> I guess this shouldn't be a bid deal.
>> >> AFAICS compiler-rt uses this information only in diagnostic message.
>> >
>> > +address-sanitizer mailing list
>> >
>> > Hi,
>> >
>> > These diagnostic messages are then parsed and analyzed, and access
>> > type is used at least during automatic security pre-assessment. Being
>> > capable to read arbitrary memory is different from being able to write
>> > arbitrary memory. Though, I don't know how we treat UNKNOWN. If it's
>> > the same as WRITE, then it's probably fine.
>>
>> This would disable read/write detection for almost all wild pointer
>> dereferences, since they are more than likely to have a 1 in one of
>> the higher order bits. This is undesirable, but not the end of the
>> world. Is it possible to sanitize ESR instead of omitting it
>> altogether?
>
> What do you do with the read/write information for a totally wild pointer
> like this?
>
> Sanitising is an option on the table -- we could fake up something like a
> level 0 translation fault, I'm just wary of faking things up if we could
> avoid the problem altogether (hence this email).
>
> It's also worth noting that the WnR bit needs special treatment for things
> like atomic instructions and cache maintenance instructions which currently
> appears to be missing. Would it be to bad to look at si_addr to spot kernel
> addresses and handle them specially?

We use this for preliminary classification of bugs, on the assumption
that bad writes are scarier than bad reads. We don't care about
si_addr permissions, what matters is the type of access requested by
the faulting instruction, i.e. PC at the time of the fault. Worst
case, we could disassemble the instruction to find out if its a read
or a write, and on arm64 it is not even that hard.

>
>> Also, your patch looks like it would disable ESR context for all
>> faults on tagged addresses, too. Since Linux unconditionally enables
>> TBI, it should be safe to zero out the MSB before the address check.
>
> The tag is currently removed as part of the early exception entry code,
> so it won't propagate this far.

That's good to know.