Date: Wed, 9 Nov 2011 14:53:29 +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: <20111109125329.GX50300@deviant.kiev.zoral.com.ua> In-Reply-To: <20111109124455.GW50300@deviant.kiev.zoral.com.ua> References: <86vcr21agm.fsf@kopusha.home.net> <20111105135801.GT50300@deviant.kiev.zoral.com.ua> <86ehxmpogp.fsf@kopusha.home.net> <20111105154443.GB50300@deviant.kiev.zoral.com.ua> <86ehxmjsza.fsf@kopusha.home.net> <20111105194553.GK50300@deviant.kiev.zoral.com.ua> <8662iyjof9.fsf@kopusha.home.net> <20111106181041.GH50300@deviant.kiev.zoral.com.ua> <86r51iqoad.fsf@kopusha.home.net> <20111109124455.GW50300@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
--WN6YUpv1TkvwTGnb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 09, 2011 at 02:44:55PM +0200, Kostik Belousov wrote: > On Tue, Nov 08, 2011 at 11:47:54PM +0200, Mikolaj Golub wrote: > >=20 > > On Sun, 6 Nov 2011 20:10:41 +0200 Kostik Belousov wrote: > >=20 > > KB> On Sat, Nov 05, 2011 at 10:37:46PM +0200, Mikolaj Golub wrote: > > >>=20 > > >> http://people.freebsd.org/~trociny/env.sys.3.patch > >=20 > > KB> Oops, I missed this in the previous review. You cannot use fubyte = in > > KB> proc_read_mem(). fubyte reads a byte from the address space of the= current > > KB> process. The fix is easy, use proc_rwmem for 1 byte. > >=20 > > KB> I do not think that fall back to single byte read is warranted for > > KB> proc_read_mem calls e.g. for ps_strings. Add a flag to indicate wh= ether > > KB> the proc_read_mem should fall back to byte read ? > >=20 > > KB> I would prefer using sizeof(uint64_t) and sizeof(uint32_t) instead= of 8 > > KB> and 4 constants in the align checks. > >=20 > > KB> Might be, add PROC_ASSERT_HELD() to get_ps_string() ? > >=20 > > KB> procfs patch looks good. > >=20 > > Thanks. The updated version: > >=20 > > http://people.freebsd.org/~trociny/env.sys.4.patch > >=20 > > Investigating cases when EFAULT was returned and if the fallback was > > successful I noticed that most of the cases were when p->p_comm changed= during > > the read, so the process was in exec in that time. In order to avoid th= is > > error I added a check for P_INEXEC flag. > And now you return success and nothing gets copied out for the process > in P_INEXEC state. Either you should return an error like EAGAIN, or > consider the P_INEXEC state as transitional and wait till process > leaves it. Or, ignore the state as it was before, and return whatever > error proc_rwmem generated (my preference). Forgot to say that the check does not change much because you drop process lock immediately after the check, so the process may enter the INEXEC state right after the check. I believe you already tried to do this with P_WEXIT. >=20 > >=20 > > After this I observed EFAULT (very rarely) only when reading arg or env > > strings and fallback was successful for those cases. So I modified the = patch > > to do fallback only when reading strings (as it was in one of my earlier > > versions but with wrong fubyte), and returned your comment which explai= ns why > > it may happen :-) > >=20 > > Also in the procfs patch I have added the check for process state. > >=20 > > The userland part has not been changed since my first report: > >=20 > > http://people.freebsd.org/~trociny/env.user.patch > >=20 > > --=20 > > Mikolaj Golub --WN6YUpv1TkvwTGnb Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iEYEARECAAYFAk66d8kACgkQC3+MBN1Mb4hIiQCeN5R/OyDpj0fpaib4NDklEqGW f1MAnAv1fqRzd56yGcX+2toiTN0lLf4F =vYoF -----END PGP SIGNATURE----- --WN6YUpv1TkvwTGnb--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111109125329.GX50300>