Replace expand_name() function with corefile_open() function, which not

only returns name, but also vnode of corefile to use.

This simplifies the code and closes few races, especially in %I handling.

Reviewed by:	kib
Obtained from:	WHEEL Systems

sys/kern/kern_sig.c

@@ -106,7 +106,6 @@ SDT_PROBE_ARGTYPE(proc, kernel, , signal_discard, 1, "struct proc *");
 SDT_PROBE_ARGTYPE(proc, kernel, , signal_discard, 2, "int");
 
 static int	coredump(struct thread *);
-static char	*expand_name(const char *, uid_t, pid_t, struct thread *, int);
 static int	killpg1(struct thread *td, int sig, int pgid, int all,
 		    ksiginfo_t *ksi);
 static int	issignal(struct thread *td, int stop_allowed);
@@ -3034,8 +3033,9 @@ SYSCTL_STRING(_kern, OID_AUTO, corefile, CTLFLAG_RW, corefilename,
     sizeof(corefilename), "Process corefile name format string");
 
 /*
- * expand_name(name, uid, pid, td, compress)
- * Expand the name described in corefilename, using name, uid, and pid.
+ * corefile_open(comm, uid, pid, td, compress, vpp, namep)
+ * Expand the name described in corefilename, using name, uid, and pid
+ * and open/create core file.
  * corefilename is a printf-like string, with three format specifiers:
  *	%N	name of process ("name")
  *	%P	process id (pid)
@@ -3044,15 +3044,15 @@ SYSCTL_STRING(_kern, OID_AUTO, corefile, CTLFLAG_RW, corefilename,
  * by using "/dev/null", or all core files can be stored in "/cores/%U/%N-%P".
  * This is controlled by the sysctl variable kern.corefile (see above).
  */
-static char *
-expand_name(const char *comm, uid_t uid, pid_t pid, struct thread *td,
-    int compress)
+static int
+corefile_open(const char *comm, uid_t uid, pid_t pid, struct thread *td,
+    int compress, struct vnode **vpp, char **namep)
 {
+	struct nameidata nd;
 	struct sbuf sb;
 	const char *format;
-	char *name;
-	int i, indexpos;
-	char *hostname;
+	char *hostname, *name;
+	int indexpos, i, error, cmode, flags, oflags;
 
 	hostname = NULL;
 	format = corefilename;
@@ -3110,30 +3110,25 @@ expand_name(const char *comm, uid_t uid, pid_t pid, struct thread *td,
 		    "long\n", (long)pid, comm, (u_long)uid);
 		sbuf_delete(&sb);
 		free(name, M_TEMP);
-		return (NULL);
+		return (ENOMEM);
 	}
 	sbuf_finish(&sb);
 	sbuf_delete(&sb);
 
+	cmode = S_IRUSR | S_IWUSR;
+	oflags = VN_OPEN_NOAUDIT | (capmode_coredump ? VN_OPEN_NOCAPCHECK : 0);
+
 	/*
 	 * If the core format has a %I in it, then we need to check
 	 * for existing corefiles before returning a name.
 	 * To do this we iterate over 0..num_cores to find a
 	 * non-existing core file name to use.
 	 */
 	if (indexpos != -1) {
-		struct nameidata nd;
-		int cmode, flags, oflags, error;
-
-		cmode = S_IRUSR | S_IWUSR;
-		oflags = VN_OPEN_NOAUDIT |
-		    (capmode_coredump ? VN_OPEN_NOCAPCHECK : 0);
-
 		for (i = 0; i < num_cores; i++) {
 			flags = O_CREAT | O_EXCL | FWRITE | O_NOFOLLOW;
 			name[indexpos] = '0' + i;
-			NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE,
-			    name, td);
+			NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td);
 			error = vn_open_cred(&nd, &flags, cmode, oflags,
 			    td->td_ucred, NULL);
 			if (error) {
@@ -3143,25 +3138,26 @@ expand_name(const char *comm, uid_t uid, pid_t pid, struct thread *td,
 				    "pid %d (%s), uid (%u):  Path `%s' failed "
 				    "on initial open test, error = %d\n",
 				    pid, comm, uid, name, error);
-				free(name, M_TEMP);
-				return (NULL);
-			}
-			NDFREE(&nd, NDF_ONLY_PNBUF);
-			VOP_UNLOCK(nd.ni_vp, 0);
-			error = vn_close(nd.ni_vp, FWRITE, td->td_ucred, td);
-			if (error) {
-				log(LOG_ERR,
-				    "pid %d (%s), uid (%u):  Path `%s' failed "
-				    "on close after initial open test, "
-				    "error = %d\n",
-				    pid, comm, uid, name, error);
-				free(name, M_TEMP);
-				return (NULL);
 			}
-			break;
+			goto out;
 		}
 	}
-	return (name);
+
+	flags = O_CREAT | FWRITE | O_NOFOLLOW;
+	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td);
+	error = vn_open_cred(&nd, &flags, cmode, oflags, td->td_ucred, NULL);
+out:
+	if (error) {
+#ifdef AUDIT
+		audit_proc_coredump(td, name, error);
+#endif
+		free(name, M_TEMP);
+		return (error);
+	}
+	NDFREE(&nd, NDF_ONLY_PNBUF);
+	*vpp = nd.ni_vp;
+	*namep = name;
+	return (0);
 }
 
 /*
@@ -3179,9 +3175,8 @@ coredump(struct thread *td)
 	struct ucred *cred = td->td_ucred;
 	struct vnode *vp;
 	struct flock lf;
-	struct nameidata nd;
 	struct vattr vattr;
-	int error, error1, flags, locked;
+	int error, error1, locked;
 	struct mount *mp;
 	char *name;			/* name of corefile */
 	off_t limit;
@@ -3216,25 +3211,11 @@ coredump(struct thread *td)
 	}
 	PROC_UNLOCK(p);
 
-	name = expand_name(p->p_comm, cred->cr_uid, p->p_pid, td, compress);
-	if (name == NULL)
-		return (EINVAL);
-
 restart:
-	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td);
-	flags = O_CREAT | FWRITE | O_NOFOLLOW;
-	error = vn_open_cred(&nd, &flags, S_IRUSR | S_IWUSR,
-	    VN_OPEN_NOAUDIT | (capmode_coredump ? VN_OPEN_NOCAPCHECK : 0),
-	    cred, NULL);
-	if (error) {
-#ifdef AUDIT
-		audit_proc_coredump(td, name, error);
-#endif
-		free(name, M_TEMP);
+	error = corefile_open(p->p_comm, cred->cr_uid, p->p_pid, td, compress,
+	    &vp, &name);
+	if (error != 0)
 		return (error);
-	}
-	NDFREE(&nd, NDF_ONLY_PNBUF);
-	vp = nd.ni_vp;
 
 	/* Don't dump to non-regular files or files with links. */
 	if (vp->v_type != VREG || VOP_GETATTR(vp, &vattr, cred) != 0 ||