Date: Sat, 5 Nov 2011 17:44:43 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Mikolaj Golub <trociny@freebsd.org> Cc: freebsd-hackers@freebsd.org, Robert Watson <rwatson@freebsd.org> Subject: Re: "ps -e" without procfs(5) Message-ID: <20111105154443.GB50300@deviant.kiev.zoral.com.ua> In-Reply-To: <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> <86ehxmpogp.fsf@kopusha.home.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--08AydYmtx6Uix/Nv
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sat, Nov 05, 2011 at 05:40:22PM +0200, Mikolaj Golub wrote:
>=20
> On Sat, 5 Nov 2011 15:58:01 +0200 Kostik Belousov wrote:
>=20
> KB> + if (error =3D=3D EFAULT) {
> KB> + for (i =3D 0; i < len; i++) {
> KB> + c =3D fubyte(sptr + i);
> KB> + if (c < 0)
> KB> As a purely stylistical issue, compare with -1.
>=20
> KB> + return (EFAULT);
> KB> + buf[i] =3D (char)c;
> KB> + if (c =3D=3D '\0')
> KB> + break;
> KB> + }
> KB> + error =3D 0;
> KB> + }
> KB> + return error;
> KB> Put () around error.
>=20
> Thanks.
>=20
> KB> + /*
> KB> + * Check that that the address is in user space.
> KB> + */
> KB> + if (vptr + 1 < VM_MIN_ADDRESS + 1 || vptr >=3D VM_MAXUSER_A=
DDRESS)
> KB> + return (ENOEXEC);
> KB> Why is this needed ?
>=20
> I saw this check in libkvm for ps_argvstr and ps_envstr and decided to ad=
d 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 u=
seful
> I remove it.
The proc_rwmem() must handle access outside the user VA (and it does).
>=20
> KB> I think that the aux vector must be naturally aligned. You can return
> KB> ENOEXEC early if vptr is not aligned.
>=20
> Not sure I see what you mean. vptr for auxv is calculated just couple lin=
es
> above, and I check the result here, in the part common for all vector typ=
es.
You do not check for the alignment. Am I wrong ?
>=20
> BTW, investigating the cases when I got
>=20
> procstat: sysctl: kern.proc.args: 58002: 8: Exec format error
>=20
> they were because the PROC_VECTOR_MAX limit (512 entries, as it is in
> linprocfs and libkvm) is small for real world cases:
>=20
> get_proc_vector(pid =3D rm[47883], type =3D 0): vsize (3009) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[47883], type =3D 0): vsize (3009) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[47890], type =3D 0): vsize (3008) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[47890], type =3D 0): vsize (3008) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[47897], type =3D 0): vsize (4511) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[47897], type =3D 0): vsize (4511) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[47897], type =3D 0): vsize (4511) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[48044], type =3D 0): vsize (611) > PROC_VECTOR=
_MAX (512))
> get_proc_vector(pid =3D rm[52189], type =3D 0): vsize (772) > PROC_VECTOR=
_MAX (512))
> get_proc_vector(pid =3D rm[52192], type =3D 0): vsize (1157) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[55685], type =3D 0): vsize (1041) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[55687], type =3D 0): vsize (1040) > PROC_VECTO=
R_MAX (512))
> get_proc_vector(pid =3D rm[55690], type =3D 0): vsize (1559) > PROC_VECTO=
R_MAX (512))
>=20
> So I am going to change it to ARG_MAX and use independent limit (256 entr=
ies)
> for auxv.
Ok.
>=20
> KB> Why the blank after the loop statement in get_ps_strings() ?
>=20
> 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?
+ for (sptr =3D proc_vector[i]; ; sptr +=3D GET_PS_STRINGS_CHUNK_SZ) {
+
+ error =3D proc_read_mem(td, p, (vm_offset_t)sptr,
The line between for() and error =3D .
>=20
> KB> There shall be blank lines after the '{' in proc_getargv() and proc_=
getenvv().
>=20
> Ah, sure.
>=20
> KB> Note that using cached pargs is somewhat inconsistent with the diggi=
ng
> KB> into ps_strings.
>=20
> KB> procfs_doproccmdline() can benefit from your work.
>=20
> Thanks, I will look at it :-).
>=20
> --=20
> Mikolaj Golub
--08AydYmtx6Uix/Nv
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)
iEYEARECAAYFAk61WesACgkQC3+MBN1Mb4gRCQCeMB3QrNQvG3UWVDIE/lYLYhc8
59MAoPRQHL/caLPhBlC01+ZawijZKxoB
=FRlh
-----END PGP SIGNATURE-----
--08AydYmtx6Uix/Nv--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111105154443.GB50300>