Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 Aug 2019 20:45:00 +0000 (UTC)
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r351042 - in head: sys/fs/fuse tests/sys/fs/fusefs
Message-ID:  <201908142045.x7EKj0eu086047@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: asomers
Date: Wed Aug 14 20:45:00 2019
New Revision: 351042
URL: https://svnweb.freebsd.org/changeset/base/351042

Log:
  fusefs: Fix the size of fuse_getattr_in
  
  In FUSE protocol 7.9, the size of the FUSE_GETATTR request has increased.
  However, the fusefs driver is currently not sending the additional fields.
  In our implementation, the additional fields are always zero, so I there
  haven't been any test failures until now.  But fusefs-lkl requires the
  request's length to be correct.
  
  Fix this bug, and also enhance the test suite to catch similar bugs.
  
  PR:		239830
  MFC after:	2 weeks
  MFC-With:	350665
  Sponsored by:	The FreeBSD Foundation

Modified:
  head/sys/fs/fuse/fuse_internal.c
  head/tests/sys/fs/fusefs/getattr.cc
  head/tests/sys/fs/fusefs/mockfs.cc
  head/tests/sys/fs/fusefs/mockfs.hh

Modified: head/sys/fs/fuse/fuse_internal.c
==============================================================================
--- head/sys/fs/fuse/fuse_internal.c	Wed Aug 14 19:21:26 2019	(r351041)
+++ head/sys/fs/fuse/fuse_internal.c	Wed Aug 14 20:45:00 2019	(r351042)
@@ -868,7 +868,7 @@ fuse_internal_do_getattr(struct vnode *vp, struct vatt
 	enum vtype vtyp;
 	int err;
 
-	fdisp_init(&fdi, 0);
+	fdisp_init(&fdi, sizeof(*fgai));
 	fdisp_make_vp(&fdi, FUSE_GETATTR, vp, td, cred);
 	fgai = fdi.indata;
 	/* 
@@ -877,7 +877,7 @@ fuse_internal_do_getattr(struct vnode *vp, struct vatt
 	 * care.
 	 */
 	fgai->getattr_flags = 0;
-	if ((err = fdisp_simple_putget_vp(&fdi, FUSE_GETATTR, vp, td, cred))) {
+	if ((err = fdisp_wait_answ(&fdi))) {
 		if (err == ENOENT)
 			fuse_internal_vnode_disappear(vp);
 		goto out;

Modified: head/tests/sys/fs/fusefs/getattr.cc
==============================================================================
--- head/tests/sys/fs/fusefs/getattr.cc	Wed Aug 14 19:21:26 2019	(r351041)
+++ head/tests/sys/fs/fusefs/getattr.cc	Wed Aug 14 20:45:00 2019	(r351042)
@@ -195,6 +195,7 @@ TEST_F(Getattr, ok)
 	EXPECT_CALL(*m_mock, process(
 		ResultOf([](auto in) {
 			return (in.header.opcode == FUSE_GETATTR &&
+				in.body.getattr.getattr_flags == 0 &&
 				in.header.nodeid == ino);
 		}, Eq(true)),
 		_)

Modified: head/tests/sys/fs/fusefs/mockfs.cc
==============================================================================
--- head/tests/sys/fs/fusefs/mockfs.cc	Wed Aug 14 19:21:26 2019	(r351041)
+++ head/tests/sys/fs/fusefs/mockfs.cc	Wed Aug 14 20:45:00 2019	(r351042)
@@ -467,6 +467,156 @@ MockFS::~MockFS() {
 		close(m_kq);
 }
 
+void MockFS::audit_request(const mockfs_buf_in &in) {
+	uint32_t inlen = in.header.len;
+	size_t fih = sizeof(in.header);
+	switch (in.header.opcode) {
+	case FUSE_LOOKUP:
+	case FUSE_RMDIR:
+	case FUSE_SYMLINK:
+	case FUSE_UNLINK:
+		ASSERT_GT(inlen, fih) << "Missing request filename";
+		break;
+	case FUSE_FORGET:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.forget));
+		break;
+	case FUSE_GETATTR:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.getattr));
+		break;
+	case FUSE_SETATTR:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.setattr));
+		break;
+	case FUSE_READLINK:
+		ASSERT_EQ(inlen, fih) << "Unexpected request body";
+		break;
+	case FUSE_MKNOD:
+		{
+			size_t s;
+			if (m_kernel_minor_version >= 12)
+				s = sizeof(in.body.mknod);
+			else
+				s = FUSE_COMPAT_MKNOD_IN_SIZE;
+			ASSERT_GE(inlen, fih + s) << "Missing request body";
+			ASSERT_GT(inlen, fih + s) << "Missing request filename";
+			break;
+		}
+	case FUSE_MKDIR:
+		ASSERT_GE(inlen, fih + sizeof(in.body.mkdir)) <<
+			"Missing request body";
+		ASSERT_GT(inlen, fih + sizeof(in.body.mkdir)) <<
+			"Missing request filename";
+		break;
+	case FUSE_RENAME:
+		ASSERT_GE(inlen, fih + sizeof(in.body.rename)) <<
+			"Missing request body";
+		ASSERT_GT(inlen, fih + sizeof(in.body.rename)) <<
+			"Missing request filename";
+		break;
+	case FUSE_LINK:
+		ASSERT_GE(inlen, fih + sizeof(in.body.link)) <<
+			"Missing request body";
+		ASSERT_GT(inlen, fih + sizeof(in.body.link)) <<
+			"Missing request filename";
+		break;
+	case FUSE_OPEN:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.open));
+		break;
+	case FUSE_READ:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.read));
+		break;
+	case FUSE_WRITE:
+		{
+			size_t s;
+
+			if (m_kernel_minor_version >= 9)
+				s = sizeof(in.body.write);
+			else
+				s = FUSE_COMPAT_WRITE_IN_SIZE;
+			ASSERT_GE(inlen, fih + s) << "Missing request body";
+			// I suppose a 0-byte write should be allowed
+			break;
+		}
+	case FUSE_DESTROY:
+	case FUSE_STATFS:
+		ASSERT_EQ(inlen, fih);
+		break;
+	case FUSE_RELEASE:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.release));
+		break;
+	case FUSE_FSYNC:
+	case FUSE_FSYNCDIR:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.fsync));
+		break;
+	case FUSE_SETXATTR:
+		ASSERT_GE(inlen, fih + sizeof(in.body.setxattr)) <<
+			"Missing request body";
+		ASSERT_GT(inlen, fih + sizeof(in.body.setxattr)) <<
+			"Missing request attribute name";
+		break;
+	case FUSE_GETXATTR:
+		ASSERT_GE(inlen, fih + sizeof(in.body.setxattr)) <<
+			"Missing request body";
+		ASSERT_GT(inlen, fih + sizeof(in.body.setxattr)) <<
+			"Missing request attribute name";
+		break;
+	case FUSE_LISTXATTR:
+		ASSERT_GE(inlen, fih + sizeof(in.body.listxattr)) <<
+			"Missing request body";
+		ASSERT_GT(inlen, fih + sizeof(in.body.listxattr)) <<
+			"Missing namespace";
+		break;
+	case FUSE_REMOVEXATTR:
+		ASSERT_GT(inlen, fih) << "Missing request attribute name";
+		break;
+	case FUSE_FLUSH:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.flush));
+		break;
+	case FUSE_INIT:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.init));
+		break;
+	case FUSE_OPENDIR:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.opendir));
+		break;
+	case FUSE_READDIR:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.readdir));
+		break;
+	case FUSE_RELEASEDIR:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.releasedir));
+		break;
+	case FUSE_GETLK:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.getlk));
+		break;
+	case FUSE_SETLK:
+	case FUSE_SETLKW:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.setlk));
+		break;
+	case FUSE_ACCESS:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.access));
+		break;
+	case FUSE_CREATE:
+		ASSERT_GE(inlen, fih + sizeof(in.body.create)) <<
+			"Missing request body";
+		ASSERT_GT(inlen, fih + sizeof(in.body.create)) <<
+			"Missing request filename";
+		break;
+	case FUSE_INTERRUPT:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.interrupt));
+		break;
+	case FUSE_BMAP:
+		ASSERT_EQ(inlen, fih + sizeof(in.body.bmap));
+		break;
+	case FUSE_NOTIFY_REPLY:
+	case FUSE_BATCH_FORGET:
+	case FUSE_FALLOCATE:
+	case FUSE_IOCTL:
+	case FUSE_POLL:
+	case FUSE_READDIRPLUS:
+		FAIL() << "Unsupported opcode?";
+	default:
+		FAIL() << "Unknown opcode " << in.header.opcode;
+	}
+}
+
 void MockFS::init(uint32_t flags) {
 	std::unique_ptr<mockfs_buf_in> in(new mockfs_buf_in);
 	std::unique_ptr<mockfs_buf_out> out(new mockfs_buf_out);
@@ -515,6 +665,7 @@ void MockFS::loop() {
 			break;
 		if (verbosity > 0)
 			debug_request(*in);
+		audit_request(*in);
 		if (pid_ok((pid_t)in->header.pid)) {
 			process(*in, out);
 		} else {
@@ -686,6 +837,12 @@ void MockFS::read_request(mockfs_buf_in &in) {
 		m_quit = true;
 	}
 	ASSERT_TRUE(res >= static_cast<ssize_t>(sizeof(in.header)) || m_quit);
+	/*
+	 * Inconsistently, fuse_in_header.len is the size of the entire
+	 * request,including header, even though fuse_out_header.len excludes
+	 * the size of the header.
+	 */
+	ASSERT_TRUE(res == in.header.len || m_quit);
 }
 
 void MockFS::write_response(const mockfs_buf_out &out) {

Modified: head/tests/sys/fs/fusefs/mockfs.hh
==============================================================================
--- head/tests/sys/fs/fusefs/mockfs.hh	Wed Aug 14 19:21:26 2019	(r351041)
+++ head/tests/sys/fs/fusefs/mockfs.hh	Wed Aug 14 20:45:00 2019	(r351042)
@@ -145,6 +145,7 @@ union fuse_payloads_in {
 	fuse_fsync_in	fsync;
 	fuse_fsync_in	fsyncdir;
 	fuse_forget_in	forget;
+	fuse_getattr_in	getattr;
 	fuse_interrupt_in interrupt;
 	fuse_lk_in	getlk;
 	fuse_getxattr_in getxattr;
@@ -282,6 +283,7 @@ class MockFS {
 	/* Timestamp granularity in nanoseconds */
 	unsigned m_time_gran;
 
+	void audit_request(const mockfs_buf_in &in);
 	void debug_request(const mockfs_buf_in&);
 	void debug_response(const mockfs_buf_out&);
 



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