svn commit: r349324 - head/sys/kern

[Konstantin Belousov]

Author: kib
Date: Sun Jun 23 21:15:31 2019
New Revision: 349324
URL: https://svnweb.freebsd.org/changeset/base/349324

Log:
  Switch to check for effective user id in r349320, and disable dumping
  into existing files for sugid processes.
  
  Despite using real user id pronounces the intent, it actually breaks
  suid coredumps, while not making any difference for non-sugid
  processes.  The reason for the breakage is that non-existent core file
  is created with the effective uid (unless weird hacks like SUIDDIR are
  configured).
  
  Then, if user enabled kern.sugid_coredump, core dumping should not
  overwrite core files owned by effective uid, but we cannot pretend to
  use real uid for dumping.
  
  PR:	68905
  admbugs:	358
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/sys/kern/kern_sig.c

Modified: head/sys/kern/kern_sig.c
==============================================================================
--- head/sys/kern/kern_sig.c	Sun Jun 23 21:06:56 2019	(r349323)
+++ head/sys/kern/kern_sig.c	Sun Jun 23 21:15:31 2019	(r349324)
@@ -3398,10 +3398,16 @@ corefile_open_last(struct thread *td, char *name, int 
 	}
 
 	if (oldvp != NULL) {
-		if (nextvp == NULL)
-			nextvp = oldvp;
-		else
+		if (nextvp == NULL) {
+			if ((td->td_proc->p_flag & P_SUGID) != 0) {
+				error = EFAULT;
+				vnode_close_locked(td, oldvp);
+			} else {
+				nextvp = oldvp;
+			}
+		} else {
 			vnode_close_locked(td, oldvp);
+		}
 	}
 	if (error != 0) {
 		if (nextvp != NULL)
@@ -3521,6 +3527,8 @@ corefile_open(const char *comm, uid_t uid, pid_t pid, 
 		oflags = VN_OPEN_NOAUDIT | VN_OPEN_NAMECACHE |
 		    (capmode_coredump ? VN_OPEN_NOCAPCHECK : 0);
 		flags = O_CREAT | FWRITE | O_NOFOLLOW;
+		if ((td->td_proc->p_flag & P_SUGID) != 0)
+			flags |= O_EXCL;
 
 		NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td);
 		error = vn_open_cred(&nd, &flags, cmode, oflags, td->td_ucred,
@@ -3597,11 +3605,11 @@ coredump(struct thread *td)
 
 	/*
 	 * Don't dump to non-regular files or files with links.
-	 * Do not dump into system files. Real user must own the corefile.
+	 * Do not dump into system files. Effective user must own the corefile.
 	 */
 	if (vp->v_type != VREG || VOP_GETATTR(vp, &vattr, cred) != 0 ||
 	    vattr.va_nlink != 1 || (vp->v_vflag & VV_SYSTEM) != 0 ||
-	    vattr.va_uid != cred->cr_ruid) {
+	    vattr.va_uid != cred->cr_uid) {
 		VOP_UNLOCK(vp, 0);
 		error = EFAULT;
 		goto out;