Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 01 Nov 2011 09:07:11 +0200
From:      Mikolaj Golub <trociny@freebsd.org>
To:        Kostik Belousov <kostikbel@gmail.com>
Cc:        freebsd-hackers@freebsd.org, Robert Watson <rwatson@freebsd.org>
Subject:   Re: "ps -e" without procfs(5)
Message-ID:  <86boswjp7k.fsf@in138.ua3>
In-Reply-To: <20111031094948.GB50300@deviant.kiev.zoral.com.ua> (Kostik Belousov's message of "Mon, 31 Oct 2011 11:49:48 %2B0200")
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>

next in thread | previous in thread | raw e-mail | index | archive | help

On Mon, 31 Oct 2011 11:49:48 +0200 Kostik Belousov wrote:

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

After looking at it more closely, I am not sure if I need to use
exec_alloc_args. I malloc explicitly only for array vector (proc_vector). And
actually it should be much smaller than 2 * (PATH_MAX + ARG_MAX). Currently in
linprocfs the limit is 512 entries:

        #define MAX_ARGV_STR    512     /* Max number of argv-like strings */

The same limit is in libkvm:

        /*
         * Check that there aren't an unreasonable number of arguments,
         * and that the address is in user space.  Special test for
         * VM_MIN_ADDRESS as it evaluates to zero, but is not a simple zero
         * constant for some archs.  We cannot use the pre-processor here and
         * for some archs the compiler would trigger a signedness warning.
         */
        if (narg > 512 || addr + 1 < VM_MIN_ADDRESS + 1 || addr >= VM_MAXUSER_ADDRESS)
                return (0);

(BTW, may be the VM_MIN_ADDRESS - VM_MAXUSER_ADDRESS is worth adding in my
code too?)

So it looks like I should use the same limit (512 * sizeof(char *)) for the
allocated array. I could use exec_alloc_args() for the allocation but it would
reqire some changes: I would have to free using kmem_free_wakeup(), which
requires size of the region, while I return the number of entries. So I'd
rather not use exec_alloc_args() for vector allocation because the benefit is
not significant here.

For strings I use sbuf and set it up using sbuf_new_for_sysctl. I could set it
up manually as SBUF_FIXEDLEN allocating buf (up to 2 * (PATH_MAX + ARG_MAX))
with exec_alloc_args() but this would complicate things a little. Do you think
it is worth doing?

-- 
Mikolaj Golub



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86boswjp7k.fsf>