Skip to content

Commit 951cdb3

Browse files
backesmibrunin
authored andcommitted
[Backport] Security bug 1201340
Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/2875210: Merged: [liftoff] Fix >=2GB memory accesses on 32-bit We were inconsistent in handling offsets >= 2GB on 32-bit systems. The code was still relying on this being detected as statically out of bounds, but with the increase of {kV8MaxWasmMemoryPages} to support 4GB memories, this is not the case any more. This CL fixes this by again detecting such situations as statically OOB. We do not expect to be able to allocate memories of size >2GB on such systems. If this assumptions turns out to be wrong, we will erroneously trap. If that happens, we will have to explicitly disallow memories of such size on 32-bit systems. Tbr: [email protected] (cherry picked from commit 7ad5b961553d7d9bc30da1bb839726be2b92bb51) Bug: v8:7881, chromium:1201340 Change-Id: I8a91dd067a1c63a6d1caacb874a27b44b0983774 No-Try: true No-Presubmit: true No-Tree-Checks: true Reviewed-by: Clemens Backes <[email protected]> Commit-Queue: Clemens Backes <[email protected]> Cr-Commit-Position: refs/branch-heads/9.0@{#51} Cr-Branched-From: bd0108b4c88e0d6f2350cb79b5f363fbd02f3eb7-refs/heads/9.0.257@{#1} Cr-Branched-From: 349bcc6a075411f1a7ce2d866c3dfeefc2efa39d-refs/heads/master@{#73001} Reviewed-by: Allan Sandfeld Jensen <[email protected]>
1 parent 9827f0c commit 951cdb3

File tree

4 files changed

+12
-27
lines changed

4 files changed

+12
-27
lines changed

chromium/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -689,13 +689,8 @@ void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr,
689689
Register offset_reg, uint32_t offset_imm,
690690
LoadType type, LiftoffRegList pinned,
691691
uint32_t* protected_load_pc, bool is_load_mem) {
692-
// If offset_imm cannot be converted to int32 safely, we abort as a separate
693-
// check should cause this code to never be executed.
694-
// TODO(7881): Support when >2GB is required.
695-
if (!is_uint31(offset_imm)) {
696-
TurboAssembler::Abort(AbortReason::kOffsetOutOfRange);
697-
return;
698-
}
692+
// Offsets >=2GB are statically OOB on 32-bit systems.
693+
DCHECK_LE(offset_imm, std::numeric_limits<int32_t>::max());
699694
liftoff::LoadInternal(this, dst, src_addr, offset_reg,
700695
static_cast<int32_t>(offset_imm), type, pinned,
701696
protected_load_pc, is_load_mem);
@@ -705,13 +700,8 @@ void LiftoffAssembler::Store(Register dst_addr, Register offset_reg,
705700
uint32_t offset_imm, LiftoffRegister src,
706701
StoreType type, LiftoffRegList pinned,
707702
uint32_t* protected_store_pc, bool is_store_mem) {
708-
// If offset_imm cannot be converted to int32 safely, we abort as a separate
709-
// check should cause this code to never be executed.
710-
// TODO(7881): Support when >2GB is required.
711-
if (!is_uint31(offset_imm)) {
712-
TurboAssembler::Abort(AbortReason::kOffsetOutOfRange);
713-
return;
714-
}
703+
// Offsets >=2GB are statically OOB on 32-bit systems.
704+
DCHECK_LE(offset_imm, std::numeric_limits<int32_t>::max());
715705
UseScratchRegisterScope temps(this);
716706
if (type.value() == StoreType::kF64Store) {
717707
Register actual_dst_addr = liftoff::CalculateActualAddress(

chromium/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -324,13 +324,8 @@ void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr,
324324
Register offset_reg, uint32_t offset_imm,
325325
LoadType type, LiftoffRegList pinned,
326326
uint32_t* protected_load_pc, bool is_load_mem) {
327-
if (offset_imm > static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) {
328-
// We do not generate code here, because such an offset should never pass
329-
// the bounds check. However, the spec requires us to compile code with such
330-
// an offset.
331-
Trap();
332-
return;
333-
}
327+
// Offsets >=2GB are statically OOB on 32-bit systems.
328+
DCHECK_LE(offset_imm, std::numeric_limits<int32_t>::max());
334329
DCHECK_EQ(type.value_type() == kWasmI64, dst.is_gp_pair());
335330
Operand src_op = offset_reg == no_reg
336331
? Operand(src_addr, offset_imm)
@@ -406,6 +401,7 @@ void LiftoffAssembler::Store(Register dst_addr, Register offset_reg,
406401
StoreType type, LiftoffRegList pinned,
407402
uint32_t* protected_store_pc, bool is_store_mem) {
408403
DCHECK_EQ(type.value_type() == kWasmI64, src.is_gp_pair());
404+
// Offsets >=2GB are statically OOB on 32-bit systems.
409405
DCHECK_LE(offset_imm, std::numeric_limits<int32_t>::max());
410406
Operand dst_op = offset_reg == no_reg
411407
? Operand(dst_addr, offset_imm)

chromium/v8/src/wasm/baseline/liftoff-compiler.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2100,10 +2100,7 @@ class LiftoffCompiler {
21002100
bool BoundsCheckMem(FullDecoder* decoder, uint32_t access_size,
21012101
uint64_t offset, Register index, LiftoffRegList pinned,
21022102
ForceCheck force_check) {
2103-
// If the offset does not fit in a uintptr_t, this can never succeed on this
2104-
// machine.
21052103
const bool statically_oob =
2106-
offset > std::numeric_limits<uintptr_t>::max() ||
21072104
!base::IsInBounds<uintptr_t>(offset, access_size,
21082105
env_->max_memory_size);
21092106

chromium/v8/src/wasm/compilation-environment.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,11 @@ struct CompilationEnv {
6363

6464
const LowerSimd lower_simd;
6565

66-
static constexpr uint32_t kMaxMemoryPagesAtRuntime =
67-
std::min(kV8MaxWasmMemoryPages,
68-
std::numeric_limits<uintptr_t>::max() / kWasmPageSize);
66+
// We assume that memories of size >= half of the virtual address space
67+
// cannot be allocated (see https://crbug.com/1201340).
68+
static constexpr uint32_t kMaxMemoryPagesAtRuntime = std::min(
69+
kV8MaxWasmMemoryPages,
70+
(uintptr_t{1} << (kSystemPointerSize == 4 ? 31 : 63)) / kWasmPageSize);
6971

7072
constexpr CompilationEnv(const WasmModule* module,
7173
UseTrapHandler use_trap_handler,

0 commit comments

Comments
 (0)