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

diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c
index 924d437c2fe3b8047bebce9f8e3d79721bdd21f3..f1b1e0c66ffcf9902602a4e71c76c75fafa80c80 100644 (file)
--- a/sys/fs/fuse/fuse_io.c
+++ b/sys/fs/fuse/fuse_io.c
@@ -555,9 +555,17 @@ fuse_write_directbackend(struct vnode *vp, struct uio *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 *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);

diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc
index ee0c8fb99d01a2c9f8f21b0480edb5eadc75ad64..f43afd247502306240f666b77a3a8d166ddd6f6e 100644 (file)
--- a/tests/sys/fs/fusefs/mockfs.cc
+++ b/tests/sys/fs/fusefs/mockfs.cc
@@ -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<mockfs_buf_in> in(new mockfs_buf_in);
        std::unique_ptr<mockfs_buf_out> 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<mockfs_buf_in> 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];

diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh
index d4d5cac7c5c7fbfca136c0910fe1546f760e5c71..4cf6daea7e7685bcedb07bb1d0cafb8fb0acd9b5 100644 (file)
--- a/tests/sys/fs/fusefs/mockfs.hh
+++ b/tests/sys/fs/fusefs/mockfs.hh
@@ -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);