Always close FUSE file descriptor when we're exiting the thread.
We had previously relied on the process being killed when we're unable
to start the FUSE daemon successfully; but while the current
onStartSession() code would throw an exception, that would just get
thrown over binder, without the process crashing. This could result in
the following sequence:
1) We mount a new FUSE filesystem and call onStartSession() with the
fuse FD, fd1
2) onStartSession() starts the FUSE daemon thread and starts waiting
for it to come up
3) For whatever reaosn, the FUSE thread doesn't come up and exits;
now we throw an IllegalStateException, but that just gets swallowed
by binder, and the MP process stays up
4) We try to start the session again; this may be successful, but we
still hold the original file descriptor fd1 open, and any requests
outstanding to this filesystem would hang forever
5) llkd notices these threads are stuck for 10 minutes on I/O, and
panics the kernel
To prevent this from happening, just give the FUSE daemon thread
ownership of the FD, and make sure it gets closed whenever something
goes wrong by using unique_fd.
As for why 3) can happen, it appears to be that we can attempt to mount
the filesystem for user 10 before user 0. This needs a separate fix.
Bug: 153816731
Test: device boots ok, filesystems mounted
Change-Id: Ica89d77357c1c822dc536d5e4c946d5235670a73
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index f1bc4e4..01837bf 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -1488,7 +1488,7 @@
return active.load(std::memory_order_acquire);
}
-void FuseDaemon::Start(const int fd, const std::string& path) {
+void FuseDaemon::Start(android::base::unique_fd fd, const std::string& path) {
struct fuse_args args;
struct fuse_cmdline_opts opts;
diff --git a/jni/FuseDaemon.h b/jni/FuseDaemon.h
index d9925d4..3c4b947 100644
--- a/jni/FuseDaemon.h
+++ b/jni/FuseDaemon.h
@@ -20,6 +20,8 @@
#include <memory>
#include <string>
+#include <android-base/unique_fd.h>
+
#include "MediaProviderWrapper.h"
#include "jni.h"
@@ -35,7 +37,7 @@
/**
* Start the FUSE daemon loop that will handle filesystem calls.
*/
- void Start(const int fd, const std::string& path);
+ void Start(android::base::unique_fd fd, const std::string& path);
/**
* Checks if the FUSE daemon is started.
diff --git a/jni/com_android_providers_media_FuseDaemon.cpp b/jni/com_android_providers_media_FuseDaemon.cpp
index ae1c175..3a65696 100644
--- a/jni/com_android_providers_media_FuseDaemon.cpp
+++ b/jni/com_android_providers_media_FuseDaemon.cpp
@@ -24,6 +24,7 @@
#include "FuseDaemon.h"
#include "MediaProviderWrapper.h"
#include "android-base/logging.h"
+#include "android-base/unique_fd.h"
namespace mediaprovider {
namespace {
@@ -42,12 +43,14 @@
LOG(DEBUG) << "Starting the FUSE daemon...";
fuse::FuseDaemon* const daemon = reinterpret_cast<fuse::FuseDaemon*>(java_daemon);
+ android::base::unique_fd ufd(fd);
+
ScopedUtfChars utf_chars_path(env, java_path);
if (!utf_chars_path.c_str()) {
return;
}
- daemon->Start(fd, utf_chars_path.c_str());
+ daemon->Start(std::move(ufd), utf_chars_path.c_str());
}
bool com_android_providers_media_FuseDaemon_is_started(JNIEnv* env, jobject self,
diff --git a/src/com/android/providers/media/fuse/FuseDaemon.java b/src/com/android/providers/media/fuse/FuseDaemon.java
index 95a160b..3b7a0f8 100644
--- a/src/com/android/providers/media/fuse/FuseDaemon.java
+++ b/src/com/android/providers/media/fuse/FuseDaemon.java
@@ -151,7 +151,10 @@
}
private native long native_new(MediaProvider mediaProvider);
+
+ // Takes ownership of the passed in file descriptor!
private native void native_start(long daemon, int deviceFd, String path);
+
private native void native_delete(long daemon);
private native boolean native_should_open_with_fuse(long daemon, String path, boolean readLock,
int fd);