[llvm] ed5a349 - Make setSanitizerMetadata byval.

[Mitch Phillips via llvm-commits]

Author: Mitch Phillips
Date: 2022-06-16T14:47:27-07:00
New Revision: ed5a349b89e9ccc1c3dbe427de27d28e145f8203

URL: https://github.com/llvm/llvm-project/commit/ed5a349b89e9ccc1c3dbe427de27d28e145f8203
DIFF: https://github.com/llvm/llvm-project/commit/ed5a349b89e9ccc1c3dbe427de27d28e145f8203.diff

LOG: Make setSanitizerMetadata byval.

This fixes a UaF bug in llvm::GlobalObject::copyAttributesFrom, where a
sanitizer metadata object is captured by reference, and passed by
reference to llvm::GlobalValue::setSanitizerMetadata. The reference
comes from the same map that the new value is going to be inserted to,
and the map insertion triggers iterator invalidation - leading to a
use-after-free on the dangling reference.

This patch fixes that bug by making setSanitizerMetadata's argument
byval. This should also systematically prevent the problem from
happening in future, as it's a very easy pattern to have. This shouldn't
be any performance problem, the SanitizerMetadata struct is a bitfield
POD.

Added: 
    

Modified: 
    llvm/include/llvm/IR/GlobalValue.h
    llvm/lib/IR/Globals.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 0f901e0fca506..a17423dd965b7 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -324,7 +324,11 @@ class GlobalValue : public Constant {
 
   bool hasSanitizerMetadata() const { return HasSanitizerMetadata; }
   const SanitizerMetadata &getSanitizerMetadata() const;
-  void setSanitizerMetadata(const SanitizerMetadata &Meta);
+  // Note: Not byref as it's a POD and otherwise it's too easy to call
+  // G.setSanitizerMetadata(G2.getSanitizerMetadata()), and the argument becomes
+  // dangling when the backing storage allocates the metadata for `G`, as the
+  // storage is shared between `G1` and `G2`.
+  void setSanitizerMetadata(SanitizerMetadata Meta);
   void removeSanitizerMetadata();
 
   static LinkageTypes getLinkOnceLinkage(bool ODR) {

diff  --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index d2ddfd4d8a1e1..9f1ed1d4b120b 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -228,7 +228,7 @@ const SanitizerMetadata &GlobalValue::getSanitizerMetadata() const {
   return getContext().pImpl->GlobalValueSanitizerMetadata[this];
 }
 
-void GlobalValue::setSanitizerMetadata(const SanitizerMetadata &Meta) {
+void GlobalValue::setSanitizerMetadata(SanitizerMetadata Meta) {
   getContext().pImpl->GlobalValueSanitizerMetadata[this] = Meta;
   HasSanitizerMetadata = true;
 }