"ps -e" without procfs(5)

[Kostik Belousov]

On Sat, Oct 29, 2011 at 01:32:39PM +0300, Mikolaj Golub wrote:
> 
> On Tue, 25 Oct 2011 11:24:51 +0300 Kostik Belousov wrote:
> 
>  KB> On Tue, Oct 25, 2011 at 12:13:10AM +0300, Mikolaj Golub wrote:
>  >> 
>  >> On Sun, 16 Oct 2011 20:10:05 +0300 Kostik Belousov wrote:
>  >> 
>  >>  KB> In my opinion, the way to implement the feature is to (re)use
>  >>  KB> linprocfs_doargv() and provide another kern.proc sysctl to retrieve the
>  >>  KB> argv and env vectors. Then, ps(1) and procstat(1) can use it, as well as
>  >>  KB> procfs and linprocfs inside the kernel.
>  >> 
>  >> Thanks! I am testing a patch (without auxv vector so far) and have some
>  >> questions.
>  >> 
>  >> Original ps -e returns environment only for user owned processes (the access is
>  >> restricted by the permissions of /proc/pid/mem file). My kern.proc.env sysctl
>  >> does not have such a restriction. I suppose I should add it? What function I
>  >> could use for this?
>  >> 
>  >> BTW, linprocfs allows to read other user's environment.
>  KB> linprocfs uses p_cansee() to check the permissions. There are sysctls
>  KB> security.bsd.see_other_{ug}ids that control the behaviour.
> 
>  KB> I believe that the new sysctl shall use the same check.
> 
>  >> 
>  >>  KB> While you are at the code, it would be useful to also export the auxv vector,
>  >>  KB> which is immediately before env.
>  >> 
>  >> It looks I can find the location of auxv but what about the size? Or do you
>  >> propose to extend struct ps_strings to store location and size of auxv? I
>  >> could do this way...
> 
>  KB> No, extending ps_strings is not needed and it is too radical change.
>  KB> The auxv vector must end by the AT_NULL aux entry. You can also artificially
>  KB> limit the amount of read aux vectors to, say, 256, which is much more then
>  KB> it is currently defined.
> 
> What do you think about the attached patch? This is a kernel
> part. COMPAT_FREEBSD32 has not been tested after the last update (just checked
> that it compiles): it looks I will not have access to amd64 box for testing
> during the weekend. I will test it after the weekend.
> 
> Both kernel and userland parts are available here:
> 
> http://people.freebsd.org/~trociny/env.sys.patch
> http://people.freebsd.org/~trociny/env.user.patch
> 
> Currently there is an issue with procstat -x: if one tried to run it on 64 bit
> for a 32 bit process it would not detect this so would output a garbage. Could
> somebody recommend a way how to get this info about a process from userlend?

I think it is better to use sys/elf.h over the machine/elf.h.

Please change the comment for PROC_AUXV_MAX to "Safety limit on auxv size".
Also, it worth adding a comment saying that we are reading aux vectors twice,
first to get a size, second time to fetch a content, for simplicity.

When reading aux vector, if the PROC_AUXV_MAX entries are iterated over,
and we still not reached AT_NULL, the return error is 0. Was it intended ?

For PROC_ARG and PROC_ENV, you blindly trust the read values of the arg and
env vector sizes. This can easily cause kernel panics due to unability to
malloc the requested memory. I recommend to put some clump, and twice
of (PATH_MAX + ARG_MAX) is probably enough (see kern_exec.c, in particular,
exec_alloc_args). Also, you might use the swappable memory for the strings
as well, in the style of exec_alloc_args().

I suspect this is my bug: Reading the GET_PS_STRINGS_CHUNK_SZ may validly
return EFAULT if the string is shorter than the chunk and aligned at
the end of the page, assuming the next page is not mapped. There should
be a fallback to fubyte() read loop. I remember that copyinstr() was
unsuitable.

The checks for P_WEXIT in the linprocfs routines look strange. Since
you are unlocking the process right after the check, it does not make
sense. In fact, the checks are not needed, I believe, since pseudofs
already did the hold (see e.g. pfs_read and pfs_visible).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20111031/1699e82c/attachment.pgp