summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rwxr-xr-xjni/FuseDaemon.cpp47
-rw-r--r--jni/MediaProviderWrapper.cpp14
-rw-r--r--jni/MediaProviderWrapper.h20
-rw-r--r--jni/node-inl.h31
-rw-r--r--jni/node_test.cpp15
-rw-r--r--src/com/android/providers/media/FileLookupResult.java7
-rw-r--r--src/com/android/providers/media/MediaProvider.java38
7 files changed, 100 insertions, 72 deletions
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index a75660c11..b30cffa15 100755
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -390,12 +390,14 @@ static void fuse_inval(fuse_session* se, fuse_ino_t parent_ino, fuse_ino_t child
}
}
-static double get_attr_timeout(const string& path, bool should_inval, node* node,
- struct fuse* fuse) {
- if (fuse->disable_dentry_cache || should_inval || is_package_owned_path(path, fuse->path)) {
+static double get_attr_timeout(const string& path, node* node, struct fuse* fuse) {
+ if (fuse->disable_dentry_cache || node->ShouldInvalidate() ||
+ is_package_owned_path(path, fuse->path)) {
// We set dentry timeout to 0 for the following reasons:
// 1. The dentry cache was completely disabled
- // 2. Case-insensitive lookups need to invalidate other case-insensitive dentry matches
+ // 2.1 Case-insensitive lookups need to invalidate other case-insensitive dentry matches
+ // 2.2 Nodes supporting transforms need to be invalidated, so that subsequent lookups by a
+ // uid requiring a transform is guaranteed to come to the FUSE daemon.
// 3. With app data isolation enabled, app A should not guess existence of app B from the
// Android/{data,obb}/<package> paths, hence we prevent the kernel from caching that
// information.
@@ -404,8 +406,7 @@ static double get_attr_timeout(const string& path, bool should_inval, node* node
return std::numeric_limits<double>::max();
}
-static double get_entry_timeout(const string& path, bool should_inval, node* node,
- struct fuse* fuse) {
+static double get_entry_timeout(const string& path, node* node, struct fuse* fuse) {
string media_path = fuse->GetEffectiveRootPath() + "/Android/media";
if (path.find(media_path, 0) == 0) {
// Installd might delete Android/media/<package> dirs when app data is cleared.
@@ -413,7 +414,7 @@ static double get_entry_timeout(const string& path, bool should_inval, node* nod
// dir via FUSE.
return 0;
}
- return get_attr_timeout(path, should_inval, node, fuse);
+ return get_attr_timeout(path, node, fuse);
}
static std::string get_path(node* node) {
@@ -433,7 +434,7 @@ static node* make_node_entry(fuse_req_t req, node* parent, const string& name, c
return NULL;
}
- bool should_inval = false;
+ bool should_invalidate = false;
bool transforms_complete = true;
int transforms = 0;
string io_path;
@@ -441,7 +442,7 @@ static node* make_node_entry(fuse_req_t req, node* parent, const string& name, c
if (S_ISREG(e->attr.st_mode)) {
// Handle potential file transforms
std::unique_ptr<mediaprovider::fuse::FileLookupResult> file_lookup_result =
- fuse->mp->FileLookup(path, req->ctx.uid);
+ fuse->mp->FileLookup(path, req->ctx.uid, req->ctx.pid);
if (!file_lookup_result) {
// Fail lookup if we can't fetch FileLookupResult for path
@@ -453,37 +454,34 @@ static node* make_node_entry(fuse_req_t req, node* parent, const string& name, c
transforms = file_lookup_result->transforms;
io_path = file_lookup_result->io_path;
transforms_complete = file_lookup_result->transforms_complete;
+ // Invalidate if the inode supports transforms so that we always get a lookup into userspace
+ should_invalidate = file_lookup_result->transforms_supported;
// Update size with io_path size if io_path is not same as path
if (!io_path.empty() && (io_path != path) && (lstat(io_path.c_str(), &e->attr) < 0)) {
*error_code = errno;
return NULL;
}
-
- // Invalidate if there are any transforms so that we always get a lookup into userspace
- should_inval = should_inval || transforms;
}
node = parent->LookupChildByName(name, true /* acquire */, transforms);
if (!node) {
- node = ::node::Create(parent, name, io_path, transforms_complete, transforms, &fuse->lock,
- &fuse->tracker);
+ node = ::node::Create(parent, name, io_path, should_invalidate, transforms_complete,
+ transforms, &fuse->lock, &fuse->tracker);
} else if (!mediaprovider::fuse::containsMount(path, std::to_string(getuid() / PER_USER_RANGE))) {
- should_inval = should_inval || node->HasCaseInsensitiveMatch();
// Only invalidate a path if it does not contain mount.
// Invalidate both names to ensure there's no dentry left in the kernel after the following
// operations:
// 1) touch foo, touch FOO, unlink *foo*
// 2) touch foo, touch FOO, unlink *FOO*
// Invalidating lookup_name fixes (1) and invalidating node_name fixes (2)
- // |should_inval| invalidates lookup_name by using 0 timeout below and we explicitly
+ // SetShouldInvalidate invalidates lookup_name by using 0 timeout below and we explicitly
// invalidate node_name if different case
// Note that we invalidate async otherwise we will deadlock the kernel
if (name != node->GetName()) {
- should_inval = true;
// Record that we have made a case insensitive lookup, this allows us invalidate nodes
// correctly on subsequent lookups for the case of |node|
- node->SetCaseInsensitiveMatch();
+ node->SetShouldInvalidate();
// Make copies of the node name and path so we're not attempting to acquire
// any node locks from the invalidation thread. Depending on timing, we may end
@@ -503,8 +501,8 @@ static node* make_node_entry(fuse_req_t req, node* parent, const string& name, c
// reuse inode numbers.
e->generation = 0;
e->ino = fuse->ToInode(node);
- e->entry_timeout = get_entry_timeout(path, should_inval, node, fuse);
- e->attr_timeout = get_attr_timeout(path, should_inval, node, fuse);
+ e->entry_timeout = get_entry_timeout(path, node, fuse);
+ e->attr_timeout = get_attr_timeout(path, node, fuse);
return node;
}
@@ -693,10 +691,7 @@ static void pf_getattr(fuse_req_t req,
if (lstat(path.c_str(), &s) < 0) {
fuse_reply_err(req, errno);
} else {
- fuse_reply_attr(
- req, &s,
- get_attr_timeout(path, node->GetTransforms() || node->HasCaseInsensitiveMatch(),
- node, fuse));
+ fuse_reply_attr(req, &s, get_attr_timeout(path, node, fuse));
}
}
@@ -791,9 +786,7 @@ static void pf_setattr(fuse_req_t req,
}
lstat(path.c_str(), attr);
- fuse_reply_attr(req, attr,
- get_attr_timeout(path, node->GetTransforms() || node->HasCaseInsensitiveMatch(),
- node, fuse));
+ fuse_reply_attr(req, attr, get_attr_timeout(path, node, fuse));
}
static void pf_canonical_path(fuse_req_t req, fuse_ino_t ino)
diff --git a/jni/MediaProviderWrapper.cpp b/jni/MediaProviderWrapper.cpp
index 27f93397e..b1926c136 100644
--- a/jni/MediaProviderWrapper.cpp
+++ b/jni/MediaProviderWrapper.cpp
@@ -295,7 +295,7 @@ MediaProviderWrapper::MediaProviderWrapper(JNIEnv* env, jobject media_provider)
/*is_static*/ false);
mid_file_lookup_ =
CacheMethod(env, "onFileLookup",
- "(Ljava/lang/String;I)Lcom/android/providers/media/FileLookupResult;",
+ "(Ljava/lang/String;II)Lcom/android/providers/media/FileLookupResult;",
/*is_static*/ false);
file_lookup_result_class_ = env->FindClass("com/android/providers/media/FileLookupResult");
@@ -305,7 +305,9 @@ MediaProviderWrapper::MediaProviderWrapper(JNIEnv* env, jobject media_provider)
file_lookup_result_class_ =
reinterpret_cast<jclass>(env->NewGlobalRef(file_lookup_result_class_));
fid_file_lookup_transforms_ = CacheFileLookupField(env, "transforms", "I");
+ fid_file_lookup_uid_ = CacheFileLookupField(env, "uid", "I");
fid_file_lookup_transforms_complete_ = CacheFileLookupField(env, "transformsComplete", "Z");
+ fid_file_lookup_transforms_supported_ = CacheFileLookupField(env, "transformsSupported", "Z");
fid_file_lookup_io_path_ = CacheFileLookupField(env, "ioPath", "Ljava/lang/String;");
}
@@ -476,7 +478,7 @@ bool MediaProviderWrapper::IsAppCloneUser(uid_t userId) {
}
std::unique_ptr<FileLookupResult> MediaProviderWrapper::FileLookup(const std::string& path,
- uid_t uid) {
+ uid_t uid, pid_t tid) {
JNIEnv* env = MaybeAttachCurrentThread();
ScopedLocalRef<jstring> j_path(env, env->NewStringUTF(path.c_str()));
@@ -489,15 +491,19 @@ std::unique_ptr<FileLookupResult> MediaProviderWrapper::FileLookup(const std::st
}
int transforms = env->GetIntField(j_res_file_lookup_object.get(), fid_file_lookup_transforms_);
+ int original_uid = env->GetIntField(j_res_file_lookup_object.get(), fid_file_lookup_uid_);
bool transforms_complete = env->GetBooleanField(j_res_file_lookup_object.get(),
fid_file_lookup_transforms_complete_);
+ bool transforms_supported = env->GetBooleanField(j_res_file_lookup_object.get(),
+ fid_file_lookup_transforms_supported_);
ScopedLocalRef<jstring> j_io_path(
env,
(jstring)env->GetObjectField(j_res_file_lookup_object.get(), fid_file_lookup_io_path_));
ScopedUtfChars j_io_path_utf(env, j_io_path.get());
- std::unique_ptr<FileLookupResult> file_lookup_result = std::make_unique<FileLookupResult>(
- transforms, transforms_complete, string(j_io_path_utf.c_str()));
+ std::unique_ptr<FileLookupResult> file_lookup_result =
+ std::make_unique<FileLookupResult>(transforms, original_uid, transforms_complete,
+ transforms_supported, string(j_io_path_utf.c_str()));
return file_lookup_result;
}
diff --git a/jni/MediaProviderWrapper.h b/jni/MediaProviderWrapper.h
index 31d9d1bc1..a807b9b5c 100644
--- a/jni/MediaProviderWrapper.h
+++ b/jni/MediaProviderWrapper.h
@@ -17,6 +17,7 @@
#ifndef MEDIAPROVIDER_FUSE_MEDIAPROVIDERWRAPPER_H_
#define MEDIAPROVIDER_FUSE_MEDIAPROVIDERWRAPPER_H_
+#include <android-base/logging.h>
#include <jni.h>
#include <sys/types.h>
@@ -40,15 +41,26 @@ namespace fuse {
* status and the ioPath. Provided by MediaProvider.java via a JNI call.
*/
struct FileLookupResult {
- FileLookupResult(int transforms, bool transforms_complete, const std::string& io_path)
- : transforms(transforms), transforms_complete(transforms_complete), io_path(io_path) {}
+ FileLookupResult(int transforms, uid_t uid, bool transforms_complete, bool transforms_supported,
+ const std::string& io_path)
+ : transforms(transforms),
+ uid(uid),
+ transforms_complete(transforms_complete),
+ transforms_supported(transforms_supported),
+ io_path(io_path) {
+ if (transforms != 0) {
+ CHECK(transforms_supported);
+ }
+ }
/**
* This field is not to be interpreted, it is determined and populated from MediaProvider
* via a JNI call.
*/
const int transforms;
+ const uid_t uid;
const bool transforms_complete;
+ const bool transforms_supported;
const std::string io_path;
};
@@ -193,7 +205,7 @@ class MediaProviderWrapper final {
/**
* Returns FileLookupResult to determine transform info for a path and uid.
*/
- std::unique_ptr<FileLookupResult> FileLookup(const std::string& path, uid_t uid);
+ std::unique_ptr<FileLookupResult> FileLookup(const std::string& path, uid_t uid, pid_t tid);
/** Transforms from src to dst file */
bool Transform(const std::string& src, const std::string& dst, int transforms, uid_t uid);
@@ -244,7 +256,9 @@ class MediaProviderWrapper final {
jmethodID mid_file_lookup_;
/** Cached FileLookupResult field IDs **/
jfieldID fid_file_lookup_transforms_;
+ jfieldID fid_file_lookup_uid_;
jfieldID fid_file_lookup_transforms_complete_;
+ jfieldID fid_file_lookup_transforms_supported_;
jfieldID fid_file_lookup_io_path_;
/**
diff --git a/jni/node-inl.h b/jni/node-inl.h
index 1fa62b01a..2b577bf4a 100644
--- a/jni/node-inl.h
+++ b/jni/node-inl.h
@@ -119,13 +119,14 @@ class node {
public:
// Creates a new node with the specified parent, name and lock.
static node* Create(node* parent, const std::string& name, const std::string& io_path,
- bool transforms_complete, const int transforms, std::recursive_mutex* lock,
- NodeTracker* tracker) {
+ bool should_invalidate, bool transforms_complete, const int transforms,
+ std::recursive_mutex* lock, NodeTracker* tracker) {
// Place the entire constructor under a critical section to make sure
// node creation, tracking (if enabled) and the addition to a parent are
// atomic.
std::lock_guard<std::recursive_mutex> guard(*lock);
- return new node(parent, name, io_path, transforms_complete, transforms, lock, tracker);
+ return new node(parent, name, io_path, should_invalidate, transforms_complete, transforms,
+ lock, tracker);
}
// Creates a new root node. Root nodes have no parents by definition
@@ -133,7 +134,8 @@ class node {
static node* CreateRoot(const std::string& path, std::recursive_mutex* lock,
NodeTracker* tracker) {
std::lock_guard<std::recursive_mutex> guard(*lock);
- node* root = new node(nullptr, path, path, true, 0, lock, tracker);
+ node* root = new node(nullptr, path, path, false /* should_invalidate */,
+ true /* transforms_complete */, 0, lock, tracker);
// The root always has one extra reference to avoid it being
// accidentally collected.
@@ -303,14 +305,14 @@ class node {
return false;
}
- bool HasCaseInsensitiveMatch() const {
+ bool ShouldInvalidate() const {
std::lock_guard<std::recursive_mutex> guard(*lock_);
- return has_case_insensitive_match_;
+ return should_invalidate_;
}
- void SetCaseInsensitiveMatch() {
+ void SetShouldInvalidate() {
std::lock_guard<std::recursive_mutex> guard(*lock_);
- has_case_insensitive_match_ = true;
+ should_invalidate_ = true;
}
bool HasRedactedCache() const {
@@ -346,8 +348,9 @@ class node {
static const node* LookupAbsolutePath(const node* root, const std::string& absolute_path);
private:
- node(node* parent, const std::string& name, const std::string& io_path, bool transforms_complete,
- const int transforms, std::recursive_mutex* lock, NodeTracker* tracker)
+ node(node* parent, const std::string& name, const std::string& io_path,
+ const bool should_invalidate, const bool transforms_complete, const int transforms,
+ std::recursive_mutex* lock, NodeTracker* tracker)
: name_(name),
io_path_(io_path),
transforms_complete_(transforms_complete),
@@ -355,7 +358,7 @@ class node {
refcount_(0),
parent_(nullptr),
has_redacted_cache_(false),
- has_case_insensitive_match_(false),
+ should_invalidate_(should_invalidate),
deleted_(false),
lock_(lock),
tracker_(tracker) {
@@ -366,6 +369,10 @@ class node {
if (parent != nullptr) {
AddToParent(parent);
}
+ // If the node requires transforms, we MUST never cache it in the VFS
+ if (transforms) {
+ CHECK(should_invalidate_);
+ }
}
// Acquires a reference to a node. This maps to the "lookup count" specified
@@ -499,7 +506,7 @@ class node {
// List of directory handles associated with this node. Guarded by |lock_|.
std::vector<std::unique_ptr<dirhandle>> dirhandles_;
bool has_redacted_cache_;
- bool has_case_insensitive_match_;
+ bool should_invalidate_;
bool deleted_;
std::recursive_mutex* lock_;
diff --git a/jni/node_test.cpp b/jni/node_test.cpp
index 8c3406e3c..6496f8084 100644
--- a/jni/node_test.cpp
+++ b/jni/node_test.cpp
@@ -32,8 +32,9 @@ class NodeTest : public ::testing::Test {
typedef std::unique_ptr<node, decltype(&NodeTest::destroy)> unique_node_ptr;
unique_node_ptr CreateNode(node* parent, const std::string& path, const int transforms = 0) {
- return unique_node_ptr(node::Create(parent, path, "", true, transforms, &lock_, &tracker_),
- &NodeTest::destroy);
+ return unique_node_ptr(
+ node::Create(parent, path, "", true, true, transforms, &lock_, &tracker_),
+ &NodeTest::destroy);
}
static class node* ForChild(class node* node, const std::string& name,
@@ -67,7 +68,7 @@ TEST_F(NodeTest, TestCreate_withParent) {
}
TEST_F(NodeTest, TestRelease) {
- node* node = node::Create(nullptr, "/path", "", true, 0, &lock_, &tracker_);
+ node* node = node::Create(nullptr, "/path", "", false, true, 0, &lock_, &tracker_);
acquire(node);
acquire(node);
ASSERT_EQ(3, GetRefCount(node));
@@ -277,10 +278,10 @@ TEST_F(NodeTest, DeleteTree) {
unique_node_ptr parent = CreateNode(nullptr, "/path");
// This is the tree that we intend to delete.
- node* child = node::Create(parent.get(), "subdir", "", true, 0, &lock_, &tracker_);
- node::Create(child, "s1", "", true, 0, &lock_, &tracker_);
- node* subchild2 = node::Create(child, "s2", "", true, 0, &lock_, &tracker_);
- node::Create(subchild2, "sc2", "", true, 0, &lock_, &tracker_);
+ node* child = node::Create(parent.get(), "subdir", "", false, true, 0, &lock_, &tracker_);
+ node::Create(child, "s1", "", false, true, 0, &lock_, &tracker_);
+ node* subchild2 = node::Create(child, "s2", "", false, true, 0, &lock_, &tracker_);
+ node::Create(subchild2, "sc2", "", false, true, 0, &lock_, &tracker_);
ASSERT_EQ(child, parent->LookupChildByName("subdir", false /* acquire */));
node::DeleteTree(child);
diff --git a/src/com/android/providers/media/FileLookupResult.java b/src/com/android/providers/media/FileLookupResult.java
index d5531fc61..4dc4d034c 100644
--- a/src/com/android/providers/media/FileLookupResult.java
+++ b/src/com/android/providers/media/FileLookupResult.java
@@ -22,12 +22,17 @@ package com.android.providers.media;
*/
public final class FileLookupResult {
public final int transforms;
+ public final int uid;
public final boolean transformsComplete;
+ public final boolean transformsSupported;
public final String ioPath;
- public FileLookupResult(int transforms, boolean transformsComplete, String ioPath) {
+ public FileLookupResult(int transforms, int uid, boolean transformsComplete,
+ boolean transformsSupported, String ioPath) {
this.transforms = transforms;
+ this.uid = uid;
this.transformsComplete = transformsComplete;
+ this.transformsSupported = transformsSupported;
this.ioPath = ioPath;
}
}
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 5685dbbe6..55b9c40e5 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -241,10 +241,8 @@ public class MediaProvider extends ContentProvider {
static final Pattern PATTERN_SELECTION_ID = Pattern.compile(
"(?:image_id|video_id)\\s*=\\s*(\\d+)");
- /** File supports transforms and uid requires transcoding */
+ /** File access by uid requires the transcoding transform */
private static final int FLAG_TRANSFORM_TRANSCODING = 1;
- /** File supports transforms */
- private static final int FLAG_TRANSFORM_SUPPORTED = 1 << 30;
/**
* These directory names aren't declared in Environment as final variables, and so we need to
@@ -1387,20 +1385,29 @@ public class MediaProvider extends ContentProvider {
* for transform lookup query for a file and uid.
*
* @param path file path to get transforms for
- * @param uid app requesting IO
+ * @param uid app requesting IO form kernel
+ * @param tid FUSE thread id handling IO request from kernel
*
* Called from JNI in jni/MediaProviderWrapper.cpp
*/
@Keep
- public FileLookupResult onFileLookupForFuse(String path, int uid) {
+ public FileLookupResult onFileLookupForFuse(String path, int uid, int tid) {
String ioPath = "";
boolean transformsComplete = true;
- int transforms = getTransformsForFuse(path, uid);
- if (transforms != 0) {
- ioPath = getIoPathForFuse(path, uid);
- transformsComplete = Objects.equals(path, ioPath);
+ boolean transformsSupported = mTranscodeHelper.supportsTranscode(path);
+ int transforms = 0;
+
+ if (transformsSupported) {
+ // TODO(b/170974147): Avoid duplicate shouldTranscode calls in getTransformsForFuse and
+ // getIoPathForFuse
+ transforms = getTransformsForFuse(path, uid);
+ if (transforms != 0) {
+ ioPath = getIoPathForFuse(path, uid);
+ transformsComplete = false;
+ }
}
- return new FileLookupResult(transforms, transformsComplete, ioPath);
+ return new FileLookupResult(transforms, uid, transformsComplete, transformsSupported,
+ ioPath);
}
/**
@@ -1430,15 +1437,10 @@ public class MediaProvider extends ContentProvider {
* @see {@link transformForFuse}
*/
private int getTransformsForFuse(String path, int uid) {
- int result = 0;
- if (mTranscodeHelper.supportsTranscode(path)) {
- result |= FLAG_TRANSFORM_SUPPORTED;
-
- if (mTranscodeHelper.shouldTranscode(path, uid, null /* bundle */)) {
- result |= FLAG_TRANSFORM_TRANSCODING;
- }
+ if (mTranscodeHelper.shouldTranscode(path, uid, null /* bundle */)) {
+ return FLAG_TRANSFORM_TRANSCODING;
}
- return result;
+ return 0;
}
/**
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-20 06:01:12 +0000