From owner-freebsd-hackers@FreeBSD.ORG Sat Nov 5 15:44:48 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E9EFC1065670; Sat, 5 Nov 2011 15:44:47 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 858DD8FC08; Sat, 5 Nov 2011 15:44:47 +0000 (UTC) Received: from alf.home (alf.kiev.zoral.com.ua [10.1.1.177]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id pA5Fihi0020514 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 5 Nov 2011 17:44:43 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: from alf.home (kostik@localhost [127.0.0.1]) by alf.home (8.14.5/8.14.5) with ESMTP id pA5FihsP051342; Sat, 5 Nov 2011 17:44:43 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by alf.home (8.14.5/8.14.5/Submit) id pA5FihNr051341; Sat, 5 Nov 2011 17:44:43 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: alf.home: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 5 Nov 2011 17:44:43 +0200 From: Kostik Belousov To: Mikolaj Golub Message-ID: <20111105154443.GB50300@deviant.kiev.zoral.com.ua> 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> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="08AydYmtx6Uix/Nv" Content-Disposition: inline In-Reply-To: <86ehxmpogp.fsf@kopusha.home.net> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.9 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-hackers@freebsd.org, Robert Watson Subject: Re: "ps -e" without procfs(5) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Nov 2011 15:44:48 -0000 --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--