From b23adb6d153b94517545dbb8a11aa063f7abe7d3 Mon Sep 17 00:00:00 2001
From: Jason Tedor <jason@tedor.me>
Date: Sat, 18 Mar 2017 20:02:18 -0400
Subject: [PATCH] Avoid overflow when computing total FS stats

When adding filesystem stats from individual filesystems, free and
available can overflow. This commit guards against this by adjusting
these situations to Long.MAX_VALUE.

Relates #23641
---
 .../org/elasticsearch/monitor/fs/FsInfo.java  |  4 +-
 .../monitor/fs/FsProbeTests.java              | 72 ++++++++++++++-----
 2 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java b/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
index 7bb1e51cd23..e20eb42427f 100644
--- a/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
+++ b/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
@@ -138,8 +138,8 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
 
         public void add(Path path) {
             total = FsProbe.adjustForHugeFilesystems(addLong(total, path.total));
-            free = addLong(free, path.free);
-            available = addLong(available, path.available);
+            free = FsProbe.adjustForHugeFilesystems(addLong(free, path.free));
+            available = FsProbe.adjustForHugeFilesystems(addLong(available, path.available));
             if (path.spins != null && path.spins.booleanValue()) {
                 // Spinning is contagious!
                 spins = Boolean.TRUE;
diff --git a/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java b/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java
index 3d4903e0472..0db1709e92c 100644
--- a/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java
+++ b/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java
@@ -36,6 +36,8 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
@@ -93,26 +95,64 @@ public class FsProbeTests extends ESTestCase {
     }
 
     public void testFsInfoOverflow() throws Exception {
-        FsInfo.Path pathStats = new FsInfo.Path("/foo/bar", null,
-                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+        final FsInfo.Path pathStats =
+                new FsInfo.Path(
+                        "/foo/bar",
+                        null,
+                        randomNonNegativeLong(),
+                        randomNonNegativeLong(),
+                        randomNonNegativeLong());
 
-        // While not overflowing, keep adding
-        FsInfo.Path pathToAdd = new FsInfo.Path("/foo/baz", null,
-                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
-        while ((pathStats.total + pathToAdd.total) > 0) {
-            // Add itself as a path, to increase the total bytes until it overflows
-            logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total);
-            pathStats.add(pathToAdd);
-            pathToAdd = new FsInfo.Path("/foo/baz", null,
-                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
-        }
+        addUntilOverflow(
+                pathStats,
+                p -> p.total,
+                "total",
+                () -> new FsInfo.Path("/foo/baz", null, randomNonNegativeLong(), 0, 0));
 
-        logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total);
-        assertThat(pathStats.total + pathToAdd.total, lessThan(0L));
-        pathStats.add(pathToAdd);
+        addUntilOverflow(
+                pathStats,
+                p -> p.free,
+                "free",
+                () -> new FsInfo.Path("/foo/baz", null, 0, randomNonNegativeLong(), 0));
 
-        // Even after overflowing, it should not be negative
+        addUntilOverflow(
+                pathStats,
+                p -> p.available,
+                "available",
+                () -> new FsInfo.Path("/foo/baz", null, 0, 0, randomNonNegativeLong()));
+
+        // even after overflowing these should not be negative
         assertThat(pathStats.total, greaterThan(0L));
+        assertThat(pathStats.free, greaterThan(0L));
+        assertThat(pathStats.available, greaterThan(0L));
+    }
+
+    private void addUntilOverflow(
+            final FsInfo.Path pathStats,
+            final Function<FsInfo.Path, Long> getter,
+            final String field,
+            final Supplier<FsInfo.Path> supplier) {
+        FsInfo.Path pathToAdd = supplier.get();
+        while ((getter.apply(pathStats) + getter.apply(pathToAdd)) > 0) {
+            // add a path to increase the total bytes until it overflows
+            logger.info(
+                    "--> adding {} bytes to {}, {} will be: {}",
+                    getter.apply(pathToAdd),
+                    getter.apply(pathStats),
+                    field,
+                    getter.apply(pathStats) + getter.apply(pathToAdd));
+            pathStats.add(pathToAdd);
+            pathToAdd = supplier.get();
+        }
+        // this overflows
+        logger.info(
+                "--> adding {} bytes to {}, {} will be: {}",
+                getter.apply(pathToAdd),
+                getter.apply(pathStats),
+                field,
+                getter.apply(pathStats) + getter.apply(pathToAdd));
+        assertThat(getter.apply(pathStats) + getter.apply(pathToAdd), lessThan(0L));
+        pathStats.add(pathToAdd);
     }
 
     public void testIoStats() {