Date: Fri, 24 Sep 2010 16:24:18 +0400 From: pluknet <pluknet@gmail.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Dag-Erling Smorgrav <des@freebsd.org> Subject: Re: svn commit: r212723 - head/sys/compat/linprocfs Message-ID: <AANLkTimd9oqJ2sFMvTpGBXUZwJmYeex1frRnQTJ_tgt%2B@mail.gmail.com> In-Reply-To: <20100924115311.GH34228@deviant.kiev.zoral.com.ua> References: <201009160756.o8G7uZrg065332@svn.freebsd.org> <AANLkTi=20QDe6o2YxM8PTKOLXB3ZuxUCNQEPtWy9P4Rc@mail.gmail.com> <20100924115311.GH34228@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
2010/9/24 Kostik Belousov <kostikbel@gmail.com>: > On Fri, Sep 24, 2010 at 02:17:29PM +0400, pluknet wrote: >> On 16 September 2010 11:56, Dag-Erling Smorgrav <des@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: >> > =A0Implement proc/$$/environment. >> > >> [...] >> >> > =A0/* >> > =A0* Filler function for proc/pid/environ >> > =A0*/ >> > =A0static int >> > =A0linprocfs_doprocenviron(PFS_FILL_ARGS) >> > =A0{ >> > + =A0 =A0 =A0 int ret; >> > >> > - =A0 =A0 =A0 sbuf_printf(sb, "doprocenviron\n%c", '\0'); >> > - =A0 =A0 =A0 return (0); >> > + =A0 =A0 =A0 PROC_LOCK(p); >> >> With this change I observe the following sleepable after non-sleepable: >> [LOR there] >> >> >> > + >> > + =A0 =A0 =A0 if ((ret =3D p_cansee(td, p)) !=3D 0) { >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 PROC_UNLOCK(p); >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return ret; >> > + =A0 =A0 =A0 } >> > + >> > + =A0 =A0 =A0 ret =3D linprocfs_doargv(td, p, sb, ps_string_env); >> > + =A0 =A0 =A0 PROC_UNLOCK(p); >> > + =A0 =A0 =A0 return (ret); >> > =A0} > > 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. --=20 wbr, pluknet
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimd9oqJ2sFMvTpGBXUZwJmYeex1frRnQTJ_tgt%2B>