-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[MTE] Add an attribute to opt-in memory tagging of global variables while using fsanitize=memtag-globals (#166380) #168535
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
base: main
Are you sure you want to change the base?
Conversation
…hile using fsanitize=memtag-globals (llvm#166380)
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang Author: Tarcísio Fischer (tarcisiofischer) ChangesThe current behaviour is that any global variable inside sections is not MTE tagged, and (to the best of my knowledge) there's no way to opt-in. This PR proposes a way to opt-in. One example of where this could potentially be useful is the ProtectedMemory class on Chromium, which adds all the "protected" variables inside a single section to control it's access (https://source.chromium.org/chromium/chromium/src/+/main:base/memory/protected_memory.h;l=157;drc=299864be12719dba5dee143363b6aaa150cb96f8). Full diff: https://github.com/llvm/llvm-project/pull/168535.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 8dfe4bc08c48e..10a3c95f56f43 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3908,6 +3908,12 @@ def X86ForceAlignArgPointer : InheritableAttr, TargetSpecificAttr<TargetAnyX86>
let Documentation = [X86ForceAlignArgPointerDocs];
}
+def SectionMemtag : InheritableAttr {
+ let Spellings = [ClangGCC<"section_memtag">];
+ let Subjects = SubjectList<[GlobalVar]>;
+ let Documentation = [SectionMemtagDocs];
+}
+
def NoSanitize : InheritableAttr {
let Spellings = [ClangGCC<"no_sanitize">];
let Args = [VariadicStringArgument<"Sanitizers">];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 4813191d2d602..678789c2d0b9c 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3655,6 +3655,16 @@ full list of supported sanitizer flags.
}];
}
+def SectionMemtagDocs : Documentation {
+ let Category = DocCatVariable;
+ let Content = [{
+Use the ``section_memtag`` attribute on a global variable declaration that also
+has the attribute section and the memory tag sanitizer is active to force the
+global variable to be MTE tagged. Global variables under sections are not
+tagged by default, so you need to explicitly opt-in using this attribute.
+ }];
+}
+
def DisableSanitizerInstrumentationDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 3eeb1718e455a..7228de8637abb 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3823,6 +3823,10 @@ bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind,
return false;
}
+bool CodeGenModule::userForcedSectionMemtag(llvm::GlobalVariable *GV) const {
+ return GV->getMetadata("section_memtag") != nullptr;
+}
+
bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc,
StringRef Category) const {
const auto &XRayFilter = getContext().getXRayFilter();
@@ -6141,6 +6145,9 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
if (NeedsGlobalCtor || NeedsGlobalDtor)
EmitCXXGlobalVarDeclInitFunc(D, GV, NeedsGlobalCtor);
+ if (D->hasAttr<SectionMemtagAttr>()) {
+ GV->setMetadata("section_memtag", llvm::MDNode::get(GV->getContext(), {}));
+ }
SanitizerMD->reportGlobal(GV, *D, NeedsGlobalCtor);
// Emit global variable debug information.
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 2acfc83338a0c..f6ac61b6ce0cf 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1520,6 +1520,8 @@ class CodeGenModule : public CodeGenTypeCache {
SourceLocation Loc, QualType Ty,
StringRef Category = StringRef()) const;
+ bool userForcedSectionMemtag(llvm::GlobalVariable *GV) const;
+
/// Imbue XRay attributes to a function, applying the always/never attribute
/// lists in the process. Returns true if we did imbue attributes this way,
/// false otherwise.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a9e7b44ac9d73..9c5e289fa44b6 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6455,6 +6455,10 @@ static bool isSanitizerAttributeAllowedOnGlobals(StringRef Sanitizer) {
Sanitizer == "memtag";
}
+static void handleSectionMemtagAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ D->addAttr(SectionMemtagAttr::CreateImplicit(S.Context, AL));
+}
+
static void handleNoSanitizeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (!AL.checkAtLeastNumArgs(S, 1))
return;
@@ -7650,6 +7654,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_PtGuardedVar:
handlePtGuardedVarAttr(S, D, AL);
break;
+ case ParsedAttr::AT_SectionMemtag:
+ handleSectionMemtagAttr(S, D, AL);
+ break;
case ParsedAttr::AT_NoSanitize:
handleNoSanitizeAttr(S, D, AL);
break;
diff --git a/clang/test/CodeGen/memtag-globals.cpp b/clang/test/CodeGen/memtag-globals.cpp
index 407eea1c37cfa..74475b5e9a483 100644
--- a/clang/test/CodeGen/memtag-globals.cpp
+++ b/clang/test/CodeGen/memtag-globals.cpp
@@ -11,6 +11,7 @@
int global;
int __attribute__((__section__("my_section"))) section_global;
+int __attribute__((__section__("my_section"))) __attribute__((section_memtag)) section_global_tagged;
int __attribute__((no_sanitize("memtag"))) attributed_global;
int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
int ignorelisted_global;
@@ -28,6 +29,11 @@ void func() {
// This DOES NOT mean we are instrumenting the section global,
// but we are ignoring it in AsmPrinter rather than in clang.
// CHECK: @{{.*}}section_global{{.*}} ={{.*}} sanitize_memtag
+
+// In order to opt-in memory tagging in globals in sections,
+// __attribute__((section_memtag)) must be used.
+// CHECK: @{{.*}}section_global_tagged{{.*}} ={{.*}} sanitize_memtag,{{.*}} !section_memtag
+
// CHECK: @{{.*}}attributed_global{{.*}} =
// CHECK-NOT: sanitize_memtag
// CHECK: @{{.*}}disable_instrumentation_global{{.*}} =
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 3aa245b7f3f1e..e5d863afcc838 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -2633,7 +2633,8 @@ static bool shouldTagGlobal(const llvm::GlobalVariable &G) {
// To mitigate both these cases, and because specifying a section is rare
// outside of these two cases, disable MTE protection for globals in any
// section.
- if (G.hasSection())
+ bool ForceSectionMemtag = G.getMetadata("section_memtag") != nullptr;
+ if (G.hasSection() && !ForceSectionMemtag)
return false;
return globalSize(G) > 0;
|
|
@llvm/pr-subscribers-clang-codegen Author: Tarcísio Fischer (tarcisiofischer) ChangesThe current behaviour is that any global variable inside sections is not MTE tagged, and (to the best of my knowledge) there's no way to opt-in. This PR proposes a way to opt-in. One example of where this could potentially be useful is the ProtectedMemory class on Chromium, which adds all the "protected" variables inside a single section to control it's access (https://source.chromium.org/chromium/chromium/src/+/main:base/memory/protected_memory.h;l=157;drc=299864be12719dba5dee143363b6aaa150cb96f8). Full diff: https://github.com/llvm/llvm-project/pull/168535.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 8dfe4bc08c48e..10a3c95f56f43 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3908,6 +3908,12 @@ def X86ForceAlignArgPointer : InheritableAttr, TargetSpecificAttr<TargetAnyX86>
let Documentation = [X86ForceAlignArgPointerDocs];
}
+def SectionMemtag : InheritableAttr {
+ let Spellings = [ClangGCC<"section_memtag">];
+ let Subjects = SubjectList<[GlobalVar]>;
+ let Documentation = [SectionMemtagDocs];
+}
+
def NoSanitize : InheritableAttr {
let Spellings = [ClangGCC<"no_sanitize">];
let Args = [VariadicStringArgument<"Sanitizers">];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 4813191d2d602..678789c2d0b9c 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3655,6 +3655,16 @@ full list of supported sanitizer flags.
}];
}
+def SectionMemtagDocs : Documentation {
+ let Category = DocCatVariable;
+ let Content = [{
+Use the ``section_memtag`` attribute on a global variable declaration that also
+has the attribute section and the memory tag sanitizer is active to force the
+global variable to be MTE tagged. Global variables under sections are not
+tagged by default, so you need to explicitly opt-in using this attribute.
+ }];
+}
+
def DisableSanitizerInstrumentationDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 3eeb1718e455a..7228de8637abb 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3823,6 +3823,10 @@ bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind,
return false;
}
+bool CodeGenModule::userForcedSectionMemtag(llvm::GlobalVariable *GV) const {
+ return GV->getMetadata("section_memtag") != nullptr;
+}
+
bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc,
StringRef Category) const {
const auto &XRayFilter = getContext().getXRayFilter();
@@ -6141,6 +6145,9 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
if (NeedsGlobalCtor || NeedsGlobalDtor)
EmitCXXGlobalVarDeclInitFunc(D, GV, NeedsGlobalCtor);
+ if (D->hasAttr<SectionMemtagAttr>()) {
+ GV->setMetadata("section_memtag", llvm::MDNode::get(GV->getContext(), {}));
+ }
SanitizerMD->reportGlobal(GV, *D, NeedsGlobalCtor);
// Emit global variable debug information.
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 2acfc83338a0c..f6ac61b6ce0cf 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1520,6 +1520,8 @@ class CodeGenModule : public CodeGenTypeCache {
SourceLocation Loc, QualType Ty,
StringRef Category = StringRef()) const;
+ bool userForcedSectionMemtag(llvm::GlobalVariable *GV) const;
+
/// Imbue XRay attributes to a function, applying the always/never attribute
/// lists in the process. Returns true if we did imbue attributes this way,
/// false otherwise.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a9e7b44ac9d73..9c5e289fa44b6 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6455,6 +6455,10 @@ static bool isSanitizerAttributeAllowedOnGlobals(StringRef Sanitizer) {
Sanitizer == "memtag";
}
+static void handleSectionMemtagAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ D->addAttr(SectionMemtagAttr::CreateImplicit(S.Context, AL));
+}
+
static void handleNoSanitizeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (!AL.checkAtLeastNumArgs(S, 1))
return;
@@ -7650,6 +7654,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_PtGuardedVar:
handlePtGuardedVarAttr(S, D, AL);
break;
+ case ParsedAttr::AT_SectionMemtag:
+ handleSectionMemtagAttr(S, D, AL);
+ break;
case ParsedAttr::AT_NoSanitize:
handleNoSanitizeAttr(S, D, AL);
break;
diff --git a/clang/test/CodeGen/memtag-globals.cpp b/clang/test/CodeGen/memtag-globals.cpp
index 407eea1c37cfa..74475b5e9a483 100644
--- a/clang/test/CodeGen/memtag-globals.cpp
+++ b/clang/test/CodeGen/memtag-globals.cpp
@@ -11,6 +11,7 @@
int global;
int __attribute__((__section__("my_section"))) section_global;
+int __attribute__((__section__("my_section"))) __attribute__((section_memtag)) section_global_tagged;
int __attribute__((no_sanitize("memtag"))) attributed_global;
int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
int ignorelisted_global;
@@ -28,6 +29,11 @@ void func() {
// This DOES NOT mean we are instrumenting the section global,
// but we are ignoring it in AsmPrinter rather than in clang.
// CHECK: @{{.*}}section_global{{.*}} ={{.*}} sanitize_memtag
+
+// In order to opt-in memory tagging in globals in sections,
+// __attribute__((section_memtag)) must be used.
+// CHECK: @{{.*}}section_global_tagged{{.*}} ={{.*}} sanitize_memtag,{{.*}} !section_memtag
+
// CHECK: @{{.*}}attributed_global{{.*}} =
// CHECK-NOT: sanitize_memtag
// CHECK: @{{.*}}disable_instrumentation_global{{.*}} =
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 3aa245b7f3f1e..e5d863afcc838 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -2633,7 +2633,8 @@ static bool shouldTagGlobal(const llvm::GlobalVariable &G) {
// To mitigate both these cases, and because specifying a section is rare
// outside of these two cases, disable MTE protection for globals in any
// section.
- if (G.hasSection())
+ bool ForceSectionMemtag = G.getMetadata("section_memtag") != nullptr;
+ if (G.hasSection() && !ForceSectionMemtag)
return false;
return globalSize(G) > 0;
|
|
| } | ||
|
|
||
| def SectionMemtag : InheritableAttr { | ||
| let Spellings = [ClangGCC<"section_memtag">]; |
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.
maybe force_memtag? I don't think section_memtag is very meaningful if you don't know the context
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.
OTOH, force_memtag may sound a bit too generic. Perhaps force_memtag_section or enable_memtag_section?
(If you still prefer force_memtag, that's fine)
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.
We could reuse force_memtag to e.g. force the stack instrumentation to not use stack safety analysis for a particular local variable and always tag it with MTE.
| // but we are ignoring it in AsmPrinter rather than in clang. | ||
| // CHECK: @{{.*}}section_global{{.*}} ={{.*}} sanitize_memtag | ||
|
|
||
| // In order to opt-in memory tagging in globals in sections, |
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.
maybe also add to memtag-globals-asm.cpp?
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) ClangClang.Misc/pragma-attribute-supported-attributes-list.testIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
The current behaviour is that any global variable inside sections is not MTE tagged, and (to the best of my knowledge) there's no way to opt-in. This PR proposes a way to opt-in.
One example of where this could potentially be useful is the ProtectedMemory class on Chromium, which adds all the "protected" variables inside a single section to control it's access (https://source.chromium.org/chromium/chromium/src/+/main:base/memory/protected_memory.h;l=157;drc=299864be12719dba5dee143363b6aaa150cb96f8).