A few fixes for kvm_getargv(3)/kvm_getenv(3):

  1. Most notable: this splits argv buffer into argv-specific one
     and environ-specific one. This makes ps -eww totally happy.

  2. realloc() usage in kvm_argv() is now ENOMEM-prone.

  3. The "int off" changed to "ptrdiff_t off", as it should be.

input & okay deraadt@, millert@

3 files changed, 97 insertions, 63 deletions

diff --git a/lib/libkvm/kvm.c b/lib/libkvm/kvm.c
index dce4359fc78..fc0b3ededb0 100644
--- a/lib/libkvm/kvm.c
+++ b/lib/libkvm/kvm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kvm.c,v 1.63 2017/12/14 17:06:33 guenther Exp $ */
+/*	$OpenBSD: kvm.c,v 1.64 2018/05/03 15:47:41 zhuk Exp $ */
 /*	$NetBSD: kvm.c,v 1.43 1996/05/05 04:31:59 gwr Exp $	*/
 
 /*-
@@ -191,6 +191,9 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, const char *sf,
 	kd->argspc = 0;
 	kd->argbuf = 0;
 	kd->argv = 0;
+	kd->envspc = 0;
+	kd->envbuf = 0;
+	kd->envp = 0;
 	kd->vmst = NULL;
 	kd->vm_page_buckets = 0;
 	kd->kcore_hdr = 0;
@@ -660,6 +663,12 @@ kvm_close(kvm_t *kd)
 		free((void *)kd->argbuf);
 	if (kd->argv != 0)
 		free((void *)kd->argv);
+	if (kd->envspc != 0)
+		free((void *)kd->envspc);
+	if (kd->envbuf != 0)
+		free((void *)kd->envbuf);
+	if (kd->envp != 0)
+		free((void *)kd->envp);
 	free((void *)kd);
 
 	return (error);
diff --git a/lib/libkvm/kvm_private.h b/lib/libkvm/kvm_private.h
index 3fe186e7365..f3780055ac8 100644
--- a/lib/libkvm/kvm_private.h
+++ b/lib/libkvm/kvm_private.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kvm_private.h,v 1.25 2017/12/14 17:06:33 guenther Exp $ */
+/*	$OpenBSD: kvm_private.h,v 1.26 2018/05/03 15:47:41 zhuk Exp $ */
 /*	$NetBSD: kvm_private.h,v 1.7 1996/05/05 04:32:15 gwr Exp $	*/
 
 /*-
@@ -53,11 +53,17 @@ struct __kvm {
 	struct kinfo_file *filebase;
 	int	nbpg;		/* page size */
 	char	*swapspc;	/* (dynamic) storage for swapped pages */
+
 	char	*argspc, *argbuf; /* (dynamic) storage for argv strings */
 	int	arglen;		/* length of the above */
 	char	**argv;		/* (dynamic) storage for argv pointers */
 	int	argc;		/* length of above (not actual # present) */
 
+	char	*envspc, *envbuf; /* (dynamic) storage for environ strings */
+	int	envlen;		/* length of the above */
+	char	**envp;		/* (dynamic) storage for environ pointers */
+	int	envc;		/* length of above (not actual # present) */
+
 	/*
 	 * Header structures for kernel dumps. Only gets filled in for
 	 * dead kernels.
diff --git a/lib/libkvm/kvm_proc.c b/lib/libkvm/kvm_proc.c
index 12221d565c8..a18e9983ae0 100644
--- a/lib/libkvm/kvm_proc.c
+++ b/lib/libkvm/kvm_proc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kvm_proc.c,v 1.58 2016/11/07 00:26:33 guenther Exp $	*/
+/*	$OpenBSD: kvm_proc.c,v 1.59 2018/05/03 15:47:41 zhuk Exp $	*/
 /*	$NetBSD: kvm_proc.c,v 1.30 1999/03/24 05:50:50 mrg Exp $	*/
 /*-
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -76,6 +76,7 @@
 #include <sys/stat.h>
 #include <sys/ioctl.h>
 #include <sys/tty.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
@@ -100,9 +101,9 @@
 static char	*_kvm_ureadm(kvm_t *, const struct kinfo_proc *, u_long, u_long *);
 static ssize_t	kvm_ureadm(kvm_t *, const struct kinfo_proc *, u_long, char *, size_t);
 
-static char	**kvm_argv(kvm_t *, const struct kinfo_proc *, u_long, int, int);
+static char	**kvm_argv(kvm_t *, const struct kinfo_proc *, u_long, int, int, int);
 
-static char	**kvm_doargv(kvm_t *, const struct kinfo_proc *, int,
+static char	**kvm_doargv(kvm_t *, const struct kinfo_proc *, int, int,
 		    void (*)(struct ps_strings *, u_long *, int *));
 static int	proc_verify(kvm_t *, const struct kinfo_proc *);
 static void	ps_str_a(struct ps_strings *, u_long *, int *);
@@ -257,11 +258,12 @@ _kvm_reallocarray(kvm_t *kd, void *p, size_t i, size_t n)
  */
 static char **
 kvm_argv(kvm_t *kd, const struct kinfo_proc *p, u_long addr, int narg,
-    int maxcnt)
+    int maxcnt, int isenv)
 {
-	char *np, *cp, *ep, *ap, **argv;
+	char *np, *cp, *ep, *ap, **argv, ***pargv, **pargspc, **pargbuf;
 	u_long oaddr = -1;
-	int len, cc;
+	int len, cc, *parglen, *pargc;
+	size_t argc;
 
 	/*
 	 * Check that there aren't an unreasonable number of arguments,
@@ -270,77 +272,88 @@ kvm_argv(kvm_t *kd, const struct kinfo_proc *p, u_long addr, int narg,
 	if (narg > ARG_MAX || addr < VM_MIN_ADDRESS || addr >= VM_MAXUSER_ADDRESS)
 		return (0);
 
-	if (kd->argv == 0) {
-		/*
-		 * Try to avoid reallocs.
-		 */
-		kd->argc = MAX(narg + 1, 32);
-		kd->argv = _kvm_reallocarray(kd, NULL, kd->argc,
-		    sizeof(*kd->argv));
-		if (kd->argv == 0)
-			return (0);
-	} else if (narg + 1 > kd->argc) {
-		kd->argc = MAX(2 * kd->argc, narg + 1);
-		kd->argv = (char **)_kvm_reallocarray(kd, kd->argv, kd->argc,
-		    sizeof(*kd->argv));
-		if (kd->argv == 0)
-			return (0);
+	if (isenv) {
+		pargspc = &kd->envspc;
+		pargbuf = &kd->envbuf;
+		parglen = &kd->envlen;
+		pargv = &kd->envp;
+		pargc = &kd->envc;
+	} else {
+		pargspc = &kd->argspc;
+		pargbuf = &kd->argbuf;
+		parglen = &kd->arglen;
+		pargv = &kd->argv;
+		pargc = &kd->argc;
 	}
-	if (kd->argspc == 0) {
-		kd->argspc = _kvm_malloc(kd, kd->nbpg);
-		if (kd->argspc == 0)
+
+	if (*pargv == 0)
+		argc = MAX(narg + 1, 32);
+	else if (narg + 1 > *pargc)
+		argc = MAX(2 * (*pargc), narg + 1);
+	else
+		goto argv_allocated;
+	argv = _kvm_reallocarray(kd, *pargv, argc, sizeof(**pargv));
+	if (argv == 0)
+		return (0);
+	*pargv = argv;
+	*pargc = argc;
+
+argv_allocated:
+	if (*pargspc == 0) {
+		*pargspc = _kvm_malloc(kd, kd->nbpg);
+		if (*pargspc == 0)
 			return (0);
-		kd->arglen = kd->nbpg;
+		*parglen = kd->nbpg;
 	}
-	if (kd->argbuf == 0) {
-		kd->argbuf = _kvm_malloc(kd, kd->nbpg);
-		if (kd->argbuf == 0)
+	if (*pargbuf == 0) {
+		*pargbuf = _kvm_malloc(kd, kd->nbpg);
+		if (*pargbuf == 0)
 			return (0);
 	}
 	cc = sizeof(char *) * narg;
-	if (kvm_ureadm(kd, p, addr, (char *)kd->argv, cc) != cc)
+	if (kvm_ureadm(kd, p, addr, (char *)*pargv, cc) != cc)
 		return (0);
-	ap = np = kd->argspc;
-	argv = kd->argv;
+	ap = np = *pargspc;
+	argv = *pargv;
 	len = 0;
 
 	/*
 	 * Loop over pages, filling in the argument vector.
 	 */
-	while (argv < kd->argv + narg && *argv != 0) {
+	while (argv < *pargv + narg && *argv != 0) {
 		addr = (u_long)*argv & ~(kd->nbpg - 1);
 		if (addr != oaddr) {
-			if (kvm_ureadm(kd, p, addr, kd->argbuf, kd->nbpg) !=
+			if (kvm_ureadm(kd, p, addr, *pargbuf, kd->nbpg) !=
 			    kd->nbpg)
 				return (0);
 			oaddr = addr;
 		}
 		addr = (u_long)*argv & (kd->nbpg - 1);
-		cp = kd->argbuf + addr;
+		cp = *pargbuf + addr;
 		cc = kd->nbpg - addr;
 		if (maxcnt > 0 && cc > maxcnt - len)
 			cc = maxcnt - len;
 		ep = memchr(cp, '\0', cc);
 		if (ep != 0)
 			cc = ep - cp + 1;
-		if (len + cc > kd->arglen) {
-			int off;
+		if (len + cc > *parglen) {
+			ptrdiff_t off;
 			char **pp;
-			char *op = kd->argspc;
+			char *op = *pargspc;
 			char *newp;
 
-			newp = _kvm_reallocarray(kd, kd->argspc,
-			    kd->arglen, 2);
+			newp = _kvm_reallocarray(kd, *pargspc,
+			    *parglen, 2);
 			if (newp == 0)
 				return (0);
-			kd->argspc = newp;
-			kd->arglen *= 2;
+			*pargspc = newp;
+			*parglen *= 2;
 			/*
 			 * Adjust argv pointers in case realloc moved
 			 * the string space.
 			 */
-			off = kd->argspc - op;
-			for (pp = kd->argv; pp < argv; pp++)
+			off = *pargspc - op;
+			for (pp = *pargv; pp < argv; pp++)
 				*pp += off;
 			ap += off;
 			np += off;
@@ -367,7 +380,7 @@ kvm_argv(kvm_t *kd, const struct kinfo_proc *p, u_long addr, int narg,
 	}
 	/* Make sure argv is terminated. */
 	*argv = 0;
-	return (kd->argv);
+	return (*pargv);
 }
 
 static void
@@ -412,7 +425,7 @@ proc_verify(kvm_t *kd, const struct kinfo_proc *p)
 }
 
 static char **
-kvm_doargv(kvm_t *kd, const struct kinfo_proc *p, int nchr,
+kvm_doargv(kvm_t *kd, const struct kinfo_proc *p, int nchr, int isenv,
     void (*info)(struct ps_strings *, u_long *, int *))
 {
 	static struct ps_strings *ps;
@@ -444,7 +457,7 @@ kvm_doargv(kvm_t *kd, const struct kinfo_proc *p, int nchr,
 	(*info)(&arginfo, &addr, &cnt);
 	if (cnt == 0)
 		return (0);
-	ap = kvm_argv(kd, p, addr, cnt, nchr);
+	ap = kvm_argv(kd, p, addr, cnt, nchr, isenv);
 	/*
 	 * For live kernels, make sure this process didn't go away.
 	 */
@@ -454,47 +467,53 @@ kvm_doargv(kvm_t *kd, const struct kinfo_proc *p, int nchr,
 }
 
 static char **
-kvm_arg_sysctl(kvm_t *kd, pid_t pid, int nchr, int env)
+kvm_arg_sysctl(kvm_t *kd, pid_t pid, int nchr, int isenv)
 {
 	size_t len, orglen;
 	int mib[4], ret;
-	char *buf;
+	char *buf, **pargbuf;
 
-	orglen = env ? kd->nbpg : 8 * kd->nbpg;	/* XXX - should be ARG_MAX */
-	if (kd->argbuf == NULL &&
-	    (kd->argbuf = _kvm_malloc(kd, orglen)) == NULL)
+	if (isenv) {
+		pargbuf = &kd->envbuf;
+		orglen = kd->nbpg;
+	} else {
+		pargbuf = &kd->argbuf;
+		orglen = 8 * kd->nbpg;	/* XXX - should be ARG_MAX */
+	}
+	if (*pargbuf == NULL &&
+	    (*pargbuf = _kvm_malloc(kd, orglen)) == NULL)
 		return (NULL);
 
 again:
 	mib[0] = CTL_KERN;
 	mib[1] = KERN_PROC_ARGS;
 	mib[2] = (int)pid;
-	mib[3] = env ? KERN_PROC_ENV : KERN_PROC_ARGV;
+	mib[3] = isenv ? KERN_PROC_ENV : KERN_PROC_ARGV;
 
 	len = orglen;
-	ret = (sysctl(mib, 4, kd->argbuf, &len, NULL, 0) < 0);
+	ret = (sysctl(mib, 4, *pargbuf, &len, NULL, 0) < 0);
 	if (ret && errno == ENOMEM) {
-		buf = _kvm_reallocarray(kd, kd->argbuf, orglen, 2);
+		buf = _kvm_reallocarray(kd, *pargbuf, orglen, 2);
 		if (buf == NULL)
 			return (NULL);
 		orglen *= 2;
-		kd->argbuf = buf;
+		*pargbuf = buf;
 		goto again;
 	}
 
 	if (ret) {
-		free(kd->argbuf);
-		kd->argbuf = NULL;
+		free(*pargbuf);
+		*pargbuf = NULL;
 		_kvm_syserr(kd, kd->program, "kvm_arg_sysctl");
 		return (NULL);
 	}
 #if 0
-	for (argv = (char **)kd->argbuf; *argv != NULL; argv++)
+	for (argv = (char **)*pargbuf; *argv != NULL; argv++)
 		if (strlen(*argv) > nchr)
 			*argv[nchr] = '\0';
 #endif
 
-	return (char **)(kd->argbuf);
+	return (char **)(*pargbuf);
 }
 
 /*
@@ -505,7 +524,7 @@ kvm_getargv(kvm_t *kd, const struct kinfo_proc *kp, int nchr)
 {
 	if (ISALIVE(kd))
 		return (kvm_arg_sysctl(kd, kp->p_pid, nchr, 0));
-	return (kvm_doargv(kd, kp, nchr, ps_str_a));
+	return (kvm_doargv(kd, kp, nchr, 0, ps_str_a));
 }
 
 char **
@@ -513,7 +532,7 @@ kvm_getenvv(kvm_t *kd, const struct kinfo_proc *kp, int nchr)
 {
 	if (ISALIVE(kd))
 		return (kvm_arg_sysctl(kd, kp->p_pid, nchr, 1));
-	return (kvm_doargv(kd, kp, nchr, ps_str_e));
+	return (kvm_doargv(kd, kp, nchr, 1, ps_str_e));
 }
 
 /*