-
Notifications
You must be signed in to change notification settings - Fork 574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
i#1569 AArch64: Strip memory tags in vmareas/memquery #7347
base: master
Are you sure you want to change the base?
Conversation
Android 11+ uses the scudo heap allocator [1] which always returns tagged pointers if running on hardware that supports AArch64 TBI (Top Byte Ignore) or MTE (Memory Tagging Extension). App pointers that were obtained from malloc or new will contain a tag in the top byte of the pointer. In most situations this doesn't matter to DynamoRIO, its just part of the pointer value. However, memory tags are assigned in userspace by the allocator. They are not stored in the process's page tables (they would be too granular for that) and addresses in the process's memory map do not contain tags. When DynamoRIO looks up an `app_pc` in vmareas or memquery we need to strip the tag out so it can be compared to the untagged VAs in the memory map. This was discovered when testing on AArch64 Android but the same problem happens with Linux systems using a tagging allocator. I have reproduced it on Linux with glibc with the env var `GLIBC_TUNABLES=glibc.mem.tagging=3` set [2]. This fixes asserts seen running the test linux.bad-signal-stack: dynamorio/core/unix/memcache.c:382 found CURIOSITY : !exists || vmvector_overlap(all_memory_areas, start, end) || are_dynamo_vm_areas_stale() || !dynamo_initialized in file dynamorio/core/unix/memcache.c line 340 [1] https://source.android.com/docs/security/test/scudo [2] https://www.gnu.org/software/libc/manual/html_node/Memory-Related-Tunables.html Issue: #1569, #2154
@@ -335,6 +335,7 @@ bool | |||
memquery_from_os(const byte *pc, DR_PARAM_OUT dr_mem_info_t *info, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In most situations this doesn't matter to DynamoRIO, its just part of the pointer value.
grammar: it's
@@ -335,6 +335,7 @@ bool | |||
memquery_from_os(const byte *pc, DR_PARAM_OUT dr_mem_info_t *info, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In most situations this doesn't matter to DynamoRIO, its just part of the pointer value.
If you could elaborate on this in this description: the tags are included in application addresses used for code cache tags and in indirect branch lookup tables? Please explicitly state that policy to be 100% clear.
Xref the pointer authentication bits: which DR strips out on Mac in PR #5610 via XPACI when mangling, but these I believe are limited to the new PAC branch opcodes.
#if defined(AARCH64) && defined(LINUX) | ||
/* Tagged pointers (whether based on TBI or MTE) use the top byte of the pointer as a | ||
* memory tag. AArch64 has 48/52-bit VAs with the upper bits set to 0 for userspace | ||
* addresses so we should just be able to mask the tag out to get an untagged address. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add that we store the tagged version as the address for code cache purposes, but may need to remove the tag to compare to kernel-exported maps files.
@@ -335,6 +335,7 @@ bool | |||
memquery_from_os(const byte *pc, DR_PARAM_OUT dr_mem_info_t *info, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
App pointers that were obtained from malloc or new will contain a tag in the top byte of the pointer. In most situations this doesn't matter to DynamoRIO
This breaks drmemtrace's assumptions about canonical pointers: line 916 of trace_entry.h: // We assume that a 64-bit address ... top bits 48..63 are always identical.
Seems best to file a new issue on handling non-canonical addresses, use that for this PR, and have it cover drmemtrace and whatever other corner cases come up later.
@@ -335,6 +335,7 @@ bool | |||
memquery_from_os(const byte *pc, DR_PARAM_OUT dr_mem_info_t *info, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want to leave the tags in place? Will that lead to aliasing problems where the same lower 48 bits should go to the same address, but DR treats them as different? Do we want to instead strip all the tags, just like we strip LSB=1 in Thumb mode, in canonicalize_pc_target
?
@@ -917,6 +917,8 @@ add_vm_area(vm_area_vector_t *v, app_pc start, app_pc end, uint vm_flags, uint f | |||
void *data _IF_DEBUG(const char *comment)) | |||
{ | |||
int i, j, diff; | |||
start = STRIP_MEMORY_TAG(start); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does an app PC get here: do you have a sample callstack?
Android 11+ uses the scudo heap allocator [1] which always returns tagged pointers if running on hardware that supports AArch64 TBI (Top Byte Ignore) or MTE (Memory Tagging Extension). App pointers that were obtained from malloc or new will contain a tag in the top byte of the pointer. In most situations this doesn't matter to DynamoRIO, its just part of the pointer value.
However, memory tags are assigned in userspace by the allocator. They are not stored in the process's page tables (they would be too granular for that) and addresses in the process's memory map do not contain tags. When DynamoRIO looks up an
app_pc
in vmareas or memquery we need to strip the tag out so it can be compared to the untagged VAs in the memory map.This was discovered when testing on AArch64 Android but the same problem happens with Linux systems using a tagging allocator. I have reproduced it on Linux with glibc with the env var
GLIBC_TUNABLES=glibc.mem.tagging=3
set [2].This fixes asserts seen running the test linux.bad-signal-stack:
dynamorio/core/unix/memcache.c:382 found
CURIOSITY : !exists || vmvector_overlap(all_memory_areas, start, end) ||
are_dynamo_vm_areas_stale() || !dynamo_initialized in file
dynamorio/core/unix/memcache.c line 340
[1] https://source.android.com/docs/security/test/scudo
[2] https://www.gnu.org/software/libc/manual/html_node/Memory-Related-Tunables.html
Issue: #1569, #2154