jabberwocky238/libfuse
1
0
Fork 0
mirror of https://github.com/libfuse/libfuse.git synced 2026-07-07 14:47:41 +08:00
Code Issues Packages Projects Releases Wiki Activity

example/passthrough_hp: fix race between forget_one and do_lookup

Browse Source 
During fsstress stress testing using passthrough_hp as the backend, the
backend process crashes. The root cause is that when forget_one() and
do_lookup() concurrently process the same inode, do_lookup may return
either an invalid inode or a different inode reusing the same memory
address.

 CPU0                           CPU1
 -----------------------        --------------------
 forget_one                     do_lookup
                                  lock fs
                                  inode = fs.inodes[id] //inode.fd > 0
                                  unlock fs
   lock inode
   inode.nlookup -= n
                 <inode.nlookup equal to 0>
   lock fs
   unlock inode
   fs.inodes.erase
   unlock fs
                                 lock inode
                                 inode.nlookup++
                                 unlock inode
                 <lookup a invalid inode>

This can lead to abnormalities in the inode nlookup count. Since the
value of inode.nlookup determines the inode's lifecycle, and considering
the locking order requirements between the inode lock and fs lock, using
the inode lock alone does not resolve the issue effectively. The fix is
to convert inode.nlookup to an atomic type, which removes the need for
write protection via inode lock, while using fs lock to guard the inode's
lifetime.

Signed-off-by: Long Li <leo.lilong@huawei.com>
This commit is contained in:
Long Li
2025-11-22 22:57:23 +08:00
committed by Bernd Schubert
parent 8ad076e458
commit 1e19235c54
1 changed files with 6 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
example/passthrough_hp.cc
+6 -10
View File
@@ -78,6 +78,7 @@
#include "cxxopts.hpp"
#include <mutex>
#include <syslog.h>
#include <atomic>
#include "passthrough_helpers.h"
@@ -130,7 +131,7 @@ struct Inode {
int generation{ 0 };
int backing_id{ 0 };
uint64_t nopen{ 0 };
uint64_t nlookup{ 0 };
std::atomic<uint64_t> nlookup{ 0 };
std::mutex m;
// Delete copy constructor and assignments. We could implement
@@ -411,12 +412,10 @@ static int do_lookup(fuse_ino_t parent, const char *name, fuse_entry_param *e)
}
if (inode.fd > 0) { // found existing inode
fs_lock.unlock();
if (fs.debug)
cerr << "DEBUG: lookup(): inode " << e->attr.st_ino
<< " (userspace) already known; fd = " << inode.fd
<< endl;
lock_guard<mutex> g{ inode.m };
inode.nlookup++;
if (fs.debug)
@@ -424,6 +423,7 @@ static int do_lookup(fuse_ino_t parent, const char *name, fuse_entry_param *e)
<< "inode " << inode.src_ino << " count "
<< inode.nlookup << endl;
fs_lock.unlock();
close(newfd);
} else { // no existing inode
/* This is just here to make Helgrind happy. It violates the
@@ -548,7 +548,6 @@ static void sfs_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent,
}
e.ino = reinterpret_cast<fuse_ino_t>(&inode);
{
lock_guard<mutex> g{ inode.m };
inode.nlookup++;
if (fs.debug)
cerr << "DEBUG:" << __func__ << ":" << __LINE__ << " "
@@ -621,7 +620,6 @@ static void sfs_unlink(fuse_req_t req, fuse_ino_t parent, const char *name)
static void forget_one(fuse_ino_t ino, uint64_t n)
{
Inode &inode = get_inode(ino);
unique_lock<mutex> l{ inode.m };
if (n > inode.nlookup) {
cerr << "INTERNAL ERROR: Negative lookup count for inode "
@@ -636,12 +634,10 @@ static void forget_one(fuse_ino_t ino, uint64_t n)
<< endl;
if (!inode.nlookup) {
if (fs.debug)
lock_guard<mutex> g_fs{ fs.mutex };
if (!inode.nlookup) {
cerr << "DEBUG: forget: cleaning up inode "
<< inode.src_ino << endl;
{
lock_guard<mutex> g_fs{ fs.mutex };
l.unlock();
<< inode.src_ino << endl;
fs.inodes.erase({ inode.src_ino, inode.src_dev });
}
} else if (fs.debug)