Skip to content
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

[sanitizer] Disallow external_symbolizer_path with AT_SECURE #92611

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented May 17, 2024

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented May 17, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Florian Mayer (fmayer)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/92611.diff

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp (+8-1)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
index 0ddc24802d216..68d51dd1cb9b7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
@@ -25,6 +25,7 @@
 #  include "sanitizer_common.h"
 #  include "sanitizer_file.h"
 #  include "sanitizer_flags.h"
+#  include "sanitizer_getauxval.h"
 #  include "sanitizer_internal_defs.h"
 #  include "sanitizer_linux.h"
 #  include "sanitizer_placement_new.h"
@@ -408,7 +409,13 @@ const char *Symbolizer::PlatformDemangle(const char *name) {
 
 static SymbolizerTool *ChooseExternalSymbolizer(LowLevelAllocator *allocator) {
   const char *path = common_flags()->external_symbolizer_path;
-
+  // This is so we can use the weak definition from sanitizer_getauxval.h
+  if (&getauxval && getauxval(/* AT_SECURE */ 23) != 0) {
+    Report(
+        "ERROR: external_symbolizer_path cannot be used for AT_SECURE "
+        "(e.g. setuid binaries).\n");
+    Die();
+  }
   if (path && internal_strchr(path, '%')) {
     char *new_path = (char *)InternalAlloc(kMaxPathLength);
     SubstituteForFlagValue(path, new_path, kMaxPathLength);

@fmayer
Copy link
Contributor Author

fmayer commented May 17, 2024

@bigb4ng FYI

@fmayer fmayer requested a review from thurstond May 17, 2024 22:15
@fmayer
Copy link
Contributor Author

fmayer commented May 17, 2024

@fmayer Cool. Friendly reminders:

  1. SANITIZER_GO is currently not affected by this patch due to

If ran on a system that has getauxval, it will also work for SANITIZER_GO, because of the weak declaration in the #else.

  1. log_path=stdout and log_path=stderr for AT_SECURE are not supported in this version.

This is about external_symbolizer_path :)

Trust your judgement on those. I will close #92593.

Wait, this addresses a different option than #92593. Please re-open your CL.

@fmayer
Copy link
Contributor Author

fmayer commented Aug 8, 2024

Seems like we don't want to implement this after all.

@fmayer fmayer closed this Aug 8, 2024
fmayer added a commit to fmayer/llvm-project that referenced this pull request Sep 13, 2024
fmayer pushed a commit that referenced this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants