svn commit: r212723 - head/sys/compat/linprocfs

[pluknet]

2010/9/24 Kostik Belousov <kostikbel at gmail.com>:
> On Fri, Sep 24, 2010 at 02:17:29PM +0400, pluknet wrote:
>> On 16 September 2010 11:56, Dag-Erling Smorgrav <des at 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:
>> >  Implement proc/$$/environment.
>> >
>> [...]
>>
>> >  /*
>> >  * Filler function for proc/pid/environ
>> >  */
>> >  static int
>> >  linprocfs_doprocenviron(PFS_FILL_ARGS)
>> >  {
>> > +       int ret;
>> >
>> > -       sbuf_printf(sb, "doprocenviron\n%c", '\0');
>> > -       return (0);
>> > +       PROC_LOCK(p);
>>
>> With this change I observe the following sleepable after non-sleepable:
>>
[LOR there]
>>
>>
>> > +
>> > +       if ((ret = p_cansee(td, p)) != 0) {
>> > +               PROC_UNLOCK(p);
>> > +               return ret;
>> > +       }
>> > +
>> > +       ret = linprocfs_doargv(td, p, sb, ps_string_env);
>> > +       PROC_UNLOCK(p);
>> > +       return (ret);
>> >  }
>
> 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.

-- 
wbr,
pluknet