Package: fusedav
Version: 0.2-3
Severity: normal
Tags: upstream patch
This is a two-part fix
Subject: [PATCH 12/13] implement dav_flush to avoid race from dav_release
dav_release unlinks a file from the cache before syncing,
allowing a concurrent thread to open a new cached entry
against stale file data. So ensure we implement dav_flush
as libfuse2 will always flush before release.
Subject: [PATCH 13/13] fix release() with multiple open() to the same path
dav_release() should not unlink a file from the cache until its
ref count drops to zero. The ref count may be non-zero after
release because because dav_open() may be called multiple times
without a dav_release() in between, so relying on a boolean dead
flag is problematic.
Instead of relying on a boolean dead flag, rely on the ref count
of the file and only unlink it when the counter drops to zero.
Btw, all my patches are available in my git repository at
git://bogomips.org/fusedav.git
I have made the "bugfix-only" branch for things I consider bugfixes
(including Sebastian's existing patches in Debian). I also have "home"
for whatever I'm running at home (which may have more experimental
changes).
>From ad701acac54f8dcc60b697332c1a3ca1971cbc63 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalper...@yhbt.net>
Date: Thu, 1 Nov 2012 23:24:36 +0000
Subject: [PATCH 12/13] implement dav_flush to avoid race from dav_release
dav_release unlinks a file from the cache before syncing,
allowing a concurrent thread to open a new cached entry
against stale file data. So ensure we implement dav_flush
as libfuse2 will always flush before release.
---
src/fusedav.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/fusedav.c b/src/fusedav.c
index 6a92799..69fed1b 100644
--- a/src/fusedav.c
+++ b/src/fusedav.c
@@ -549,7 +549,7 @@ finish:
return r;
}
-static int dav_fsync(const char *path, __unused int isdatasync, __unused
struct fuse_file_info *info) {
+static int flush_internal(const char *fn, const char *path, __unused struct
fuse_file_info *fi) {
void *f = NULL;
int r = 0;
ne_session *session;
@@ -558,7 +558,7 @@ static int dav_fsync(const char *path, __unused int
isdatasync, __unused struct
path = path_cvt(path);
if (debug)
- fprintf(stderr, "fsync(%s)\n", path);
+ fprintf(stderr, "%s(%s)\n", fn, path);
if (!(session = session_get(1))) {
r = -EIO;
@@ -566,7 +566,7 @@ static int dav_fsync(const char *path, __unused int
isdatasync, __unused struct
}
if (!(f = file_cache_get(path))) {
- fprintf(stderr, "fsync() called for closed file\n");
+ fprintf(stderr, "%s() called for closed file\n", fn);
r = -EFAULT;
goto finish;
}
@@ -590,6 +590,14 @@ finish:
return r;
}
+static int dav_flush(const char *path, struct fuse_file_info *fi) {
+ return flush_internal("flush", path, fi);
+}
+
+static int dav_fsync(const char *path, __unused int isdatasync, struct
fuse_file_info *fi) {
+ return flush_internal("fsync", path, fi);
+}
+
static int dav_mknod(const char *path, mode_t mode, __unused dev_t rdev) {
char tempfile[PATH_MAX];
int fd;
@@ -1167,6 +1175,7 @@ static struct fuse_operations dav_oper = {
.chmod = dav_chmod,
.truncate = dav_truncate,
.utime = dav_utime,
+ .flush = dav_flush,
.open = dav_open,
.read = dav_read,
.write = dav_write,
--
1.8.0.3.gdd57fab.dirty
>From 6fb7e3cf48c1cd4d07c4807e2ff3bd0fd6f14fb9 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalper...@yhbt.net>
Date: Fri, 2 Nov 2012 23:56:24 +0000
Subject: [PATCH 13/13] fix release() with multiple open() to the same path
dav_release() should not unlink a file from the cache until its
ref count drops to zero. The ref count may be non-zero after
release because because dav_open() may be called multiple times
without a dav_release() in between, so relying on a boolean dead
flag is problematic.
Instead of relying on a boolean dead flag, rely on the ref count
of the file and only unlink it when the counter drops to zero.
---
src/filecache.c | 44 ++++++++++++++++++--------------------------
src/filecache.h | 2 --
src/fusedav.c | 13 +++++--------
3 files changed, 23 insertions(+), 36 deletions(-)
diff --git a/src/filecache.c b/src/filecache.c
index f291e15..eded0aa 100644
--- a/src/filecache.c
+++ b/src/filecache.c
@@ -57,7 +57,7 @@ struct file_info {
int modified;
- int ref, dead;
+ int ref;
pthread_mutex_t mutex;
@@ -68,6 +68,7 @@ struct file_info {
static struct file_info *files = NULL;
static pthread_mutex_t files_mutex = PTHREAD_MUTEX_INITIALIZER;
+static void file_cache_unlink(struct file_info *fi);
static int file_cache_sync_unlocked(struct file_info *fi);
static void* file_cache_get_unlocked(const char *path) {
@@ -76,7 +77,7 @@ static void* file_cache_get_unlocked(const char *path) {
for (f = files; f; f = f->next) {
pthread_mutex_lock(&f->mutex);
- if (!f->dead && f->filename && !strcmp(path, f->filename)) {
+ if (f->ref && f->filename && !strcmp(path, f->filename)) {
f->ref++;
r = f;
}
@@ -100,7 +101,7 @@ void* file_cache_get(const char *path) {
}
static void file_cache_free_unlocked(struct file_info *fi) {
- assert(fi && fi->dead && fi->ref == 0);
+ assert(fi && fi->ref == 0);
free(fi->filename);
@@ -113,6 +114,7 @@ static void file_cache_free_unlocked(struct file_info *fi) {
void file_cache_unref(void *f) {
struct file_info *fi = f;
+ int unlinked = 0;
assert(fi);
pthread_mutex_lock(&fi->mutex);
@@ -120,22 +122,29 @@ void file_cache_unref(void *f) {
assert(fi->ref >= 1);
fi->ref--;
- if (!fi->ref && fi->dead) {
- file_cache_sync_unlocked(fi);
- file_cache_free_unlocked(fi);
+ if (fi->ref == 0) {
+ if (fi->writable)
+ file_cache_sync_unlocked(fi);
+ file_cache_unlink(fi);
+ unlinked = 1;
}
pthread_mutex_unlock(&fi->mutex);
+
+ if (unlinked)
+ file_cache_free_unlocked(fi);
}
static void file_cache_unlink(struct file_info *fi) {
struct file_info *s, *prev;
+ int found = 0;
assert(fi);
pthread_mutex_lock(&files_mutex);
for (s = files, prev = NULL; s; s = s->next) {
if (s == fi) {
+ found = 1;
if (prev)
prev->next = s->next;
else
@@ -148,20 +157,9 @@ static void file_cache_unlink(struct file_info *fi) {
}
pthread_mutex_unlock(&files_mutex);
-}
-
-int file_cache_close(void *f) {
- struct file_info *fi = f;
- int r = 0;
- assert(fi);
- file_cache_unlink(f);
-
- pthread_mutex_lock(&fi->mutex);
- fi->dead = 1;
- pthread_mutex_unlock(&fi->mutex);
-
- return r;
+ if (!found)
+ fprintf(stderr, "file_cache_unlink(%s) failed", fi->filename);
}
static void file_cache_update_flags_unlocked(struct file_info *fi, int flags)
@@ -241,7 +239,6 @@ fail:
if (fi) {
pthread_mutex_unlock(&fi->mutex);
- fi->dead = 1;
file_cache_free_unlocked(fi);
}
@@ -415,7 +412,7 @@ int file_cache_sync(void *f) {
assert(fi);
pthread_mutex_lock(&fi->mutex);
- r = file_cache_sync_unlocked(fi);
+ r = fi->writable ? file_cache_sync_unlocked(fi) : 0;
pthread_mutex_unlock(&fi->mutex);
return r;
@@ -429,12 +426,7 @@ int file_cache_close_all(void) {
while (files) {
struct file_info *fi = files;
- pthread_mutex_lock(&fi->mutex);
- fi->ref++;
pthread_mutex_unlock(&fi->mutex);
-
- pthread_mutex_unlock(&files_mutex);
- file_cache_close(fi);
file_cache_unref(fi);
pthread_mutex_lock(&files_mutex);
}
diff --git a/src/filecache.h b/src/filecache.h
index c3845ca..7e217e2 100644
--- a/src/filecache.h
+++ b/src/filecache.h
@@ -29,8 +29,6 @@ void* file_cache_open(const char *path, int flags);
void* file_cache_get(const char *path);
void file_cache_unref(void *f);
-int file_cache_close(void *f);
-
int file_cache_read(void *f, char *buf, size_t size, off_t offset);
int file_cache_write(void *f, const char *buf, size_t size, off_t offset);
int file_cache_truncate(void *f, off_t s);
diff --git a/src/fusedav.c b/src/fusedav.c
index 69fed1b..be6d264 100644
--- a/src/fusedav.c
+++ b/src/fusedav.c
@@ -537,14 +537,13 @@ static int dav_release(const char *path, __unused struct
fuse_file_info *info) {
goto finish;
}
- if (file_cache_close(f) < 0) {
- r = -errno;
- goto finish;
- }
+ /* reverse the ref increment from file_cache_get() */
+ file_cache_unref(f);
+
+ /* reverse the ref increment from dav_open() */
+ file_cache_unref(f);
finish:
- if (f)
- file_cache_unref(f);
return r;
}
@@ -648,8 +647,6 @@ static int dav_open(const char *path, struct fuse_file_info
*info) {
if (!(f = file_cache_open(path, info->flags)))
return -errno;
- file_cache_unref(f);
-
return 0;
}
--
1.8.0.3.gdd57fab.dirty
