From be8026427938329de36094640c60dde252abecd8 Mon Sep 17 00:00:00 2001 From: Robert Watson Date: Sat, 7 Aug 2010 08:08:14 +0000 Subject: 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 Obtained from: NetBSD (Christos Zoulas) Security: Kernel memory disclosure; no advisory as feature experimental MFC after: 3 days --- sys/fs/coda/coda.h | 6 ++++-- sys/fs/coda/coda_venus.c | 6 ++++++ sys/fs/coda/coda_vnops.c | 3 ++- 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); } -- cgit v1.2.3