[compiler-rt][scudo] Check for failing prctl call

[leonardchan]

Summary

A bunch of MTE tests like ./ScudoUnitTest-aarch64-Test/MemtagTest.StoreTags can fail on aarch64-linux if the kernel doesn't support TBI. It looks like the call to prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0) can return -1, which casted to an unsigned int and masked will return a value not equal to PR_MTE_TCF_NONE, meaning systemDetectsMemoryTagFaultsTestOnly can return an incorrect value.

This updates the check to account for a failing prctl call.

[pcc]

Please remember to upload patches with context: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

The tests in MemtagTests are meant to be skipped if the hardware doesn't support MTE (see the top of MemtagTest::SetUp()). Is that not working for you?

I don't think you should normally need to be defining SCUDO_DISABLE_TBI because it's controlled by the operating system, and all arm64 Linux systems that we normally care about support TBI. I figured that you were maybe trying to run these tests on Fuchsia, but then I noticed that memtag_test.cpp is wrapped in an #if SCUDO_LINUX, so I'm not sure why you're running into an issue. Are you trying to run these tests on arm64 non-Android Linux? Maybe the GTEST_SKIP() isn't working correctly for you for some reason?

compiler-rt/lib/scudo/standalone/memtag.h

21

Can we change this to:

#if (__clang_major__ >= 12 && defined(__aarch64__) && SCUDO_LINUX && !defined(SCUDO_DISABLE_TBI)) || defined(SCUDO_FUZZ)

and remove the #if on line 26 instead? (This is orthogonal to your issue though.)

[leonardchan]

In D110888#3035193, @pcc wrote:

Please remember to upload patches with context: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

The tests in MemtagTests are meant to be skipped if the hardware doesn't support MTE (see the top of MemtagTest::SetUp()). Is that not working for you?

I don't think you should normally need to be defining SCUDO_DISABLE_TBI because it's controlled by the operating system, and all arm64 Linux systems that we normally care about support TBI. I figured that you were maybe trying to run these tests on Fuchsia, but then I noticed that memtag_test.cpp is wrapped in an #if SCUDO_LINUX, so I'm not sure why you're running into an issue. Are you trying to run these tests on arm64 non-Android Linux? Maybe the GTEST_SKIP() isn't working correctly for you for some reason?

I'm attempting to run host-linux runtimes tests. We weren't running them before and this is the first time we're trying it. We just happen to run into these test failures when turning them on before. But I think I found out the underlying issue: prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0) (in systemDetectsMemoryTagFaultsTestOnly) can return -1 if TBI isn't supported according to https://man7.org/linux/man-pages/man2/prctl.2.html, and this seems to be the case for us. The -1 is cast to an unsigned long and masked, then systemDetectsMemoryTagFaultsTestOnly ends up returning true when comparing a non-zero value after masking. I think the proper fix might be to just account for the -1 result. I'll update the patch later.

[pcc]

In D110888#3035297, @leonardchan wrote:

In D110888#3035193, @pcc wrote:

Please remember to upload patches with context: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

The tests in MemtagTests are meant to be skipped if the hardware doesn't support MTE (see the top of MemtagTest::SetUp()). Is that not working for you?

I don't think you should normally need to be defining SCUDO_DISABLE_TBI because it's controlled by the operating system, and all arm64 Linux systems that we normally care about support TBI. I figured that you were maybe trying to run these tests on Fuchsia, but then I noticed that memtag_test.cpp is wrapped in an #if SCUDO_LINUX, so I'm not sure why you're running into an issue. Are you trying to run these tests on arm64 non-Android Linux? Maybe the GTEST_SKIP() isn't working correctly for you for some reason?

I'm attempting to run host-linux runtimes tests. We weren't running them before and this is the first time we're trying it. We just happen to run into these test failures when turning them on before. But I think I found out the underlying issue: prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0) (in systemDetectsMemoryTagFaultsTestOnly) can return -1 if TBI isn't supported according to https://man7.org/linux/man-pages/man2/prctl.2.html, and this seems to be the case for us. The -1 is cast to an unsigned long and masked, then systemDetectsMemoryTagFaultsTestOnly ends up returning true when comparing a non-zero value after masking. I think the proper fix might be to just account for the -1 result. I'll update the patch later.

Ah, that could explain it. I don't think the issue is that TBI isn't supported (it's always been supported on Linux going back to 2013 or so), it's that the tagged address ABI isn't supported in your kernel. The support for the tagged address ABI was first introduced in kernel version 5.4, but it was backported quite far back on Android , so I guess we never noticed this issue because we never ran the tests on a machine that didn't have them backported. Checking for the -1 return value seems like the right fix to me, thanks.

[leonardchan]

compiler-rt/lib/scudo/standalone/memtag.h

21

Made D111080 for this.

[pcc]

LGTM

s/TBI/the tagged address ABI/ in the commit message.

compiler-rt/lib/scudo/standalone/memtag.h

 
 #if SCUDO_LINUX
 #include <sys/auxv.h>
 #include <sys/prctl.h>
 #endif
 
 namespace scudo {
 
 #if (__clang_major__ >= 12 && defined(__aarch64__)) || defined(SCUDO_FUZZ)
 
 // We assume that Top-Byte Ignore is enabled if the architecture supports memory
 // tagging. Not all operating systems enable TBI, so we only claim architectural
 // support for memory tagging if the operating system enables TBI.
 #if SCUDO_LINUX && !defined(SCUDO_DISABLE_TBI)
 inline constexpr bool archSupportsMemoryTagging() { return true; }
 #else
 inline constexpr bool archSupportsMemoryTagging() { return false; }

 #define PR_MTE_TCF_NONE (0UL << PR_MTE_TCF_SHIFT)
 #endif
 #ifndef PR_MTE_TCF_SYNC
 #define PR_MTE_TCF_SYNC (1UL << PR_MTE_TCF_SHIFT)
 #endif
 #ifndef PR_MTE_TCF_MASK
 #define PR_MTE_TCF_MASK (3UL << PR_MTE_TCF_SHIFT)
 #endif
-  return (static_cast<unsigned long>(
-              prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0)) &
-          PR_MTE_TCF_MASK) != PR_MTE_TCF_NONE;
+  int res = prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0);
+  if (res == -1)
+    return false;
+  return (static_cast<unsigned long>(res) & PR_MTE_TCF_MASK) != PR_MTE_TCF_NONE;
 }
 
 inline void enableSystemMemoryTaggingTestOnly() {
   prctl(PR_SET_TAGGED_ADDR_CTRL,
         PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | (0xfffe << PR_MTE_TAG_SHIFT),
         0, 0, 0);
 }