From bf507497734c43167087bff79100377c3f74e058 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 14 Aug 2019 20:45:00 +0000 Subject: [PATCH] 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 --- sys/fs/fuse/fuse_internal.c | 4 +- tests/sys/fs/fusefs/getattr.cc | 1 + tests/sys/fs/fusefs/mockfs.cc | 157 +++++++++++++++++++++++++++++++++ tests/sys/fs/fusefs/mockfs.hh | 2 + 4 files changed, 162 insertions(+), 2 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index 5301e4cf9cf5..d201c3be5b3b 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -868,7 +868,7 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap, 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 vattr *vap, * 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; diff --git a/tests/sys/fs/fusefs/getattr.cc b/tests/sys/fs/fusefs/getattr.cc index 5319a4f88d3c..b88381bd28e7 100644 --- a/tests/sys/fs/fusefs/getattr.cc +++ b/tests/sys/fs/fusefs/getattr.cc @@ -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)), _) diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc index b8ab7d28f998..ff30105b3e09 100644 --- a/tests/sys/fs/fusefs/mockfs.cc +++ b/tests/sys/fs/fusefs/mockfs.cc @@ -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 in(new mockfs_buf_in); std::unique_ptr 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(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) { diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh index f340b6e86f14..d4d5cac7c5c7 100644 --- a/tests/sys/fs/fusefs/mockfs.hh +++ b/tests/sys/fs/fusefs/mockfs.hh @@ -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&);