svn commit: r212723 - head/sys/compat/linprocfs
Kostik Belousov
kostikbel at gmail.com
Fri Sep 24 13:06:53 UTC 2010
On Fri, Sep 24, 2010 at 04:24:18PM +0400, pluknet wrote:
> 2010/9/24 Kostik Belousov <kostikbel at gmail.com>:
> > On Fri, Sep 24, 2010 at 02:17:29PM +0400, pluknet wrote:
> >> On 16 September 2010 11:56, Dag-Erling Smorgrav <des at freebsd.org> wrote:
> >> > Author: des
> >> > Date: Thu Sep 16 07:56:34 2010
> >> > New Revision: 212723
> >> > URL: http://svn.freebsd.org/changeset/base/212723
> >> >
> >> > Log:
> >> > Implement proc/$$/environment.
> >> >
> >> [...]
> >>
> >> > /*
> >> > * Filler function for proc/pid/environ
> >> > */
> >> > static int
> >> > linprocfs_doprocenviron(PFS_FILL_ARGS)
> >> > {
> >> > + int ret;
> >> >
> >> > - sbuf_printf(sb, "doprocenviron\n%c", '\0');
> >> > - return (0);
> >> > + PROC_LOCK(p);
> >>
> >> With this change I observe the following sleepable after non-sleepable:
> >>
> [LOR there]
> >>
> >>
> >> > +
> >> > + if ((ret = p_cansee(td, p)) != 0) {
> >> > + PROC_UNLOCK(p);
> >> > + return ret;
> >> > + }
> >> > +
> >> > + ret = linprocfs_doargv(td, p, sb, ps_string_env);
> >> > + PROC_UNLOCK(p);
> >> > + return (ret);
> >> > }
> >
> > This is easy to fix, isn't it ? But there seems to be much more nits.
> >
> > First, allocating 512 * sizeof(char *)-byte object on the stack is not
> > good.
> >
> > Second, the initialization of iov_len for reading the array
> > of string pointers misses '* sizeof(char *)'.
> >
> > And third (probably fatal) is the lack of checks that the end of
> > array and each string fits into the user portion of the map. I do not
> > see why addr that already has u_long type is casted to u_long. Also,
> > VM_MIN_ADDRESS, VM_MAXUSER_ADDRESS constants are for the native host
> > FreeBSD ABI, they may differ from the target process limits.
> >
>
> Thanks for quick reaction.
>
> As for the latter, something doesn't quite work here.
> I see EFAULT against i386 process running on amd64.
Right, the COMPAT_FREEBSD32 case should properly convert both
struct ps_strings and array of pointers to the host structures.
So this is fourth issue.
diff --git a/sys/compat/linprocfs/linprocfs.c b/sys/compat/linprocfs/linprocfs.c
index c7fe158..db837e2 100644
--- a/sys/compat/linprocfs/linprocfs.c
+++ b/sys/compat/linprocfs/linprocfs.c
@@ -96,6 +96,10 @@ __FBSDID("$FreeBSD$");
#include <machine/md_var.h>
#endif /* __i386__ || __amd64__ */
+#ifdef COMPAT_FREEBSD32
+#include <compat/freebsd32/freebsd32_util.h>
+#endif
+
#ifdef COMPAT_LINUX32 /* XXX */
#include <machine/../linux32/linux.h>
#else
@@ -951,13 +955,14 @@ linprocfs_doargv(struct thread *td, struct proc *p, struct sbuf *sb,
struct iovec iov;
struct uio tmp_uio;
struct ps_strings pss;
- int ret, i, n_elements, found_end;
- u_long addr;
- char* env_vector[MAX_ARGV_STR];
+ int ret, i, n_elements, elm_len, found_end;
+ u_long addr, pbegin;
+ char **env_vector;
char env_string[UIO_CHUNK_SZ];
- char *pbegin;
-
-
+#ifdef COMPAT_FREEBSD32
+ struct freebsd32_ps_strings pss32;
+ uint32_t ptr;
+#endif
#define UIO_HELPER(uio, iov, base, len, cnt, offset, sz, flg, rw, td) \
do { \
@@ -971,63 +976,112 @@ do { \
uio.uio_rw = (rw); \
uio.uio_td = (td); \
} while (0)
-
- UIO_HELPER(tmp_uio, iov, &pss, sizeof(struct ps_strings), 1,
- (off_t)(p->p_sysent->sv_psstrings), sizeof(struct ps_strings),
- UIO_SYSSPACE, UIO_READ, td);
-
- ret = proc_rwmem(p, &tmp_uio);
- if (ret != 0)
- return ret;
+#define VALID_USER_ADDR(addr) \
+ ((addr) >= p->p_sysent->sv_minuser && (addr) < p->p_sysent->sv_maxuser)
+
+ env_vector = malloc(sizeof(char *) * MAX_ARGV_STR, M_TEMP, M_WAITOK);
+
+#ifdef COMPAT_FREEBSD32
+ if ((p->p_sysent->sv_flags & SV_ILP32) != 0) {
+ UIO_HELPER(tmp_uio, iov, &pss32, sizeof(pss32), 1,
+ (off_t)(p->p_sysent->sv_psstrings),
+ sizeof(pss32), UIO_SYSSPACE, UIO_READ, td);
+ ret = proc_rwmem(p, &tmp_uio);
+ if (ret != 0)
+ goto done;
+ pss.ps_argvstr = PTRIN(pss32.ps_argvstr);
+ pss.ps_nargvstr = pss32.ps_nargvstr;
+ pss.ps_envstr = PTRIN(pss32.ps_envstr);
+ pss.ps_nenvstr = pss32.ps_nenvstr;
+
+ elm_len = MAX_ARGV_STR * sizeof(int32_t);
+ } else {
+#endif
+ UIO_HELPER(tmp_uio, iov, &pss, sizeof(pss), 1,
+ (off_t)(p->p_sysent->sv_psstrings),
+ sizeof(pss), UIO_SYSSPACE, UIO_READ, td);
+ ret = proc_rwmem(p, &tmp_uio);
+ if (ret != 0)
+ goto done;
+
+ elm_len = MAX_ARGV_STR * sizeof(char *);
+#ifdef COMPAT_FREEBSD32
+ }
+#endif
/* Get the array address and the number of elements */
resolver(pss, &addr, &n_elements);
/* Consistent with lib/libkvm/kvm_proc.c */
- if (n_elements > MAX_ARGV_STR || (u_long)addr < VM_MIN_ADDRESS ||
- (u_long)addr >= VM_MAXUSER_ADDRESS) {
- /* What error code should we return? */
- return 0;
+ if (n_elements > MAX_ARGV_STR || !VALID_USER_ADDR(addr) ||
+ !VALID_USER_ADDR(addr + elm_len)) {
+ ret = EFAULT;
+ goto done;
}
- UIO_HELPER(tmp_uio, iov, env_vector, MAX_ARGV_STR, 1,
- (vm_offset_t)(addr), iov.iov_len, UIO_SYSSPACE, UIO_READ, td);
+#ifdef COMPAT_FREEBSD32
+ if ((p->p_sysent->sv_flags & SV_ILP32) != 0) {
+ for (i = 0; i < MAX_ARGV_STR; i++) {
+ UIO_HELPER(tmp_uio, iov, &ptr, sizeof(ptr), 1,
+ (vm_offset_t)(addr + i * sizeof(uint32_t)),
+ iov.iov_len, UIO_SYSSPACE, UIO_READ, td);
+ ret = proc_rwmem(p, &tmp_uio);
+ if (ret != 0)
+ goto done;
+ env_vector[i] = PTRIN(ptr);
+ }
+ } else {
+#endif
+ UIO_HELPER(tmp_uio, iov, env_vector, elm_len, 1,
+ (vm_offset_t)(addr), iov.iov_len, UIO_SYSSPACE, UIO_READ,
+ td);
+ ret = proc_rwmem(p, &tmp_uio);
+ if (ret != 0)
+ goto done;
+#ifdef COMPAT_FREEBSD32
+ }
+#endif
- ret = proc_rwmem(p, &tmp_uio);
- if (ret != 0)
- return ret;
/* Now we can iterate through the list of strings */
for (i = 0; i < n_elements; i++) {
found_end = 0;
- pbegin = env_vector[i];
- while(!found_end) {
+ pbegin = (vm_offset_t)env_vector[i];
+ while (!found_end) {
+ if (!VALID_USER_ADDR(pbegin) ||
+ !VALID_USER_ADDR(pbegin + sizeof(env_string))) {
+ ret = EFAULT;
+ goto done;
+ }
UIO_HELPER(tmp_uio, iov, env_string, sizeof(env_string), 1,
- (vm_offset_t) pbegin, iov.iov_len, UIO_SYSSPACE,
- UIO_READ, td);
+ pbegin, iov.iov_len, UIO_SYSSPACE, UIO_READ, td);
- ret = proc_rwmem(p, &tmp_uio);
- if (ret != 0)
- return ret;
+ ret = proc_rwmem(p, &tmp_uio);
+ if (ret != 0) {
+ ret = 0;
+ goto done;
+ }
- if (!strvalid(env_string, UIO_CHUNK_SZ)) {
+ if (!strvalid(env_string, UIO_CHUNK_SZ)) {
/*
- * We didn't find the end of the string
+ * We didn't find the end of the string.
* Add the string to the buffer and move
- * the pointer
+ * the pointer.
*/
sbuf_bcat(sb, env_string, UIO_CHUNK_SZ);
- pbegin = &(*pbegin) + UIO_CHUNK_SZ;
- } else {
+ pbegin += UIO_CHUNK_SZ;
+ } else
found_end = 1;
- }
}
sbuf_printf(sb, "%s", env_string);
}
+#undef VALID_USER_ADDR
#undef UIO_HELPER
- return (0);
+done:
+ free(env_vector, M_TEMP);
+ return (ret);
}
static void
@@ -1052,9 +1106,11 @@ linprocfs_doprocenviron(PFS_FILL_ARGS)
PROC_UNLOCK(p);
return ret;
}
+ _PHOLD(p);
+ PROC_UNLOCK(p);
ret = linprocfs_doargv(td, p, sb, ps_string_env);
- PROC_UNLOCK(p);
+ PRELE(p);
return (ret);
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20100924/4ca895db/attachment.pgp
More information about the svn-src-head
mailing list