You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by jo...@apache.org on 2015/07/27 18:34:56 UTC
spark git commit: [SPARK-9364] Fix array out of bounds and
use-after-free bugs in UnsafeExternalSorter
Repository: spark
Updated Branches:
refs/heads/master 90006f3c5 -> ecad9d434
[SPARK-9364] Fix array out of bounds and use-after-free bugs in UnsafeExternalSorter
This patch fixes two bugs in UnsafeExternalSorter and UnsafeExternalRowSorter:
- UnsafeExternalSorter does not properly update freeSpaceInCurrentPage, which can cause it to write past the end of memory pages and trigger segfaults.
- UnsafeExternalRowSorter has a use-after-free bug when returning the last row from an iterator.
Author: Josh Rosen <jo...@databricks.com>
Closes #7680 from JoshRosen/SPARK-9364 and squashes the following commits:
590f311 [Josh Rosen] null out row
f4cf91d [Josh Rosen] Fix use-after-free bug in UnsafeExternalRowSorter.
8abcf82 [Josh Rosen] Properly decrement freeSpaceInCurrentPage in UnsafeExternalSorter
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/ecad9d43
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/ecad9d43
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/ecad9d43
Branch: refs/heads/master
Commit: ecad9d4346ec158746e61aebdf1590215a77f369
Parents: 90006f3
Author: Josh Rosen <jo...@databricks.com>
Authored: Mon Jul 27 09:34:49 2015 -0700
Committer: Josh Rosen <jo...@databricks.com>
Committed: Mon Jul 27 09:34:49 2015 -0700
----------------------------------------------------------------------
.../unsafe/sort/UnsafeExternalSorter.java | 7 ++++++-
.../unsafe/sort/UnsafeExternalSorterSuite.java | 19 +++++++++++++++++++
.../sql/execution/UnsafeExternalRowSorter.java | 9 ++++++---
3 files changed, 31 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/ecad9d43/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
index 4d6731e..80b03d7 100644
--- a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
+++ b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
@@ -150,6 +150,11 @@ public final class UnsafeExternalSorter {
return sorter.getMemoryUsage() + (allocatedPages.size() * (long) PAGE_SIZE);
}
+ @VisibleForTesting
+ public int getNumberOfAllocatedPages() {
+ return allocatedPages.size();
+ }
+
public long freeMemory() {
long memoryFreed = 0;
for (MemoryBlock block : allocatedPages) {
@@ -257,7 +262,7 @@ public final class UnsafeExternalSorter {
currentPagePosition,
lengthInBytes);
currentPagePosition += lengthInBytes;
-
+ freeSpaceInCurrentPage -= totalSpaceRequired;
sorter.insertRecord(recordAddress, prefix);
}
http://git-wip-us.apache.org/repos/asf/spark/blob/ecad9d43/core/src/test/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorterSuite.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorterSuite.java b/core/src/test/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorterSuite.java
index ea8755e..0e391b7 100644
--- a/core/src/test/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorterSuite.java
+++ b/core/src/test/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorterSuite.java
@@ -199,4 +199,23 @@ public class UnsafeExternalSorterSuite {
}
}
+ @Test
+ public void testFillingPage() throws Exception {
+ final UnsafeExternalSorter sorter = new UnsafeExternalSorter(
+ memoryManager,
+ shuffleMemoryManager,
+ blockManager,
+ taskContext,
+ recordComparator,
+ prefixComparator,
+ 1024,
+ new SparkConf());
+
+ byte[] record = new byte[16];
+ while (sorter.getNumberOfAllocatedPages() < 2) {
+ sorter.insertRecord(record, PlatformDependent.BYTE_ARRAY_OFFSET, record.length, 0);
+ }
+ sorter.freeMemory();
+ }
+
}
http://git-wip-us.apache.org/repos/asf/spark/blob/ecad9d43/sql/catalyst/src/main/java/org/apache/spark/sql/execution/UnsafeExternalRowSorter.java
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/execution/UnsafeExternalRowSorter.java b/sql/catalyst/src/main/java/org/apache/spark/sql/execution/UnsafeExternalRowSorter.java
index be4ff40..4c3f2c6 100644
--- a/sql/catalyst/src/main/java/org/apache/spark/sql/execution/UnsafeExternalRowSorter.java
+++ b/sql/catalyst/src/main/java/org/apache/spark/sql/execution/UnsafeExternalRowSorter.java
@@ -124,7 +124,7 @@ final class UnsafeExternalRowSorter {
return new AbstractScalaRowIterator() {
private final int numFields = schema.length();
- private final UnsafeRow row = new UnsafeRow();
+ private UnsafeRow row = new UnsafeRow();
@Override
public boolean hasNext() {
@@ -141,10 +141,13 @@ final class UnsafeExternalRowSorter {
numFields,
sortedIterator.getRecordLength());
if (!hasNext()) {
- row.copy(); // so that we don't have dangling pointers to freed page
+ UnsafeRow copy = row.copy(); // so that we don't have dangling pointers to freed page
+ row = null; // so that we don't keep references to the base object
cleanupResources();
+ return copy;
+ } else {
+ return row;
}
- return row;
} catch (IOException e) {
cleanupResources();
// Scala iterators don't declare any checked exceptions, so we need to use this hack
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org