[PATCH] D93731: scudo: Support memory tagging in the secondary allocator.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/fuchsia.cpp:140
+                         UNUSED MapPlatformData *Data) {
+  const zx_vm_option_t Prot =
+      (Flags & MAP_NOACCESS) ? 0 : (ZX_VM_PERM_READ | ZX_VM_PERM_WRITE);
----------------
cryptoad wrote:
> So this ended up not working for me, because the protection must have at least 1 flag (https://fuchsia.dev/fuchsia-src/reference/syscalls/vmar_protect#description)
> Did it work for you?
It seems to work for me. I don't think that documentation is accurate. I verified that this function is being called like so:
```
diff --git a/compiler-rt/lib/scudo/standalone/fuchsia.cpp b/compiler-rt/lib/scudo/standalone/fuchsia.cpp
index bf063e11bb3d..5e90373a7fc3 100644
--- a/compiler-rt/lib/scudo/standalone/fuchsia.cpp
+++ b/compiler-rt/lib/scudo/standalone/fuchsia.cpp
@@ -137,6 +137,8 @@ void unmap(void *Addr, uptr Size, uptr Flags, MapPlatformData *Data) {
 
 void setMemoryPermission(UNUSED uptr Addr, UNUSED uptr Size, UNUSED uptr Flags,
                          UNUSED MapPlatformData *Data) {
+  if (Flags & MAP_NOACCESS)
+    dieOnMapUnmapError();
   const zx_vm_option_t Prot =
       (Flags & MAP_NOACCESS) ? 0 : (ZX_VM_PERM_READ | ZX_VM_PERM_WRITE);
   DCHECK(Data);
```
With that I observed a crash in the unit tests (I don't think this code is ever actually called in production on Fuchsia because the Fuchsia allocator does not enable the cache).

I also made this change to verify that the permissions were being applied properly:
```
diff --git a/compiler-rt/lib/scudo/standalone/fuchsia.cpp b/compiler-rt/lib/scudo/standalone/fuchsia.cpp
index bf063e11bb3d..0d4f8d9bcddd 100644
--- a/compiler-rt/lib/scudo/standalone/fuchsia.cpp
+++ b/compiler-rt/lib/scudo/standalone/fuchsia.cpp
@@ -143,6 +143,10 @@ void setMemoryPermission(UNUSED uptr Addr, UNUSED uptr Size, UNUSED uptr Flags,
   DCHECK_NE(Data->Vmar, ZX_HANDLE_INVALID);
   if (_zx_vmar_protect(Data->Vmar, Prot, Addr, Size) != ZX_OK)
     dieOnMapUnmapError();
+  if (Flags & MAP_NOACCESS) {
+    volatile char C = *(char *)Addr;
+    (void)C;
+  }
 }
 
 void releasePagesToOS(UNUSED uptr BaseAddress, uptr Offset, uptr Size,
```
And again I observed a crash.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93731/new/

https://reviews.llvm.org/D93731