From ed5a349b89e9ccc1c3dbe427de27d28e145f8203 Mon Sep 17 00:00:00 2001
From: Mitch Phillips <31459023+hctim@users.noreply.github.com>
Date: Thu, 16 Jun 2022 14:27:38 -0700
Subject: [PATCH] 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.
---
 llvm/include/llvm/IR/GlobalValue.h | 6 +++++-
 llvm/lib/IR/Globals.cpp            | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 0f901e0..a17423d 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -324,7 +324,11 @@ public:
 
   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 d2ddfd4..9f1ed1d 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;
 }
-- 
2.7.4