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

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?

Many thanks,

Will

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?

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.

Thanks for the replies. Some comments below.

>>> 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?

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.

Thanks,

Will

Thanks for the replies. Some comments below.

>>> 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.

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.

...but it's a bit irritating to do that when the hardware
provides that information to the kernel and up til now
the kernel has been providing that information to userspace.
The disassemble-the-insn approach also forces you into
continuous updates to keep up to date with new load and store
instructions that are added in new architecture revisions
(for instance the v8.1 limited-ordering load/stores and
the SVE extension loads and stores).

Write-not-Read information is useful, aarch32 signal handlers
provide it, this feature went into the aarch64 kernel specifically
to provide that WnR info, so I'd really prefer it if we keep it.
Mask out the bad stuff if it's in kernel space, sure, but I don't
see the reason to throw the baby out with the bathwater here.

From userspace's point of view, a fault for a read from the top

half of memory is no different from a fault for a read from
somewhere that doesn't have anything mapped at all, so it's
kind of weird for them to behave differently.

thanks
-- PMM