[wasm] [interpreter] Fix integer underflow in mem access

For OOB checks on memory accesses, we first subtracted the size of the
type to load/store from the memory size, and then compared against this
effective_size. If the memory size is smaller than the size of the type,
this would lead to an integer underflow, and we would try to load the
value.
This CL fixes this, and adds a test case for this.

R=ahaas@chromium.org
BUG=v8:5822

Change-Id: I26fcba0be7343c88b8459d029b0c0af095d2466a
Reviewed-on: https://chromium-review.googlesource.com/465946
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44345}
This commit is contained in:
Clemens Hammacher 2017-04-03 09:44:47 +02:00 committed by Commit Bot
parent dc662e5b74
commit d38334c575
2 changed files with 16 additions and 8 deletions

View File

@ -1456,13 +1456,17 @@ class ThreadImpl {
stack_.resize(stack_.size() - pop_count);
}
template <typename mtype>
inline bool BoundsCheck(uint32_t mem_size, uint32_t offset, uint32_t index) {
return sizeof(mtype) <= mem_size && offset <= mem_size - sizeof(mtype) &&
index <= mem_size - sizeof(mtype) - offset;
}
template <typename ctype, typename mtype>
bool ExecuteLoad(Decoder* decoder, InterpreterCode* code, pc_t pc, int& len) {
MemoryAccessOperand operand(decoder, code->at(pc), sizeof(ctype));
uint32_t index = Pop().to<uint32_t>();
size_t effective_mem_size = instance()->mem_size - sizeof(mtype);
if (operand.offset > effective_mem_size ||
index > (effective_mem_size - operand.offset)) {
if (!BoundsCheck<mtype>(instance()->mem_size, operand.offset, index)) {
DoTrap(kTrapMemOutOfBounds, pc);
return false;
}
@ -1481,9 +1485,7 @@ class ThreadImpl {
WasmVal val = Pop();
uint32_t index = Pop().to<uint32_t>();
size_t effective_mem_size = instance()->mem_size - sizeof(mtype);
if (operand.offset > effective_mem_size ||
index > (effective_mem_size - operand.offset)) {
if (!BoundsCheck<mtype>(instance()->mem_size, operand.offset, index)) {
DoTrap(kTrapMemOutOfBounds, pc);
return false;
}
@ -1844,7 +1846,7 @@ class ThreadImpl {
case kExpr##name: { \
uint32_t index = Pop().to<uint32_t>(); \
ctype result; \
if (index >= (instance()->mem_size - sizeof(mtype))) { \
if (!BoundsCheck<mtype>(instance()->mem_size, 0, index)) { \
result = defval; \
} else { \
byte* addr = instance()->mem_start + index; \
@ -1869,7 +1871,7 @@ class ThreadImpl {
case kExpr##name: { \
WasmVal val = Pop(); \
uint32_t index = Pop().to<uint32_t>(); \
if (index < (instance()->mem_size - sizeof(mtype))) { \
if (BoundsCheck<mtype>(instance()->mem_size, 0, index)) { \
byte* addr = instance()->mem_start + index; \
/* TODO(titzer): alignment for asmjs store mem? */ \
*(reinterpret_cast<mtype*>(addr)) = static_cast<mtype>(val.to<ctype>()); \

View File

@ -429,6 +429,12 @@ TEST(WasmInterpreterActivations) {
CHECK_EQ(0, thread->NumActivations());
}
TEST(InterpreterLoadWithoutMemory) {
WasmRunner<int32_t, int32_t> r(kExecuteInterpreted);
BUILD(r, WASM_LOAD_MEM(MachineType::Int32(), WASM_GET_LOCAL(0)));
CHECK_TRAP32(r.Call(0));
}
} // namespace wasm
} // namespace internal
} // namespace v8