diff mbox series

[11/25] virtiofsd: validate input buffer sizes in do_write_buf()

Message ID 20191024112718.34657-12-dgilbert@redhat.com
State New
Headers show
Series virtiofs daemon (security) | expand

Commit Message

Dr. David Alan Gilbert Oct. 24, 2019, 11:27 a.m. UTC
From: Stefan Hajnoczi <stefanha@redhat.com>

There is a small change in behavior: if fuse_write_in->size doesn't
match the input buffer size then the request is failed.  Previously
write requests with 1 fuse_buf element would truncate to
fuse_write_in->size.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 contrib/virtiofsd/fuse_lowlevel.c | 62 +++++++++++++++++++------------
 1 file changed, 38 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
index 2bd2ba00b9..7927348398 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -1006,7 +1006,8 @@  static void do_write(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
 		fuse_reply_err(req, ENOSYS);
 }
 
-static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid, const void *inarg,
+static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid,
+			 struct fuse_mbuf_iter *iter,
 			 struct fuse_bufvec *ibufv)
 {
 	struct fuse_session *se = req->se;
@@ -1015,34 +1016,36 @@  static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid, const void *inarg,
 		.buf[0] = ibufv->buf[0],
 		.count = 1,
 	};
-	struct fuse_write_in *arg = (struct fuse_write_in *) inarg;
+	struct fuse_write_in *arg;
+	size_t arg_size = sizeof(*arg);
 	struct fuse_file_info fi;
 
 	memset(&fi, 0, sizeof(fi));
+
+	if (se->conn.proto_minor < 9) {
+		arg_size = FUSE_COMPAT_WRITE_IN_SIZE;
+	}
+
+	arg = fuse_mbuf_iter_advance(iter, arg_size);
+	if (!arg) {
+		fuse_reply_err(req, EINVAL);
+		return;
+	}
+
+	/* Only access non-compat fields here! */
+	if (se->conn.proto_minor >= 9) {
+		fi.lock_owner = arg->lock_owner;
+		fi.flags = arg->flags;
+	}
+
 	fi.fh = arg->fh;
 	fi.writepage = arg->write_flags & FUSE_WRITE_CACHE;
 
 	if (ibufv->count == 1) {
-		if (se->conn.proto_minor < 9) {
-			tmpbufv.buf[0].mem = ((char *) arg) + FUSE_COMPAT_WRITE_IN_SIZE;
-			tmpbufv.buf[0].size -= sizeof(struct fuse_in_header) +
-				FUSE_COMPAT_WRITE_IN_SIZE;
-			assert(!(tmpbufv.buf[0].flags & FUSE_BUF_IS_FD));
-		} else {
-			fi.lock_owner = arg->lock_owner;
-			fi.flags = arg->flags;
-			if (!(tmpbufv.buf[0].flags & FUSE_BUF_IS_FD))
-				tmpbufv.buf[0].mem = PARAM(arg);
-
-			tmpbufv.buf[0].size -= sizeof(struct fuse_in_header) +
-				sizeof(struct fuse_write_in);
-		}
-		if (tmpbufv.buf[0].size < arg->size) {
-			fuse_log(FUSE_LOG_ERR, "fuse: do_write_buf: buffer size too small\n");
-			fuse_reply_err(req, EIO);
-			return;
-		}
-		tmpbufv.buf[0].size = arg->size;
+		assert(!(tmpbufv.buf[0].flags & FUSE_BUF_IS_FD));
+		tmpbufv.buf[0].mem = ((char *) arg) + arg_size;
+		tmpbufv.buf[0].size -= sizeof(struct fuse_in_header) +
+				       arg_size;
 		pbufv = &tmpbufv;
 	} else {
 		// Input bufv contains the headers in the first element
@@ -1050,6 +1053,12 @@  static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid, const void *inarg,
 		ibufv->buf[0].size = 0;
 	}
 
+	if (fuse_buf_size(pbufv) != arg->size) {
+		fuse_log(FUSE_LOG_ERR, "fuse: do_write_buf: buffer size doesn't match arg->size\n");
+		fuse_reply_err(req, EIO);
+		return;
+	}
+
 	se->op.write_buf(req, nodeid, pbufv, arg->offset, &fi);
 }
 
@@ -2002,12 +2011,17 @@  void fuse_session_process_buf_int(struct fuse_session *se,
 				  struct fuse_bufvec *bufv, struct fuse_chan *ch)
 {
 	const struct fuse_buf *buf = bufv->buf;
+	struct fuse_mbuf_iter iter = FUSE_MBUF_ITER_INIT(buf);
 	struct fuse_in_header *in;
 	const void *inarg;
 	struct fuse_req *req;
 	int err;
 
-	in = buf->mem;
+	/* The first buffer must be a memory buffer */
+	assert(!(buf->flags & FUSE_BUF_IS_FD));
+
+	in = fuse_mbuf_iter_advance(&iter, sizeof(*in));
+	assert(in); /* caller guarantees the input buffer is large enough */
 
 	if (se->debug) {
 		fuse_log(FUSE_LOG_DEBUG,
@@ -2074,7 +2088,7 @@  void fuse_session_process_buf_int(struct fuse_session *se,
 
 	inarg = (void *) &in[1];
 	if (in->opcode == FUSE_WRITE && se->op.write_buf)
-		do_write_buf(req, in->nodeid, inarg, bufv);
+		do_write_buf(req, in->nodeid, &iter, bufv);
 	else
 		fuse_ll_ops[in->opcode].func(req, in->nodeid, inarg);