Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 Jul 2021 17:33:53 GMT
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 46138f337bd8 - stable/12 - fusefs: ensure that FUSE ops' headers' unique values are actually unique
Message-ID:  <202107271733.16RHXr9Z037487@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=46138f337bd803f6546d0eae8b9b0f3879ca3224

commit 46138f337bd803f6546d0eae8b9b0f3879ca3224
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-06-18 00:04:59 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-07-27 17:15:40 +0000

    fusefs: ensure that FUSE ops' headers' unique values are actually unique
    
    Every FUSE operation has a unique value in its header.  As the name
    implies, these values are supposed to be unique among all outstanding
    operations.  And since FUSE_INTERRUPT is asynchronous and racy, it is
    desirable that the unique values be unique among all operations that are
    "close in time".
    
    Ensure that they are actually unique by incrementing them whenever we
    reuse a fuse_dispatcher object, for example during fsync, write, and
    listextattr.
    
    PR:             244686
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D30810
    
    (cherry picked from commit 5403f2c163f7e3d1adb9431d216f88d57cf9d74b)
---
 sys/fs/fuse/fuse_ipc.c        | 34 ++++++++++++++--------------------
 tests/sys/fs/fusefs/mockfs.cc |  9 +++++++++
 tests/sys/fs/fusefs/mockfs.hh |  3 +++
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/sys/fs/fuse/fuse_ipc.c b/sys/fs/fuse/fuse_ipc.c
index 73378dee959a..d871b7455450 100644
--- a/sys/fs/fuse/fuse_ipc.c
+++ b/sys/fs/fuse/fuse_ipc.c
@@ -103,6 +103,7 @@ static void fdisp_make_pid(struct fuse_dispatcher *fdip, enum fuse_opcode op,
 static void fuse_interrupt_send(struct fuse_ticket *otick, int err);
 static struct fuse_ticket *fticket_alloc(struct fuse_data *data);
 static void fticket_refresh(struct fuse_ticket *ftick);
+static inline void fticket_reset(struct fuse_ticket *ftick);
 static void fticket_destroy(struct fuse_ticket *ftick);
 static int fticket_wait_answer(struct fuse_ticket *ftick);
 static inline int 
@@ -319,20 +320,12 @@ fticket_ctor(void *mem, int size, void *arg, int flags)
 	FUSE_ASSERT_AW_DONE(ftick);
 
 	ftick->tk_data = data;
-
-	if (ftick->tk_unique != 0)
-		fticket_refresh(ftick);
-
-	/* May be truncated to 32 bits */
-	ftick->tk_unique = atomic_fetchadd_long(&data->ticketer, 1);
-	if (ftick->tk_unique == 0)
-		ftick->tk_unique = atomic_fetchadd_long(&data->ticketer, 1);
-
 	ftick->irq_unique = 0;
-
 	refcount_init(&ftick->tk_refcount, 1);
 	counter_u64_add(fuse_ticket_count, 1);
 
+	fticket_refresh(ftick);
+
 	return 0;
 }
 
@@ -386,26 +379,22 @@ fticket_destroy(struct fuse_ticket *ftick)
 	return uma_zfree(ticket_zone, ftick);
 }
 
-static inline
-void
+/* Prepare the ticket to be reused and clear its data buffers */
+static inline void
 fticket_refresh(struct fuse_ticket *ftick)
 {
-	FUSE_ASSERT_MS_DONE(ftick);
-	FUSE_ASSERT_AW_DONE(ftick);
+	fticket_reset(ftick);
 
 	fiov_refresh(&ftick->tk_ms_fiov);
-
-	bzero(&ftick->tk_aw_ohead, sizeof(struct fuse_out_header));
-
 	fiov_refresh(&ftick->tk_aw_fiov);
-	ftick->tk_aw_errno = 0;
-	ftick->tk_flag = 0;
 }
 
-/* Prepar the ticket to be reused, but don't clear its data buffers */
+/* Prepare the ticket to be reused, but don't clear its data buffers */
 static inline void
 fticket_reset(struct fuse_ticket *ftick)
 {
+	struct fuse_data *data = ftick->tk_data;
+
 	FUSE_ASSERT_MS_DONE(ftick);
 	FUSE_ASSERT_AW_DONE(ftick);
 
@@ -413,6 +402,11 @@ fticket_reset(struct fuse_ticket *ftick)
 
 	ftick->tk_aw_errno = 0;
 	ftick->tk_flag = 0;
+
+	/* May be truncated to 32 bits on LP32 arches */
+	ftick->tk_unique = atomic_fetchadd_long(&data->ticketer, 1);
+	if (ftick->tk_unique == 0)
+		ftick->tk_unique = atomic_fetchadd_long(&data->ticketer, 1);
 }
 
 static int
diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc
index da634b5f15cd..4effecc1cd82 100644
--- a/tests/sys/fs/fusefs/mockfs.cc
+++ b/tests/sys/fs/fusefs/mockfs.cc
@@ -370,6 +370,7 @@ MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions,
 	m_pm = pm;
 	m_time_gran = time_gran;
 	m_quit = false;
+	m_last_unique = 0;
 	if (m_pm == KQ)
 		m_kq = kqueue();
 	else
@@ -646,6 +647,14 @@ void MockFS::audit_request(const mockfs_buf_in &in, ssize_t buflen) {
 	default:
 		FAIL() << "Unknown opcode " << in.header.opcode;
 	}
+	/*
+	 * Check that the ticket's unique value is sequential.  Technically it
+	 * doesn't need to be sequential, merely unique.  But the current
+	 * fusefs driver _does_ make it sequential, and that's easy to check
+	 * for.
+	 */
+	if (in.header.unique != ++m_last_unique)
+		FAIL() << "Non-sequential unique value";
 }
 
 void MockFS::init(uint32_t flags) {
diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh
index 4cf6daea7e76..be8a05c344fa 100644
--- a/tests/sys/fs/fusefs/mockfs.hh
+++ b/tests/sys/fs/fusefs/mockfs.hh
@@ -277,6 +277,9 @@ class MockFS {
 	/* pid of the test process */
 	pid_t m_pid;
 
+	/* The unique value of the header of the last received operation */
+	uint64_t m_last_unique;
+
 	/* Method the daemon should use for I/O to and from /dev/fuse */
 	enum poll_method m_pm;
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202107271733.16RHXr9Z037487>