commit 01a0e7fa189b37b82be971bc50ecce5883523293
author Jeff Sharkey <jsharkey@android.com> Tue Oct 17 16:06:32 2017 -0600
committer Jeff Sharkey <jsharkey@android.com> Wed Oct 18 11:44:57 2017 -0600
tree a88f91865e74211b89f6678eaa2f65a900e53dcc
parent a6f628587523624d03ba78f49d2aff6f57368d99

Fix task memory leaks; better path validation.

We've been allocating task objects without freeing them, oops.  We
don't really need full classes for these tasks, so move them to
blocking methods, and invoke them from a detached thread.

Remove FIDTRIM support, which isn't meaningful on UFS-based flash
devices.  Modern devices require FBE/FDE which gives us better
protection against trimmed data lingering around.

Rename "Trim" to more generic "IdleMaint", since it'll soon extend
to include custom F2FS optimization logic.

Check for shady ".." when validating paths.

Test: cts-tradefed run commandAndExit cts-dev -m CtsOsTestCases -t android.os.storage.cts.StorageManagerTest
Test: cts-tradefed run commandAndExit cts-dev --abi armeabi-v7a -m CtsAppSecurityHostTestCases -t android.appsecurity.cts.AdoptableHostTest
Bug: 67041047
Change-Id: I4fb194c5d5ef13f413c02acedfbaaf79c567582b

VoldNativeService.cpp

diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp
index c82eb92..d8832d3 100644
--- a/VoldNativeService.cpp
+++ b/VoldNativeService.cpp
@@ -18,16 +18,17 @@
 
 #include "VoldNativeService.h"
 #include "VolumeManager.h"
-#include "BenchmarkTask.h"
-#include "MoveTask.h"
+#include "Benchmark.h"
+#include "MoveStorage.h"
 #include "Process.h"
-#include "TrimTask.h"
+#include "IdleMaint.h"
 
 #include "cryptfs.h"
 #include "Ext4Crypt.h"
 #include "MetadataCrypt.h"
 
 #include <fstream>
+#include <thread>
 
 #include <android-base/logging.h>
 #include <android-base/stringprintf.h>
@@ -120,6 +121,10 @@
         return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
                 StringPrintf("Path %s is relative", path.c_str()));
     }
+    if ((path + '/').find("/../") != std::string::npos) {
+        return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
+                StringPrintf("Path %s is shady", path.c_str()));
+    }
     for (const char& c : path) {
         if (c == '\0' || c == '\n') {
             return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
@@ -377,7 +382,9 @@
         return error("Volume " + volId + " missing path");
     }
 
-    (new android::vold::BenchmarkTask(path, listener))->start();
+    std::thread([=]() {
+        android::vold::Benchmark(path, listener);
+    }).detach();
     return ok();
 }
 
@@ -395,7 +402,10 @@
     } else if (toVol == nullptr) {
         return error("Failed to find volume " + toVolId);
     }
-    (new android::vold::MoveTask(fromVol, toVol, listener))->start();
+
+    std::thread([=]() {
+        android::vold::MoveStorage(fromVol, toVol, listener);
+    }).detach();
     return ok();
 }
 
@@ -446,7 +456,9 @@
     ENFORCE_UID(AID_SYSTEM);
     ACQUIRE_LOCK;
 
-    (new android::vold::TrimTask(fstrimFlags, listener))->start();
+    std::thread([=]() {
+        android::vold::Trim(listener);
+    }).detach();
     return ok();
 }
 
@@ -712,6 +724,7 @@
 
 binder::Status VoldNativeService::secdiscard(const std::string& path) {
     ENFORCE_UID(AID_SYSTEM);
+    CHECK_ARGUMENT_PATH(path);
     ACQUIRE_CRYPT_LOCK;
 
     return translateBool(e4crypt_secdiscard(path));