From 270f690ff6047cc3e797a3f34b7efa26e7232183 Mon Sep 17 00:00:00 2001 From: Benjamin Lerer Date: Tue, 8 Aug 2017 16:51:03 +0200 Subject: [PATCH] Prevent integer overflow on exabyte filesystems patch by Matt Wringe and Benjamin Lerer; reviewed by Alex Petrov for CASSANDRA-13067 --- CHANGES.txt | 1 + .../cassandra/config/DatabaseDescriptor.java | 28 ++- .../org/apache/cassandra/db/Directories.java | 2 +- .../apache/cassandra/io/util/FileUtils.java | 182 ++++++++++++++++-- 4 files changed, 193 insertions(+), 20 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 36c34a1c3b..f712333ecf 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.2.11 + * Prevent integer overflow on exabyte filesystems (CASSANDRA-13067) * Fix queries with LIMIT and filtering on clustering columns (CASSANDRA-11223) * Fix potential NPE when resume bootstrap fails (CASSANDRA-13272) * Fix toJSONString for the UDT, tuple and collection types (CASSANDRA-13592) diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 981026d505..90a82fe831 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -21,7 +21,6 @@ import java.io.File; import java.io.IOException; import java.net.*; import java.nio.file.FileStore; -import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; @@ -56,6 +55,9 @@ import org.apache.cassandra.thrift.ThriftServer; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.memory.*; +import static org.apache.cassandra.io.util.FileUtils.ONE_GB; +import static org.apache.cassandra.io.util.FileUtils.ONE_MB; + public class DatabaseDescriptor { private static final Logger logger = LoggerFactory.getLogger(DatabaseDescriptor.class); @@ -530,7 +532,7 @@ public class DatabaseDescriptor try { // use 1/4 of available space. See discussion on #10013 and #10199 - minSize = Ints.checkedCast((guessFileStore(conf.commitlog_directory).getTotalSpace() / 1048576) / 4); + minSize = Ints.saturatedCast((guessFileStore(conf.commitlog_directory).getTotalSpace() / 1048576) / 4); } catch (IOException e) { @@ -576,7 +578,7 @@ public class DatabaseDescriptor try { - dataFreeBytes += guessFileStore(datadir).getUnallocatedSpace(); + dataFreeBytes = saturatedSum(dataFreeBytes, guessFileStore(datadir).getUnallocatedSpace()); } catch (IOException e) { @@ -585,9 +587,9 @@ public class DatabaseDescriptor datadir), e); } } - if (dataFreeBytes < 64L * 1024 * 1048576) // 64 GB + if (dataFreeBytes < 64 * ONE_GB) logger.warn("Only {} MB free across all data volumes. Consider adding more capacity to your cluster or removing obsolete snapshots", - dataFreeBytes / 1048576); + dataFreeBytes / ONE_MB); if (conf.commitlog_directory.equals(conf.saved_caches_directory)) @@ -697,6 +699,20 @@ public class DatabaseDescriptor throw new ConfigurationException("otc_coalescing_enough_coalesced_messages must be positive", false); } + /** + * Computes the sum of the 2 specified positive values returning {@code Long.MAX_VALUE} if the sum overflow. + * + * @param left the left operand + * @param right the right operand + * @return the sum of the 2 specified positive values of {@code Long.MAX_VALUE} if the sum overflow. + */ + private static long saturatedSum(long left, long right) + { + assert left >= 0 && right >= 0; + long sum = left + right; + return sum < 0 ? Long.MAX_VALUE : sum; + } + private static FileStore guessFileStore(String dir) throws IOException { Path path = Paths.get(dir); @@ -704,7 +720,7 @@ public class DatabaseDescriptor { try { - return Files.getFileStore(path); + return FileUtils.getFileStore(path); } catch (IOException e) { diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java index 2b3662f022..fa76b610e4 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -482,7 +482,7 @@ public class Directories public long getAvailableSpace() { - return location.getUsableSpace(); + return FileUtils.getUsableSpace(location); } } diff --git a/src/java/org/apache/cassandra/io/util/FileUtils.java b/src/java/org/apache/cassandra/io/util/FileUtils.java index 8d122dd000..bf0fae5ec6 100644 --- a/src/java/org/apache/cassandra/io/util/FileUtils.java +++ b/src/java/org/apache/cassandra/io/util/FileUtils.java @@ -21,11 +21,12 @@ import java.io.*; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.nio.file.*; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.attribute.FileStoreAttributeView; import java.text.DecimalFormat; import java.util.Arrays; import java.util.concurrent.atomic.AtomicReference; -import org.apache.cassandra.config.Config; import sun.nio.ch.DirectBuffer; import org.slf4j.Logger; @@ -45,10 +46,10 @@ import static org.apache.cassandra.utils.Throwables.merge; public final class FileUtils { private static final Logger logger = LoggerFactory.getLogger(FileUtils.class); - private static final double KB = 1024d; - private static final double MB = 1024*1024d; - private static final double GB = 1024*1024*1024d; - private static final double TB = 1024*1024*1024*1024d; + public static final long ONE_KB = 1024; + public static final long ONE_MB = 1024 * ONE_KB; + public static final long ONE_GB = 1024 * ONE_MB; + public static final long ONE_TB = 1024 * ONE_GB; private static final DecimalFormat df = new DecimalFormat("#.##"); private static final boolean canCleanDirectBuffers; @@ -330,27 +331,27 @@ public final class FileUtils public static String stringifyFileSize(double value) { double d; - if ( value >= TB ) + if ( value >= ONE_TB ) { - d = value / TB; + d = value / ONE_TB; String val = df.format(d); return val + " TB"; } - else if ( value >= GB ) + else if ( value >= ONE_GB ) { - d = value / GB; + d = value / ONE_GB; String val = df.format(d); return val + " GB"; } - else if ( value >= MB ) + else if ( value >= ONE_MB ) { - d = value / MB; + d = value / ONE_MB; String val = df.format(d); return val + " MB"; } - else if ( value >= KB ) + else if ( value >= ONE_KB ) { - d = value / KB; + d = value / ONE_KB; String val = df.format(d); return val + " KB"; } @@ -478,4 +479,159 @@ public final class FileUtils { fsErrorHandler.getAndSet(handler); } + + /** + * Returns the size of the specified partition. + *

This method handles large file system by returning {@code Long.MAX_VALUE} if the size overflow. + * See JDK-8179320 for more information.

+ * + * @param file the partition + * @return the size, in bytes, of the partition or {@code 0L} if the abstract pathname does not name a partition + */ + public static long getTotalSpace(File file) + { + return handleLargeFileSystem(file.getTotalSpace()); + } + + /** + * Returns the number of unallocated bytes on the specified partition. + *

This method handles large file system by returning {@code Long.MAX_VALUE} if the number of unallocated bytes + * overflow. See JDK-8179320 for more information

+ * + * @param file the partition + * @return the number of unallocated bytes on the partition or {@code 0L} + * if the abstract pathname does not name a partition. + */ + public static long getFreeSpace(File file) + { + return handleLargeFileSystem(file.getFreeSpace()); + } + + /** + * Returns the number of available bytes on the specified partition. + *

This method handles large file system by returning {@code Long.MAX_VALUE} if the number of available bytes + * overflow. See JDK-8179320 for more information

+ * + * @param file the partition + * @return the number of available bytes on the partition or {@code 0L} + * if the abstract pathname does not name a partition. + */ + public static long getUsableSpace(File file) + { + return handleLargeFileSystem(file.getUsableSpace()); + } + + /** + * Returns the {@link FileStore} representing the file store where a file + * is located. This {@link FileStore} handles large file system by returning {@code Long.MAX_VALUE} + * from {@code FileStore#getTotalSpace()}, {@code FileStore#getUnallocatedSpace()} and {@code FileStore#getUsableSpace()} + * it the value is bigger than {@code Long.MAX_VALUE}. See JDK-8162520 + * for more information. + * + * @param path the path to the file + * @return the file store where the file is stored + */ + public static FileStore getFileStore(Path path) throws IOException + { + return new SafeFileStore(Files.getFileStore(path)); + } + + /** + * Handle large file system by returning {@code Long.MAX_VALUE} when the size overflows. + * @param size returned by the Java's FileStore methods + * @return the size or {@code Long.MAX_VALUE} if the size was bigger than {@code Long.MAX_VALUE} + */ + private static long handleLargeFileSystem(long size) + { + return size < 0 ? Long.MAX_VALUE : size; + } + + /** + * Private constructor as the class contains only static methods. + */ + private FileUtils() + { + } + + /** + * FileStore decorator used to safely handle large file system. + * + *

Java's FileStore methods (getTotalSpace/getUnallocatedSpace/getUsableSpace) are limited to reporting bytes as + * signed long (2^63-1), if the filesystem is any bigger, then the size overflows. {@code SafeFileStore} will + * return {@code Long.MAX_VALUE} if the size overflow.

+ * + * @see https://bugs.openjdk.java.net/browse/JDK-8162520. + */ + private static final class SafeFileStore extends FileStore + { + /** + * The decorated {@code FileStore} + */ + private final FileStore fileStore; + + public SafeFileStore(FileStore fileStore) + { + this.fileStore = fileStore; + } + + @Override + public String name() + { + return fileStore.name(); + } + + @Override + public String type() + { + return fileStore.type(); + } + + @Override + public boolean isReadOnly() + { + return fileStore.isReadOnly(); + } + + @Override + public long getTotalSpace() throws IOException + { + return handleLargeFileSystem(fileStore.getTotalSpace()); + } + + @Override + public long getUsableSpace() throws IOException + { + return handleLargeFileSystem(fileStore.getUsableSpace()); + } + + @Override + public long getUnallocatedSpace() throws IOException + { + return handleLargeFileSystem(fileStore.getUnallocatedSpace()); + } + + @Override + public boolean supportsFileAttributeView(Class type) + { + return fileStore.supportsFileAttributeView(type); + } + + @Override + public boolean supportsFileAttributeView(String name) + { + return fileStore.supportsFileAttributeView(name); + } + + @Override + public V getFileStoreAttributeView(Class type) + { + return fileStore.getFileStoreAttributeView(type); + } + + @Override + public Object getAttribute(String attribute) throws IOException + { + return fileStore.getAttribute(attribute); + } + } }