Date: Sat, 05 Nov 2011 17:40:22 +0200 From: Mikolaj Golub <trociny@freebsd.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: freebsd-hackers@freebsd.org, Robert Watson <rwatson@freebsd.org> Subject: Re: "ps -e" without procfs(5) Message-ID: <86ehxmpogp.fsf@kopusha.home.net> References: <86y5wkeuw9.fsf@kopusha.home.net> <20111016171005.GB50300@deviant.kiev.zoral.com.ua> <86aa8qozyx.fsf@kopusha.home.net> <20111025082451.GO50300@deviant.kiev.zoral.com.ua> <86aa8k2im0.fsf@kopusha.home.net> <20111031094948.GB50300@deviant.kiev.zoral.com.ua> <86vcr21agm.fsf@kopusha.home.net> <20111105135801.GT50300@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 5 Nov 2011 15:58:01 +0200 Kostik Belousov wrote:
KB> + if (error == EFAULT) {
KB> + for (i = 0; i < len; i++) {
KB> + c = fubyte(sptr + i);
KB> + if (c < 0)
KB> As a purely stylistical issue, compare with -1.
KB> + return (EFAULT);
KB> + buf[i] = (char)c;
KB> + if (c == '\0')
KB> + break;
KB> + }
KB> + error = 0;
KB> + }
KB> + return error;
KB> Put () around error.
Thanks.
KB> + /*
KB> + * Check that that the address is in user space.
KB> + */
KB> + if (vptr + 1 < VM_MIN_ADDRESS + 1 || vptr >= VM_MAXUSER_ADDRESS)
KB> + return (ENOEXEC);
KB> Why is this needed ?
I saw this check in libkvm for ps_argvstr and ps_envstr and decided to add it
too. Just some additional check that ps_string fields, which can be
overwritten by the user, look reasonable. If you think this is not very useful
I remove it.
KB> I think that the aux vector must be naturally aligned. You can return
KB> ENOEXEC early if vptr is not aligned.
Not sure I see what you mean. vptr for auxv is calculated just couple lines
above, and I check the result here, in the part common for all vector types.
BTW, investigating the cases when I got
procstat: sysctl: kern.proc.args: 58002: 8: Exec format error
they were because the PROC_VECTOR_MAX limit (512 entries, as it is in
linprocfs and libkvm) is small for real world cases:
get_proc_vector(pid = rm[47883], type = 0): vsize (3009) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[47883], type = 0): vsize (3009) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[47890], type = 0): vsize (3008) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[47890], type = 0): vsize (3008) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[47897], type = 0): vsize (4511) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[47897], type = 0): vsize (4511) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[47897], type = 0): vsize (4511) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[48044], type = 0): vsize (611) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[52189], type = 0): vsize (772) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[52192], type = 0): vsize (1157) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[55685], type = 0): vsize (1041) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[55687], type = 0): vsize (1040) > PROC_VECTOR_MAX (512))
get_proc_vector(pid = rm[55690], type = 0): vsize (1559) > PROC_VECTOR_MAX (512))
So I am going to change it to ARG_MAX and use independent limit (256 entries)
for auxv.
KB> Why the blank after the loop statement in get_ps_strings() ?
Sorry, what blank you mean? I don't see it in get_ps_strings(). May be you
mean the blank line in get_proc_vector() before return?
KB> There shall be blank lines after the '{' in proc_getargv() and proc_getenvv().
Ah, sure.
KB> Note that using cached pargs is somewhat inconsistent with the digging
KB> into ps_strings.
KB> procfs_doproccmdline() can benefit from your work.
Thanks, I will look at it :-).
--
Mikolaj Golub
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86ehxmpogp.fsf>