Skip to content

Commit

Permalink
deps: backport 3a9bfec from v8 upstream
Browse files Browse the repository at this point in the history
Some of the logic from `zone.cc` is found in `zone-inl.h` in this
release stream.

Original commit message:

  Fix overflow issue in Zone::New

  When requesting a large allocation near the end of the address space,
  the computation could overflow and erroneously *not* grow the Zone
  as required.

	BUG=chromium:606115
	LOG=y

  Review-Url: https://codereview.chromium.org/1930873002
  Cr-Commit-Position: refs/heads/master@{#35903}

PR-URL: nodejs-private/node-private#43
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
  • Loading branch information
Myles Borins authored and rvagg committed Jun 23, 2016
1 parent ffc55f7 commit fcb9145
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
5 changes: 4 additions & 1 deletion deps/v8/src/zone-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ inline void* Zone::New(int size) {
// Check if the requested size is available without expanding.
Address result = position_;

if (size > limit_ - position_) {
const uintptr_t limit = reinterpret_cast<uintptr_t>(limit_);
const uintptr_t position = reinterpret_cast<uintptr_t>(position_);
// position_ > limit_ can be true after the alignment correction above.
if (limit < position || size > limit - position) {
result = NewExpand(size);
} else {
position_ += size;
Expand Down
5 changes: 4 additions & 1 deletion deps/v8/src/zone.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ Address Zone::NewExpand(int size) {
// Make sure the requested size is already properly aligned and that
// there isn't enough room in the Zone to satisfy the request.
ASSERT(size == RoundDown(size, kAlignment));
ASSERT(size > limit_ - position_);
ASSERT(limit_ < position_ ||
reinterpret_cast<uintptr_t>(limit_) -
reinterpret_cast<uintptr_t>(position_) <
size);

// Compute the new segment size. We use a 'high water mark'
// strategy, where we increase the segment size every time we expand
Expand Down

3 comments on commit fcb9145

@bnoordhuis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spotrh I understand you maintain a branch of V8 3.14? You probably want to pick up this fix for CVE-2016-1669. 5a60e0d might be nice to have too but is not critical.

cc @jeroenooms

@spotrh
Copy link

@spotrh spotrh commented on fcb9145 Jul 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll add both fixes.

@jeroen
Copy link
Contributor

@jeroen jeroen commented on fcb9145 Jul 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, both patches are also backported to v8-314: https://github.com/v8-314/v8/

Please sign in to comment.