Properly bounds check ioctl/pioctl data arguments for Coda:

1. Use unsigned rather than signed lengths
2. Bound messages to/from Venus to VC_MAXMSGSIZE
3. Bound messages to/from general user processes to VC_MAXDATASIZE
4. Update comment regarding data limits for pioctl

Without (1) and (3), it may be possible for unprivileged user processes to
read sensitive portions of kernel memory.  This issue is only present if
the Coda kernel module is loaded and venus (the userspace Coda daemon) is
running and has /coda mounted.

As Coda is considered experimental and production use is warned against in
the coda(4) man page, and because Coda must be explicitly configured for a
configuration to be vulnerable, we won't be issuing a security advisory.
However, if you are using Coda, then you are advised to apply these fixes.

Reported by:	Dan J. Rosenberg <drosenberg at vsecurity.com>
Obtained from:	NetBSD (Christos Zoulas)
Security:	Kernel memory disclosure; no advisory as feature experimental
MFC after:	3 days

3 files changed, 12 insertions, 3 deletions

diff --git a/sys/fs/coda/coda.h b/sys/fs/coda/coda.h
index ee1e7601f7be..2cfb71211c30 100644
--- a/sys/fs/coda/coda.h
+++ b/sys/fs/coda/coda.h
@@ -41,7 +41,9 @@
 #ifndef _CODA_HEADER_
 #define _CODA_HEADER_
 
+#ifdef _KERNEL
 #include "opt_coda.h"	/* for CODA_COMPAT_5 option */
+#endif
 
 /* Avoid CODA_COMPAT_5 redefinition in coda5 module */
 #if defined (CODA5_MODULE) && !defined(CODA_COMPAT_5)
@@ -782,8 +784,8 @@ union coda_downcalls {
 #define PIOCPARM_MASK 0x0000ffff
 struct ViceIoctl {
         caddr_t in, out;        /* Data to be transferred in, or out */
-        short in_size;          /* Size of input buffer <= 2K */
-        short out_size;         /* Maximum size of output buffer, <= 2K */
+        unsigned short in_size;	/* Size of input buffer <= 8K */
+        unsigned short out_size; /* Maximum size of output buffer, <= 8K */
 };
 
 #if defined(__CYGWIN32__) || defined(DJGPP)
diff --git a/sys/fs/coda/coda_venus.c b/sys/fs/coda/coda_venus.c
index 9999d3ae47b5..9d012dd2c90a 100644
--- a/sys/fs/coda/coda_venus.c
+++ b/sys/fs/coda/coda_venus.c
@@ -274,6 +274,12 @@ venus_ioctl(void *mdp, struct CodaFid *fid, int com, int flag, caddr_t data,
 	tmp = ((com >> 16) & IOCPARM_MASK) - sizeof (char *) - sizeof (int);
 	inp->cmd |= (tmp & IOCPARM_MASK) << 16;
 
+	if (iap->vi.in_size > VC_MAXMSGSIZE ||
+	    iap->vi.out_size > VC_MAXMSGSIZE) {
+		CODA_FREE(inp, coda_ioctl_size);
+		return (EINVAL);
+	}
+
 	inp->rwflag = flag;
 	inp->len = iap->vi.in_size;
 	inp->data = (char *)(sizeof (struct coda_ioctl_in));
diff --git a/sys/fs/coda/coda_vnops.c b/sys/fs/coda/coda_vnops.c
index 79d53e53b10d..02f6eb501a89 100644
--- a/sys/fs/coda/coda_vnops.c
+++ b/sys/fs/coda/coda_vnops.c
@@ -471,7 +471,8 @@ coda_ioctl(struct vop_ioctl_args *ap)
 		    iap->path)););
 		return (EINVAL);
 	}
-	if (iap->vi.in_size > VC_MAXDATASIZE) {
+	if (iap->vi.in_size > VC_MAXDATASIZE ||
+	    iap->vi.out_size > VC_MAXDATASIZE) {
 		NDFREE(&ndp, 0);
 		return (EINVAL);
 	}