Skip to content

Commit

Permalink
Make setSanitizerMetadata byval.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hctim committed Jun 16, 2022
1 parent 72c1eff commit ed5a349
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
6 changes: 5 additions & 1 deletion llvm/include/llvm/IR/GlobalValue.h
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/IR/Globals.cpp
Expand Up @@ -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;
}
Expand Down

0 comments on commit ed5a349

Please sign in to comment.