Re: [PATCH] [AArch64] Sanitize the address before working with allocation tags

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 5/20/21 5:38 AM, Alan Hayward wrote:
> 
> 
>> On 18 May 2021, at 22:20, Luis Machado via Gdb-patches <gdb-patches@sourceware.org> wrote:
>>
>> On 5/18/21 5:33 PM, Simon Marchi wrote:
>>> On 2021-05-18 4:19 p.m., Luis Machado via Gdb-patches wrote:
>>>> Remove the logical tag/top byte from the address whenever we have to work with
>>>> allocation tags.
>>> Can you explain a bit more why this is needed?  What down the line
>>> doesn't like to receive an address with a logical tag?
>>
>> We shouldn't be passing an address with a non-zero top byte (or tag) to a ptrace request, for example. It may work (in fact, it works) but we are not supposed to rely on it. So we sanitize the pointer before it gets to fetch_memtags/store_memtags.
>>
>> This is clarified in the AArch64 Tagged Address ABI document (https://www.kernel.org/doc/html/latest/arm64/tagged-address-abi.html).
>>
>> In an upcoming patch to support memory tags in core files (https://sourceware.org/pipermail/gdb-patches/2021-May/178973.html), this address also gets passed down to the core target's fetch_memtags implementation. It needs to compare addresses, so it doesn't make sense to let through an address with a non-zero top byte, or else we risk not having a match due to differences in the upper byte.
>>
> 
> 
> Would it make sense to put the address_significant() at the beginning of aarch64_mte_get_atag()?
> That’d ensure any future code that calls aarch64_mte_get_atag() is safe too. And it would mean the higher functions are dealing with a single address throughout.

Yes and no. I didn't want to pollute aarch64_mte_get_atag by passing 
gdbarch just to be able to call address_significant. Alternatively, we 
could just remove the top byte by hand without using 
address_significantm, since we're within the AArch64 target anyway.

> 
> Alternatively, it could even move down into target_fetch_memtags() instead (same with target_store_memtags), but I’m less keen on that.

Yes. I considered that, but all the implementations would have to 
sanitize the address. We don't know what kinds of targets will want to 
implement those functions, so we can't safely assume they want to strip 
the top bytes. And, again, we'd need to pass/fetch the gdbarch from 
within the native layers just so we can call address_significant.

> 
> 
> Alan.
>