From jedgar@fxp.org Fri Aug 10 20:01:36 2001 Date: Sun, 5 Aug 2001 20:32:26 -0400 From: Chris Faulhaber To: Robert Watson Cc: security-officer@FreeBSD.org Subject: Re: procfs tweaks in -CURRENT On Fri, Aug 03, 2001 at 02:27:54PM -0400, Robert Watson wrote: > > Attached, please find a first pass at a -STABLE patch to correct the > problem, likewise by eliminating the kmem garbage (ps is not setgid in > -STABLE either). I also have a whitespace/style diff reduction patch to > apply once the RE gives permission. > The following is an updated patch that patches both procfs (after the whitespace commit) and linprocfs. -- Chris D. Faulhaber - jedgar@fxp.org - jedgar@FreeBSD.org -------------------------------------------------------- FreeBSD: The Power To Serve - http://www.FreeBSD.org Index: i386/linux/linprocfs/linprocfs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/i386/linux/linprocfs/Attic/linprocfs_vnops.c,v retrieving revision 1.3.2.4 diff -u -r1.3.2.4 linprocfs_vnops.c --- i386/linux/linprocfs/linprocfs_vnops.c 2001/06/25 19:46:47 1.3.2.4 +++ i386/linux/linprocfs/linprocfs_vnops.c 2001/08/06 00:14:54 @@ -64,7 +64,6 @@ #include extern struct vnode *procfs_findtextvp __P((struct proc *)); -extern int procfs_kmemaccess __P((struct proc *)); static int linprocfs_access __P((struct vop_access_args *)); static int linprocfs_badop __P((void)); @@ -143,8 +142,7 @@ return (EBUSY); p1 = ap->a_p; - if (p_trespass(p1, p2) && - !procfs_kmemaccess(p1)) + if (p_trespass(p1, p2)) return (EPERM); if (ap->a_mode & FWRITE) @@ -455,21 +453,6 @@ vap->va_atime = vap->va_mtime = vap->va_ctime; /* - * If the process has exercised some setuid or setgid - * privilege, then rip away read/write permission so - * that only root can gain access. - */ - switch (pfs->pfs_type) { - case Pmem: - /* Retain group kmem readablity. */ - if (procp->p_flag & P_SUGID) - vap->va_mode &= ~(VREAD|VWRITE); - break; - default: - break; - } - - /* * now do the object specific fields * * The size could be set from struct reg, but it's hardly @@ -545,7 +528,6 @@ vap->va_uid = 0; else vap->va_uid = procp->p_ucred->cr_uid; - vap->va_gid = KMEM_GROUP; break; case Pprocstat: Index: miscfs/procfs/procfs.h =================================================================== RCS file: /home/ncvs/src/sys/miscfs/procfs/Attic/procfs.h,v retrieving revision 1.32.2.1 diff -u -r1.32.2.1 procfs.h --- miscfs/procfs/procfs.h 2000/11/01 20:19:48 1.32.2.1 +++ miscfs/procfs/procfs.h 2001/08/06 00:14:55 @@ -88,8 +88,6 @@ ((cnp)->cn_namelen == (len) && \ (bcmp((s), (cnp)->cn_nameptr, (len)) == 0)) -#define KMEM_GROUP 2 - #define PROCFS_FILENO(pid, type) \ (((type) < Pproc) ? \ ((type) + 2) : \ @@ -147,9 +145,6 @@ int procfs_dotype __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio)); int procfs_docmdline __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio)); int procfs_dorlimit __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio)); - -/* Return 1 if process has special kernel digging privileges */ -int procfs_kmemaccess __P((struct proc *)); /* functions to check whether or not files should be displayed */ int procfs_validfile __P((struct proc *)); Index: miscfs/procfs/procfs_mem.c =================================================================== RCS file: /home/ncvs/src/sys/miscfs/procfs/Attic/procfs_mem.c,v retrieving revision 1.46.2.1 diff -u -r1.46.2.1 procfs_mem.c --- miscfs/procfs/procfs_mem.c 2000/11/01 20:19:48 1.46.2.1 +++ miscfs/procfs/procfs_mem.c 2001/08/06 00:14:55 @@ -244,21 +244,7 @@ if (uio->uio_resid == 0) return (0); - /* - * XXX - * We need to check for KMEM_GROUP because ps is sgid kmem; - * not allowing it here causes ps to not work properly. Arguably, - * this is a bug with what ps does. We only need to do this - * for Pmem nodes, and only if it's reading. This is still not - * good, as it may still be possible to grab illicit data if - * a process somehow gets to be KMEM_GROUP. Note that this also - * means that KMEM_GROUP can't change without editing procfs.h! - * All in all, quite yucky. - */ - - if ((!CHECKIO(curp, p) || p_trespass(curp, p)) && - !(uio->uio_rw == UIO_READ && - procfs_kmemaccess(curp))) + if (!CHECKIO(curp, p) || p_trespass(curp, p)) return EPERM; return (procfs_rwmem(curp, p, uio)); @@ -295,22 +281,4 @@ { return (p->p_textvp); -} - -int procfs_kmemaccess(curp) - struct proc *curp; -{ - int i; - struct ucred *cred; - - cred = curp->p_ucred; - if (suser(curp)) - return 1; - - /* XXX: Why isn't this done with file-perms ??? */ - for (i = 0; i < cred->cr_ngroups; i++) - if (cred->cr_groups[i] == KMEM_GROUP) - return 1; - - return 0; } Index: miscfs/procfs/procfs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/miscfs/procfs/Attic/procfs_vnops.c,v retrieving revision 1.76.2.4 diff -u -r1.76.2.4 procfs_vnops.c --- miscfs/procfs/procfs_vnops.c 2001/08/04 13:12:24 1.76.2.4 +++ miscfs/procfs/procfs_vnops.c 2001/08/06 00:14:58 @@ -148,8 +148,7 @@ return (EBUSY); p1 = ap->a_p; - if ((!CHECKIO(p1, p2) || p_trespass(p1, p2)) && - !procfs_kmemaccess(p1)) + if (!CHECKIO(p1, p2) || p_trespass(p1, p2)) return (EPERM); if (ap->a_mode & FWRITE) @@ -477,16 +476,12 @@ case Pregs: case Pfpregs: case Pdbregs: + case Pmem: if (procp->p_flag & P_SUGID) vap->va_mode &= ~((VREAD|VWRITE)| ((VREAD|VWRITE)>>3)| ((VREAD|VWRITE)>>6)); break; - case Pmem: - /* Retain group kmem readablity. */ - if (procp->p_flag & P_SUGID) - vap->va_mode &= ~(VREAD|VWRITE); - break; default: break; } @@ -556,7 +551,6 @@ vap->va_uid = 0; else vap->va_uid = procp->p_ucred->cr_uid; - vap->va_gid = KMEM_GROUP; break; case Pregs: Index: ufs/ufs/ufs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v retrieving revision 1.131.2.3 diff -u -r1.131.2.3 ufs_vnops.c --- ufs/ufs/ufs_vnops.c 2001/02/26 04:23:21 1.131.2.3 +++ ufs/ufs/ufs_vnops.c 2001/08/06 00:15:06 @@ -337,8 +337,12 @@ } /* If immutable bit set, nobody gets to write it. */ - if ((mode & VWRITE) && (ip->i_flags & IMMUTABLE)) + if ((mode & VWRITE) && (ip->i_flags & IMMUTABLE)) { +#ifdef SCHG_VERBOSE + printf("immutable overwrite denied for file: %s by uid %i\n", file, cred->cr_uid); +#endif return (EPERM); + } /* Otherwise, user id 0 always gets access. */ if (cred->cr_uid == 0) [Part 2, Application/PGP-SIGNATURE 240bytes] [Unable to print this part]