From owner-svn-src-head@freebsd.org Wed Aug 14 20:45:01 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 60F3DBA879; Wed, 14 Aug 2019 20:45:01 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4681mY1xB3z3wcm; Wed, 14 Aug 2019 20:45:01 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 2070F5B6F; Wed, 14 Aug 2019 20:45:01 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x7EKj0Y3086050; Wed, 14 Aug 2019 20:45:00 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x7EKj0eu086047; Wed, 14 Aug 2019 20:45:00 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201908142045.x7EKj0eu086047@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Wed, 14 Aug 2019 20:45:00 +0000 (UTC) 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 X-SVN-Group: head X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in head: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Commit-Revision: 351042 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Aug 2019 20:45:01 -0000 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 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) { 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&);