tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: more fexecve questions



In article <20190910145247.C52CC17FDE3%rebar.astron.com@localhost>,
Christos Zoulas <christos%zoulas.com@localhost> wrote:

1. So the consensus is to leave the file descriptor alone.
2. I've added the reverse cache lookup, and now script execution works.
3. Nobody suggested anything to improve security.

Here's the latest patch; if I don't hear objections soon I will commit it...

Index: compat/netbsd32/netbsd32_execve.c
===================================================================
RCS file: /cvsroot/src/sys/compat/netbsd32/netbsd32_execve.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 netbsd32_execve.c
--- compat/netbsd32/netbsd32_execve.c	3 Sep 2018 16:29:29 -0000	1.39
+++ compat/netbsd32/netbsd32_execve.c	15 Sep 2019 00:14:10 -0000
@@ -71,9 +71,8 @@ netbsd32_execve(struct lwp *l, const str
 		syscallarg(netbsd32_charpp) argp;
 		syscallarg(netbsd32_charpp) envp;
 	} */
-	const char *path = SCARG_P32(uap, path);
 
-	return execve1(l, path, SCARG_P32(uap, argp),
+	return execve1(l, SCARG_P32(uap, path), -1, SCARG_P32(uap, argp),
 	    SCARG_P32(uap, envp), netbsd32_execve_fetch_element);
 }
 
@@ -86,13 +85,9 @@ netbsd32_fexecve(struct lwp *l, const st
 		syscallarg(netbsd32_charpp) argp;
 		syscallarg(netbsd32_charpp) envp;
 	} */
-	struct sys_fexecve_args ua;
 
-	NETBSD32TO64_UAP(fd);
-	NETBSD32TOP_UAP(argp, char * const);
-	NETBSD32TOP_UAP(envp, char * const);
-
-	return sys_fexecve(l, &ua, retval);
+	return execve1(l, NULL, SCARG(uap, fd), SCARG_P32(uap, argp),
+	    SCARG_P32(uap, envp), netbsd32_execve_fetch_element);
 }
 
 static int
Index: kern/exec_elf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/exec_elf.c,v
retrieving revision 1.98
diff -u -p -u -r1.98 exec_elf.c
--- kern/exec_elf.c	7 Jun 2019 23:35:52 -0000	1.98
+++ kern/exec_elf.c	15 Sep 2019 00:14:10 -0000
@@ -157,6 +157,7 @@ elf_populate_auxv(struct lwp *l, struct 
 	size_t len, vlen;
 	AuxInfo ai[ELF_AUX_ENTRIES], *a, *execname;
 	struct elf_args *ap;
+	char *path = l->l_proc->p_path;
 	int error;
 
 	a = ai;
@@ -224,9 +225,11 @@ elf_populate_auxv(struct lwp *l, struct 
 		a->a_v = l->l_proc->p_stackbase;
 		a++;
 
-		execname = a;
-		a->a_type = AT_SUN_EXECNAME;
-		a++;
+		if (path[0] == '/' && path[1] != '\0') {
+			execname = a;
+			a->a_type = AT_SUN_EXECNAME;
+			a++;
+		}
 
 		exec_free_emul_arg(pack);
 	} else {
@@ -242,7 +245,6 @@ elf_populate_auxv(struct lwp *l, struct 
 	KASSERT(vlen <= sizeof(ai));
 
 	if (execname) {
-		char *path = l->l_proc->p_path;
 		execname->a_v = (uintptr_t)(*stackp + vlen);
 		len = strlen(path) + 1;
 		if ((error = copyout(path, (*stackp + vlen), len)) != 0)
Index: kern/exec_script.c
===================================================================
RCS file: /cvsroot/src/sys/kern/exec_script.c,v
retrieving revision 1.79
diff -u -p -u -r1.79 exec_script.c
--- kern/exec_script.c	27 Jan 2019 02:08:43 -0000	1.79
+++ kern/exec_script.c	15 Sep 2019 00:14:10 -0000
@@ -290,7 +290,7 @@ check_shell:
 	/* try loading the interpreter */
 	if ((error = exec_makepathbuf(l, shellname, UIO_SYSSPACE,
 	    &shell_pathbuf, NULL)) == 0) {
-		error = check_exec(l, epp, shell_pathbuf);
+		error = check_exec(l, epp, shell_pathbuf, NULL);
 		pathbuf_destroy(shell_pathbuf);
 	}
 
Index: kern/kern_exec.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_exec.c,v
retrieving revision 1.479
diff -u -p -u -r1.479 kern_exec.c
--- kern/kern_exec.c	7 Sep 2019 15:34:44 -0000	1.479
+++ kern/kern_exec.c	15 Sep 2019 00:14:10 -0000
@@ -257,7 +257,7 @@ struct execve_data {
 	struct ps_strings	ed_arginfo;
 	char			*ed_argp;
 	const char		*ed_pathstring;
-	char			*ed_resolvedpathbuf;
+	char			*ed_resolvedname;
 	size_t			ed_ps_strings_sz;
 	int			ed_szsigcode;
 	size_t			ed_argslen;
@@ -309,9 +309,32 @@ exec_path_free(struct execve_data *data)
 {              
 	pathbuf_stringcopy_put(data->ed_pathbuf, data->ed_pathstring);
 	pathbuf_destroy(data->ed_pathbuf);
-	PNBUF_PUT(data->ed_resolvedpathbuf);
+	if (data->ed_resolvedname)
+		PNBUF_PUT(data->ed_resolvedname);
 }
 
+static void
+exec_resolvename(struct lwp *l, struct exec_package *epp, struct vnode *vp,
+    char **rpath)
+{
+	int error;
+	char *p;
+
+	KASSERT(rpath != NULL);
+
+	*rpath = PNBUF_GET();
+	error = vnode_to_path(*rpath, MAXPATHLEN, vp, l, l->l_proc);
+	if (error) {
+		PNBUF_PUT(*rpath);
+		*rpath = NULL;
+		return;
+	}
+	epp->ep_resolvedname = *rpath;
+	if ((p = strrchr(*rpath, '/')) != NULL)
+		epp->ep_kname = p + 1;
+}
+
+
 /*
  * check exec:
  * given an "executable" described in the exec package's namei info,
@@ -339,26 +362,40 @@ exec_path_free(struct execve_data *data)
  */
 int
 /*ARGSUSED*/
-check_exec(struct lwp *l, struct exec_package *epp, struct pathbuf *pb)
+check_exec(struct lwp *l, struct exec_package *epp, struct pathbuf *pb,
+    char **rpath)
 {
 	int		error, i;
 	struct vnode	*vp;
-	struct nameidata nd;
 	size_t		resid;
 
-	// grab the absolute pathbuf here before namei() trashes it.
-	pathbuf_copystring(pb, epp->ep_resolvedname, PATH_MAX);
-	NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | TRYEMULROOT, pb);
+	if (epp->ep_resolvedname) {
+		struct nameidata nd;
 
-	/* first get the vnode */
-	if ((error = namei(&nd)) != 0)
-		return error;
-	epp->ep_vp = vp = nd.ni_vp;
+		// grab the absolute pathbuf here before namei() trashes it.
+		pathbuf_copystring(pb, epp->ep_resolvedname, PATH_MAX);
+		NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | TRYEMULROOT, pb);
 
+		/* first get the vnode */
+		if ((error = namei(&nd)) != 0)
+			return error;
+
+		epp->ep_vp = vp = nd.ni_vp;
 #ifdef DIAGNOSTIC
-	/* paranoia (take this out once namei stuff stabilizes) */
-	memset(nd.ni_pnbuf, '~', PATH_MAX);
+		/* paranoia (take this out once namei stuff stabilizes) */
+		memset(nd.ni_pnbuf, '~', PATH_MAX);
 #endif
+	} else {
+		struct file *fp;
+
+		if ((error = fd_getvnode(epp->ep_xfd, &fp)) != 0)
+			return error;
+		epp->ep_vp = vp = fp->f_vnode;
+		vref(vp);
+		fd_putfile(epp->ep_xfd);
+		exec_resolvename(l, epp, vp, rpath);
+		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
+	}
 
 	/* check access and type */
 	if (vp->v_type != VREG) {
@@ -387,19 +424,21 @@ check_exec(struct lwp *l, struct exec_pa
 	/* unlock vp, since we need it unlocked from here on out. */
 	VOP_UNLOCK(vp);
 
+	if (epp->ep_resolvedname) {
 #if NVERIEXEC > 0
-	error = veriexec_verify(l, vp, epp->ep_resolvedname,
-	    epp->ep_flags & EXEC_INDIR ? VERIEXEC_INDIRECT : VERIEXEC_DIRECT,
-	    NULL);
-	if (error)
-		goto bad2;
+		error = veriexec_verify(l, vp, epp->ep_resolvedname,
+		    epp->ep_flags & EXEC_INDIR ? VERIEXEC_INDIRECT
+		    : VERIEXEC_DIRECT, NULL);
+		if (error)
+			goto bad2;
 #endif /* NVERIEXEC > 0 */
 
 #ifdef PAX_SEGVGUARD
-	error = pax_segvguard(l, vp, epp->ep_resolvedname, false);
-	if (error)
-		goto bad2;
+		error = pax_segvguard(l, vp, epp->ep_resolvedname, false);
+		if (error)
+			goto bad2;
 #endif /* PAX_SEGVGUARD */
+	}
 
 	/* now we have the file, get the exec header */
 	error = vn_rdwr(UIO_READ, vp, epp->ep_hdr, epp->ep_hdrlen, 0,
@@ -545,7 +584,7 @@ sys_execve(struct lwp *l, const struct s
 		syscallarg(char * const *)	envp;
 	} */
 
-	return execve1(l, SCARG(uap, path), SCARG(uap, argp),
+	return execve1(l, SCARG(uap, path), -1, SCARG(uap, argp),
 	    SCARG(uap, envp), execve_fetch_element);
 }
 
@@ -559,7 +598,8 @@ sys_fexecve(struct lwp *l, const struct 
 		syscallarg(char * const *)	envp;
 	} */
 
-	return ENOSYS;
+	return execve1(l, NULL, SCARG(uap, fd), SCARG(uap, argp),
+	    SCARG(uap, envp), execve_fetch_element);
 }
 
 /*
@@ -680,7 +720,7 @@ exec_vm_minaddr(vaddr_t va_min)
 }
 
 static int
-execve_loadvm(struct lwp *l, const char *path, char * const *args,
+execve_loadvm(struct lwp *l, const char *path, int fd, char * const *args,
 	char * const *envs, execve_fetch_element_t fetch_element,
 	struct execve_data * restrict data)
 {
@@ -689,7 +729,6 @@ execve_loadvm(struct lwp *l, const char 
 	struct proc		*p;
 	char			*dp;
 	u_int			modgen;
-	size_t			offs;
 
 	KASSERT(data != NULL);
 
@@ -732,24 +771,36 @@ execve_loadvm(struct lwp *l, const char 
 	 */
 	rw_enter(&p->p_reflock, RW_WRITER);
 
-	/*
-	 * Init the namei data to point the file user's program name.
-	 * This is done here rather than in check_exec(), so that it's
-	 * possible to override this settings if any of makecmd/probe
-	 * functions call check_exec() recursively - for example,
-	 * see exec_script_makecmds().
-	 */
-	if ((error = exec_makepathbuf(l, path, UIO_USERSPACE,
-	    &data->ed_pathbuf, &offs)) != 0)
-		goto clrflg;
-	data->ed_pathstring = pathbuf_stringcopy_get(data->ed_pathbuf);
-	data->ed_resolvedpathbuf = PNBUF_GET();
+	if (path == NULL) {
+		data->ed_pathbuf = pathbuf_assimilate(strcpy(PNBUF_GET(), "/"));
+		data->ed_pathstring = pathbuf_stringcopy_get(data->ed_pathbuf);
+		epp->ep_kname = "*fexecve*";
+		data->ed_resolvedname = NULL;
+		epp->ep_resolvedname = NULL;
+		epp->ep_xfd = fd;
+	} else {
+		size_t	offs;
+		/*
+		 * Init the namei data to point the file user's program name.
+		 * This is done here rather than in check_exec(), so that it's
+		 * possible to override this settings if any of makecmd/probe
+		 * functions call check_exec() recursively - for example,
+		 * see exec_script_makecmds().
+		 */
+		if ((error = exec_makepathbuf(l, path, UIO_USERSPACE,
+		    &data->ed_pathbuf, &offs)) != 0)
+			goto clrflg;
+		data->ed_pathstring = pathbuf_stringcopy_get(data->ed_pathbuf);
+		epp->ep_kname = data->ed_pathstring + offs;
+		data->ed_resolvedname = PNBUF_GET();
+		epp->ep_resolvedname = data->ed_resolvedname;
+		epp->ep_xfd = -1;
+	}
+
 
 	/*
 	 * initialize the fields of the exec package.
 	 */
-	epp->ep_kname = data->ed_pathstring + offs;
-	epp->ep_resolvedname = data->ed_resolvedpathbuf;
 	epp->ep_hdr = kmem_alloc(exec_maxhdrsz, KM_SLEEP);
 	epp->ep_hdrlen = exec_maxhdrsz;
 	epp->ep_hdrvalid = 0;
@@ -768,7 +819,8 @@ execve_loadvm(struct lwp *l, const char 
 	rw_enter(&exec_lock, RW_READER);
 
 	/* see if we can run it. */
-	if ((error = check_exec(l, epp, data->ed_pathbuf)) != 0) {
+	if ((error = check_exec(l, epp, data->ed_pathbuf,
+	    &data->ed_resolvedname)) != 0) {
 		if (error != ENOENT && error != EACCES && error != ENOEXEC) {
 			DPRINTF(("%s: check exec failed for %s, error %d\n",
 			    __func__, epp->ep_kname, error));
@@ -942,10 +994,19 @@ execve_free_data(struct execve_data *dat
 static void
 pathexec(struct proc *p, const char *resolvedname)
 {
-	KASSERT(resolvedname[0] == '/');
-
 	/* set command name & other accounting info */
-	strlcpy(p->p_comm, strrchr(resolvedname, '/') + 1, sizeof(p->p_comm));
+	const char *cmdname;
+
+	if (resolvedname == NULL) {
+		cmdname = "*fexecve*";
+		resolvedname = "/";
+	} else {
+		cmdname = strrchr(resolvedname, '/') + 1;
+	}
+	KASSERTMSG(resolvedname[0] == '/', "bad resolvedname `%s'",
+	    resolvedname);
+
+	strlcpy(p->p_comm, cmdname, sizeof(p->p_comm));
 
 	kmem_strfree(p->p_path);
 	p->p_path = kmem_strdupsize(resolvedname, NULL, KM_SLEEP);
@@ -1346,13 +1407,13 @@ execve_runproc(struct lwp *l, struct exe
 }
 
 int
-execve1(struct lwp *l, const char *path, char * const *args,
+execve1(struct lwp *l, const char *path, int fd, char * const *args,
     char * const *envs, execve_fetch_element_t fetch_element)
 {
 	struct execve_data data;
 	int error;
 
-	error = execve_loadvm(l, path, args, envs, fetch_element, &data);
+	error = execve_loadvm(l, path, fd, args, envs, fetch_element, &data);
 	if (error)
 		return error;
 	error = execve_runproc(l, &data, false, false);
@@ -2376,7 +2437,7 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
 	 * Do the first part of the exec now, collect state
 	 * in spawn_data.
 	 */
-	error = execve_loadvm(l1, path, argv,
+	error = execve_loadvm(l1, path, -1, argv,
 	    envp, fetch, &spawn_data->sed_exec);
 	if (error == EJUSTRETURN)
 		error = 0;
Index: kern/vfs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.200
diff -u -p -u -r1.200 vfs_vnops.c
--- kern/vfs_vnops.c	7 Mar 2019 11:09:48 -0000	1.200
+++ kern/vfs_vnops.c	15 Sep 2019 00:14:11 -0000
@@ -309,6 +309,9 @@ vn_openchk(struct vnode *vp, kauth_cred_
 	if ((fflags & FREAD) != 0) {
 		permbits = VREAD;
 	}
+	if ((fflags & FEXEC) != 0) {
+		permbits |= VEXEC;
+	}
 	if ((fflags & (FWRITE | O_TRUNC)) != 0) {
 		permbits |= VWRITE;
 		if (vp->v_type == VDIR) {
Index: sys/exec.h
===================================================================
RCS file: /cvsroot/src/sys/sys/exec.h,v
retrieving revision 1.154
diff -u -p -u -r1.154 exec.h
--- sys/exec.h	27 Jan 2019 02:08:50 -0000	1.154
+++ sys/exec.h	15 Sep 2019 00:14:11 -0000
@@ -188,7 +188,8 @@ struct exec_fakearg {
 
 struct exec_package {
 	const char *ep_kname;		/* kernel-side copy of file's name */
-	char *ep_resolvedname;		/* fully resolved path from namei */
+	char	*ep_resolvedname;	/* fully resolved path from namei */
+	int	ep_xfd;			/* fexecve file descriptor */
 	void	*ep_hdr;		/* file's exec header */
 	u_int	ep_hdrlen;		/* length of ep_hdr */
 	u_int	ep_hdrvalid;		/* bytes of ep_hdr that are valid */
@@ -268,7 +269,7 @@ void	setregs			(struct lwp *, struct exe
 int	check_veriexec		(struct lwp *, struct vnode *,
 				     struct exec_package *, int);
 int	check_exec		(struct lwp *, struct exec_package *,
-				     struct pathbuf *);
+				     struct pathbuf *, char **);
 int	exec_init		(int);
 int	exec_read_from		(struct lwp *, struct vnode *, u_long off,
 				    void *, size_t);
@@ -301,7 +302,7 @@ void	new_vmcmd(struct exec_vmcmd_set *,
 	new_vmcmd(evsp,lwp,len,addr,vp,offset,prot,flags)
 
 typedef	int (*execve_fetch_element_t)(char * const *, size_t, char **);
-int	execve1(struct lwp *, const char *, char * const *, char * const *,
+int	execve1(struct lwp *, const char *, int, char * const *, char * const *,
     execve_fetch_element_t);
 
 struct posix_spawn_file_actions;
Index: sys/fcntl.h
===================================================================
RCS file: /cvsroot/src/sys/sys/fcntl.h,v
retrieving revision 1.50
diff -u -p -u -r1.50 fcntl.h
--- sys/fcntl.h	20 Feb 2018 18:20:05 -0000	1.50
+++ sys/fcntl.h	15 Sep 2019 00:14:11 -0000
@@ -121,6 +121,7 @@
 #if defined(_NETBSD_SOURCE)
 #define	O_NOSIGPIPE	0x01000000	/* don't deliver sigpipe */
 #define	O_REGULAR	0x02000000	/* fail if not a regular file */
+#define	O_EXEC		0x04000000	/* open for executing only */
 #endif
 
 #ifdef _KERNEL
@@ -132,8 +133,9 @@
 #define	O_MASK		(O_ACCMODE|O_NONBLOCK|O_APPEND|O_SHLOCK|O_EXLOCK|\
 			 O_ASYNC|O_SYNC|O_CREAT|O_TRUNC|O_EXCL|O_DSYNC|\
 			 O_RSYNC|O_NOCTTY|O_ALT_IO|O_NOFOLLOW|O_DIRECT|\
-			 O_DIRECTORY|O_CLOEXEC|O_NOSIGPIPE|O_REGULAR)
+			 O_DIRECTORY|O_CLOEXEC|O_NOSIGPIPE|O_REGULAR|O_EXEC)
 
+#define	FEXEC		O_EXEC
 #define	FMARK		0x00001000	/* mark during gc() */
 #define	FDEFER		0x00002000	/* defer for next gc pass */
 #define	FHASLOCK	0x00004000	/* descriptor holds advisory lock */
@@ -144,7 +146,7 @@
 #define	FCNTLFLAGS	(FAPPEND|FASYNC|FFSYNC|FNONBLOCK|FDSYNC|FRSYNC|FALTIO|\
 			 FDIRECT|FNOSIGPIPE)
 /* bits to save after open(2) */
-#define	FMASK		(FREAD|FWRITE|FCNTLFLAGS)
+#define	FMASK		(FREAD|FWRITE|FCNTLFLAGS|FEXEC)
 #endif /* _KERNEL */
 
 /*



Home | Main Index | Thread Index | Old Index