diff options
author | 2020-06-09 11:50:33 +0100 | |
---|---|---|
committer | 2020-06-11 15:45:04 +0000 | |
commit | 0852ba7d97970fb07c0090ebb56746639e0668e5 (patch) | |
tree | b6e06cc44d01d75e3e87559be354f79865998d6c /jni/node-inl.h | |
parent | 4db814d5ee48904d827b276160b04117f9b42e9d (diff) |
Reuse nodes for case-insensitive lookups
Previously, node lookups were case-sensitive. This meant that if the
kernel did a lookup for foo and FOO, the FuseDaemon will create two
different nodes.
This led to some unexpected behavior when doing IO with
case-insensitive files.
1. The kernel may have a dentry left in its cache even after an unlink
2. It may use different page caches when two case-insensitive matching
files are open at the same time.
Previous attempts at fixing this,
Id45f2fe398e962ca0b16f41f628ad2fc5d849481 explicitly invalidated
case-insensitive dentries but still created a new FUSE (userspace)
node. Unfortunately, this fixed only (1) and had resulted in a
segfault, b/158313907 introduced in
Ic7613d1b5325bf7466cd94fe6c713bc55aeacd79.
Now, we fix all issues by reusing nodes. So there is only one
userspace node per case-insensitive match. This causes the kernel to
reuse inodes for case-insensitive matches even though it still has two
different dentries. This fixes (2) and we still rely on explicit
invalidations to keep (1) fixed.
Refcounting while reusing nodes should be correct because:
ref(foo{kernel}) + ref(FOO{kernel}) = ref{foo{userspace}}
And since we invalidate dentries in the kernel, at the end of a
case-insensitive lookup, the userspace refcounts should still match
the refcount of the one dentry in the kernel.
Test: ScopedStorageTest#testCaseInsensitivity
Bug: 158313907
Bug: 157676443
Change-Id: I83ffd660bc3d6843901e838a53eacec6883fef52
Diffstat (limited to 'jni/node-inl.h')
-rw-r--r-- | jni/node-inl.h | 25 |
1 files changed, 3 insertions, 22 deletions
diff --git a/jni/node-inl.h b/jni/node-inl.h index c7d30df93..d6ad0ad32 100644 --- a/jni/node-inl.h +++ b/jni/node-inl.h @@ -175,13 +175,10 @@ class node { node* LookupChildByName(const std::string& name, bool acquire) const { std::lock_guard<std::recursive_mutex> guard(*lock_); + const char* name_char = name.c_str(); for (node* child : children_) { - // Use exact string comparison, nodes that differ by case - // must be considered distinct even if they refer to the same - // underlying file as otherwise operations such as "mv x x" - // will not work because the source and target nodes are the same. - - if ((name == child->name_) && !child->deleted_) { + const std::string& child_name = child->GetName(); + if (!strcasecmp(name_char, child_name.c_str()) && !child->deleted_) { if (acquire) { child->Acquire(); } @@ -221,22 +218,6 @@ class node { return parent_; } - std::vector<std::string> MatchChildrenCaseInsensitive(const std::string& name) const { - std::lock_guard<std::recursive_mutex> guard(*lock_); - - const char* name_char = name.c_str(); - std::vector<std::string> matches; - - for (node* child : children_) { - const std::string& child_name = child->GetName(); - if (!strcasecmp(name_char, child_name.c_str())) { - matches.push_back(child_name); - } - } - - return matches; - } - inline void AddHandle(handle* h) { std::lock_guard<std::recursive_mutex> guard(*lock_); handles_.emplace_back(std::unique_ptr<handle>(h)); |