From owner-svn-src-all@freebsd.org Wed Sep 11 19:29:41 2019 Return-Path: Delivered-To: svn-src-all@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 A1E74DDC36; Wed, 11 Sep 2019 19:29:41 +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 46TBmj3LBfz4L7q; Wed, 11 Sep 2019 19:29:41 +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 3E020B285; Wed, 11 Sep 2019 19:29:41 +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 x8BJTfdf080647; Wed, 11 Sep 2019 19:29:41 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x8BJTeV2080574; Wed, 11 Sep 2019 19:29:40 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201909111929.x8BJTeV2080574@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Wed, 11 Sep 2019 19:29:40 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r352230 - 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: 352230 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Sep 2019 19:29:41 -0000 Author: asomers Date: Wed Sep 11 19:29:40 2019 New Revision: 352230 URL: https://svnweb.freebsd.org/changeset/base/352230 Log: fusefs: Fix iosize for FUSE_WRITE in 7.8 compat mode When communicating with a FUSE server that implements version 7.8 (or older) of the FUSE protocol, the FUSE_WRITE request structure is 16 bytes shorter than normal. The protocol version check wasn't applied universally, leading to an extra 16 bytes being sent to such servers. The extra bytes were allocated and bzero()d, so there was no information disclosure. Reviewed by: emaste MFC after: 3 days MFC-With: r350665 Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D21557 Modified: head/sys/fs/fuse/fuse_io.c head/tests/sys/fs/fusefs/mockfs.cc head/tests/sys/fs/fusefs/mockfs.hh Modified: head/sys/fs/fuse/fuse_io.c ============================================================================== --- head/sys/fs/fuse/fuse_io.c Wed Sep 11 18:54:45 2019 (r352229) +++ head/sys/fs/fuse/fuse_io.c Wed Sep 11 19:29:40 2019 (r352230) @@ -555,9 +555,17 @@ fuse_write_directbackend(struct vnode *vp, struct uio fdisp_init(&fdi, 0); while (uio->uio_resid > 0) { + size_t sizeof_fwi; + + if (fuse_libabi_geq(data, 7, 9)) { + sizeof_fwi = sizeof(*fwi); + } else { + sizeof_fwi = FUSE_COMPAT_WRITE_IN_SIZE; + } + chunksize = MIN(uio->uio_resid, data->max_write); - fdi.iosize = sizeof(*fwi) + chunksize; + fdi.iosize = sizeof_fwi + chunksize; fdisp_make_vp(&fdi, FUSE_WRITE, vp, uio->uio_td, cred); fwi = fdi.indata; @@ -567,11 +575,8 @@ fuse_write_directbackend(struct vnode *vp, struct uio fwi->write_flags = write_flags; if (fuse_libabi_geq(data, 7, 9)) { fwi->flags = fufh_type_2_fflags(fufh->fufh_type); - fwi_data = (char *)fdi.indata + sizeof(*fwi); - } else { - fwi_data = (char *)fdi.indata + - FUSE_COMPAT_WRITE_IN_SIZE; } + fwi_data = (char *)fdi.indata + sizeof_fwi; if ((err = uiomove(fwi_data, chunksize, uio))) break; @@ -629,7 +634,7 @@ retry: break; } else { /* Resend the unwritten portion of data */ - fdi.iosize = sizeof(*fwi) + diff; + fdi.iosize = sizeof_fwi + diff; /* Refresh fdi without clearing data buffer */ fdisp_refresh_vp(&fdi, FUSE_WRITE, vp, uio->uio_td, cred); Modified: head/tests/sys/fs/fusefs/mockfs.cc ============================================================================== --- head/tests/sys/fs/fusefs/mockfs.cc Wed Sep 11 18:54:45 2019 (r352229) +++ head/tests/sys/fs/fusefs/mockfs.cc Wed Sep 11 19:29:40 2019 (r352230) @@ -155,14 +155,15 @@ void sigint_handler(int __unused sig) { // Don't do anything except interrupt the daemon's read(2) call } -void MockFS::debug_request(const mockfs_buf_in &in) +void MockFS::debug_request(const mockfs_buf_in &in, ssize_t buflen) { printf("%-11s ino=%2" PRIu64, opcode2opname(in.header.opcode), in.header.nodeid); if (verbosity > 1) { - printf(" uid=%5u gid=%5u pid=%5u unique=%" PRIu64 " len=%u", + printf(" uid=%5u gid=%5u pid=%5u unique=%" PRIu64 " len=%u" + " buflen=%zd", in.header.uid, in.header.gid, in.header.pid, - in.header.unique, in.header.len); + in.header.unique, in.header.len, buflen); } switch (in.header.opcode) { const char *name, *value; @@ -470,7 +471,7 @@ MockFS::~MockFS() { close(m_kq); } -void MockFS::audit_request(const mockfs_buf_in &in) { +void MockFS::audit_request(const mockfs_buf_in &in, ssize_t buflen) { uint32_t inlen = in.header.len; size_t fih = sizeof(in.header); switch (in.header.opcode) { @@ -478,19 +479,24 @@ void MockFS::audit_request(const mockfs_buf_in &in) { case FUSE_RMDIR: case FUSE_SYMLINK: case FUSE_UNLINK: - ASSERT_GT(inlen, fih) << "Missing request filename"; + EXPECT_GT(inlen, fih) << "Missing request filename"; + // No redundant information for checking buflen break; case FUSE_FORGET: - ASSERT_EQ(inlen, fih + sizeof(in.body.forget)); + EXPECT_EQ(inlen, fih + sizeof(in.body.forget)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_GETATTR: - ASSERT_EQ(inlen, fih + sizeof(in.body.getattr)); + EXPECT_EQ(inlen, fih + sizeof(in.body.getattr)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_SETATTR: - ASSERT_EQ(inlen, fih + sizeof(in.body.setattr)); + EXPECT_EQ(inlen, fih + sizeof(in.body.setattr)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_READLINK: - ASSERT_EQ(inlen, fih) << "Unexpected request body"; + EXPECT_EQ(inlen, fih) << "Unexpected request body"; + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_MKNOD: { @@ -499,33 +505,39 @@ void MockFS::audit_request(const mockfs_buf_in &in) { 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"; + EXPECT_GE(inlen, fih + s) << "Missing request body"; + EXPECT_GT(inlen, fih + s) << "Missing request filename"; + // No redundant information for checking buflen break; } case FUSE_MKDIR: - ASSERT_GE(inlen, fih + sizeof(in.body.mkdir)) << + EXPECT_GE(inlen, fih + sizeof(in.body.mkdir)) << "Missing request body"; - ASSERT_GT(inlen, fih + sizeof(in.body.mkdir)) << + EXPECT_GT(inlen, fih + sizeof(in.body.mkdir)) << "Missing request filename"; + // No redundant information for checking buflen break; case FUSE_RENAME: - ASSERT_GE(inlen, fih + sizeof(in.body.rename)) << + EXPECT_GE(inlen, fih + sizeof(in.body.rename)) << "Missing request body"; - ASSERT_GT(inlen, fih + sizeof(in.body.rename)) << + EXPECT_GT(inlen, fih + sizeof(in.body.rename)) << "Missing request filename"; + // No redundant information for checking buflen break; case FUSE_LINK: - ASSERT_GE(inlen, fih + sizeof(in.body.link)) << + EXPECT_GE(inlen, fih + sizeof(in.body.link)) << "Missing request body"; - ASSERT_GT(inlen, fih + sizeof(in.body.link)) << + EXPECT_GT(inlen, fih + sizeof(in.body.link)) << "Missing request filename"; + // No redundant information for checking buflen break; case FUSE_OPEN: - ASSERT_EQ(inlen, fih + sizeof(in.body.open)); + EXPECT_EQ(inlen, fih + sizeof(in.body.open)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_READ: - ASSERT_EQ(inlen, fih + sizeof(in.body.read)); + EXPECT_EQ(inlen, fih + sizeof(in.body.read)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_WRITE: { @@ -535,75 +547,94 @@ void MockFS::audit_request(const mockfs_buf_in &in) { 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 + EXPECT_GE(inlen, fih + s) << "Missing request body"; + EXPECT_EQ((size_t)buflen, fih + s + in.body.write.size); break; } case FUSE_DESTROY: case FUSE_STATFS: - ASSERT_EQ(inlen, fih); + EXPECT_EQ(inlen, fih); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_RELEASE: - ASSERT_EQ(inlen, fih + sizeof(in.body.release)); + EXPECT_EQ(inlen, fih + sizeof(in.body.release)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_FSYNC: case FUSE_FSYNCDIR: - ASSERT_EQ(inlen, fih + sizeof(in.body.fsync)); + EXPECT_EQ(inlen, fih + sizeof(in.body.fsync)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_SETXATTR: - ASSERT_GE(inlen, fih + sizeof(in.body.setxattr)) << + EXPECT_GE(inlen, fih + sizeof(in.body.setxattr)) << "Missing request body"; - ASSERT_GT(inlen, fih + sizeof(in.body.setxattr)) << + EXPECT_GT(inlen, fih + sizeof(in.body.setxattr)) << "Missing request attribute name"; + // No redundant information for checking buflen break; case FUSE_GETXATTR: - ASSERT_GE(inlen, fih + sizeof(in.body.getxattr)) << + EXPECT_GE(inlen, fih + sizeof(in.body.getxattr)) << "Missing request body"; - ASSERT_GT(inlen, fih + sizeof(in.body.getxattr)) << + EXPECT_GT(inlen, fih + sizeof(in.body.getxattr)) << "Missing request attribute name"; + // No redundant information for checking buflen break; case FUSE_LISTXATTR: - ASSERT_EQ(inlen, fih + sizeof(in.body.listxattr)); + EXPECT_EQ(inlen, fih + sizeof(in.body.listxattr)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_REMOVEXATTR: - ASSERT_GT(inlen, fih) << "Missing request attribute name"; + EXPECT_GT(inlen, fih) << "Missing request attribute name"; + // No redundant information for checking buflen break; case FUSE_FLUSH: - ASSERT_EQ(inlen, fih + sizeof(in.body.flush)); + EXPECT_EQ(inlen, fih + sizeof(in.body.flush)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_INIT: - ASSERT_EQ(inlen, fih + sizeof(in.body.init)); + EXPECT_EQ(inlen, fih + sizeof(in.body.init)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_OPENDIR: - ASSERT_EQ(inlen, fih + sizeof(in.body.opendir)); + EXPECT_EQ(inlen, fih + sizeof(in.body.opendir)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_READDIR: - ASSERT_EQ(inlen, fih + sizeof(in.body.readdir)); + EXPECT_EQ(inlen, fih + sizeof(in.body.readdir)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_RELEASEDIR: - ASSERT_EQ(inlen, fih + sizeof(in.body.releasedir)); + EXPECT_EQ(inlen, fih + sizeof(in.body.releasedir)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_GETLK: - ASSERT_EQ(inlen, fih + sizeof(in.body.getlk)); + EXPECT_EQ(inlen, fih + sizeof(in.body.getlk)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_SETLK: case FUSE_SETLKW: - ASSERT_EQ(inlen, fih + sizeof(in.body.setlk)); + EXPECT_EQ(inlen, fih + sizeof(in.body.setlk)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_ACCESS: - ASSERT_EQ(inlen, fih + sizeof(in.body.access)); + EXPECT_EQ(inlen, fih + sizeof(in.body.access)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_CREATE: - ASSERT_GE(inlen, fih + sizeof(in.body.create)) << + EXPECT_GE(inlen, fih + sizeof(in.body.create)) << "Missing request body"; - ASSERT_GT(inlen, fih + sizeof(in.body.create)) << + EXPECT_GT(inlen, fih + sizeof(in.body.create)) << "Missing request filename"; + // No redundant information for checking buflen break; case FUSE_INTERRUPT: - ASSERT_EQ(inlen, fih + sizeof(in.body.interrupt)); + EXPECT_EQ(inlen, fih + sizeof(in.body.interrupt)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_BMAP: - ASSERT_EQ(inlen, fih + sizeof(in.body.bmap)); + EXPECT_EQ(inlen, fih + sizeof(in.body.bmap)); + EXPECT_EQ((size_t)buflen, inlen); break; case FUSE_NOTIFY_REPLY: case FUSE_BATCH_FORGET: @@ -618,10 +649,13 @@ void MockFS::audit_request(const mockfs_buf_in &in) { } void MockFS::init(uint32_t flags) { + ssize_t buflen; + std::unique_ptr in(new mockfs_buf_in); std::unique_ptr out(new mockfs_buf_out); - read_request(*in); + read_request(*in, buflen); + audit_request(*in, buflen); ASSERT_EQ(FUSE_INIT, in->header.opcode); out->header.unique = in->header.unique; @@ -659,13 +693,15 @@ void MockFS::loop() { std::unique_ptr in(new mockfs_buf_in); ASSERT_TRUE(in != NULL); while (!m_quit) { + ssize_t buflen; + bzero(in.get(), sizeof(*in)); - read_request(*in); + read_request(*in, buflen); if (m_quit) break; if (verbosity > 0) - debug_request(*in); - audit_request(*in); + debug_request(*in, buflen); + audit_request(*in, buflen); if (pid_ok((pid_t)in->header.pid)) { process(*in, out); } else { @@ -766,8 +802,7 @@ void MockFS::process_default(const mockfs_buf_in& in, out.push_back(std::move(out0)); } -void MockFS::read_request(mockfs_buf_in &in) { - ssize_t res; +void MockFS::read_request(mockfs_buf_in &in, ssize_t &res) { int nready = 0; fd_set readfds; pollfd fds[1]; Modified: head/tests/sys/fs/fusefs/mockfs.hh ============================================================================== --- head/tests/sys/fs/fusefs/mockfs.hh Wed Sep 11 18:54:45 2019 (r352229) +++ head/tests/sys/fs/fusefs/mockfs.hh Wed Sep 11 19:29:40 2019 (r352230) @@ -283,8 +283,8 @@ 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 audit_request(const mockfs_buf_in &in, ssize_t buflen); + void debug_request(const mockfs_buf_in&, ssize_t buflen); void debug_response(const mockfs_buf_out&); /* Initialize a session after mounting */ @@ -300,8 +300,14 @@ class MockFS { /* Entry point for the daemon thread */ static void* service(void*); - /* Read, but do not process, a single request from the kernel */ - void read_request(mockfs_buf_in& in); + /* + * Read, but do not process, a single request from the kernel + * + * @param in Return storage for the FUSE request + * @param res Return value of read(2). If positive, the amount of + * data read from the fuse device. + */ + void read_request(mockfs_buf_in& in, ssize_t& res); /* Write a single response back to the kernel */ void write_response(const mockfs_buf_out &out);