This is an archive of the discontinued LLVM Phabricator instance.

Differential D51334

SafeStack: Prevent OOB reads with mem intrinsics
ClosedPublic

Authored by vlad.tsyrklevich on Aug 27 2018, 5:34 PM.

Details

Reviewers
eugenis
Commits
rG2499aeead93a: SafeStack: Prevent OOB reads with mem intrinsics
rL341116: SafeStack: Prevent OOB reads with mem intrinsics
Summary

Currently, the SafeStack analysis disallows out-of-bounds writes but not
out-of-bounds reads for mem intrinsics like llvm.memcpy. This could
cause leaks of pointers to the safe stack by leaking spilled registers/
frame pointers. Check for allocas used as source or destination pointers
to mem intrinsics.

Diff Detail

Event Timeline

vlad.tsyrklevich created this revision.Aug 27 2018, 5:34 PM
Herald added a subscriber: llvm-commits. · View Herald TranscriptAug 27 2018, 5:34 PM
Harbormaster completed remote builds in B21990: Diff 162782.Aug 27 2018, 5:34 PM
eugenis accepted this revision.Aug 30 2018, 11:22 AM

LGTM

This revision is now accepted and ready to land.Aug 30 2018, 11:22 AM
Closed by commit rL341116: SafeStack: Prevent OOB reads with mem intrinsics (authored by vlad.tsyrklevich). · Explain WhyAug 30 2018, 1:47 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lib/
CodeGen/
10 lines
test/
Transforms/
SafeStack/
X86/
14 lines

Diff 162782

lib/CodeGen/SafeStack.cpp

Show First 20 Lines • Show All 254 Lines • ▼ Show 20 Lines LLVM_DEBUG(
<< " " << (Safe ? "safe" : "unsafe") << "\n"); << " " << (Safe ? "safe" : "unsafe") << "\n");
return Safe; return Safe;
} }
bool SafeStack::IsMemIntrinsicSafe(const MemIntrinsic *MI, const Use &U, bool SafeStack::IsMemIntrinsicSafe(const MemIntrinsic *MI, const Use &U,
const Value *AllocaPtr, const Value *AllocaPtr,
uint64_t AllocaSize) { uint64_t AllocaSize) {
// All MemIntrinsics have destination address in Arg0 and size in Arg2. if (auto MTI = dyn_cast<MemTransferInst>(MI)) {
if (MI->getRawDest() != U) return true; if (MTI->getRawSource() != U && MTI->getRawDest() != U)
return true;
} else {
if (MI->getRawDest() != U)
return true;
}
const auto *Len = dyn_cast<ConstantInt>(MI->getLength()); const auto *Len = dyn_cast<ConstantInt>(MI->getLength());
// Non-constant size => unsafe. FIXME: try SCEV getRange. // Non-constant size => unsafe. FIXME: try SCEV getRange.
if (!Len) return false; if (!Len) return false;
return IsAccessSafe(U, Len->getZExtValue(), AllocaPtr, AllocaSize); return IsAccessSafe(U, Len->getZExtValue(), AllocaPtr, AllocaSize);
} }
/// Check whether a given allocation must be put on the safe /// Check whether a given allocation must be put on the safe
/// stack or not. The function analyzes all uses of AI and checks whether it is /// stack or not. The function analyzes all uses of AI and checks whether it is
▲ Show 20 Lines • Show All 631 Lines • Show Last 20 Lines

test/Transforms/SafeStack/X86/memintrinsic-oob-read.ll

  • This file was added.
; RUN: opt -safe-stack -S -mtriple=i386-pc-linux-gnu < %s -o - | FileCheck %s
; RUN: opt -safe-stack -S -mtriple=x86_64-pc-linux-gnu < %s -o - | FileCheck %s
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
; CHECK: __safestack_unsafe_stack_ptr
define void @oob_read(i8* %ptr) safestack {
%1 = alloca i8
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %ptr, i8* align 1 %1, i64 4, i1 false)
ret void
}