Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 09 Dec 2012 14:35:03 -0600
From:      Alan Cox <alc@rice.edu>
To:        Andre Oppermann <andre@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r243668 - in head/sys: kern sys
Message-ID:  <50C4F5F7.7080101@rice.edu>
In-Reply-To: <201211290730.qAT7Uhkv016745@svn.freebsd.org>
References:  <201211290730.qAT7Uhkv016745@svn.freebsd.org>

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

I believe that this change did not actually correct the overflow
problem.  See below for an explanation.

On 11/29/2012 01:30, Andre Oppermann wrote:
> Author: andre
> Date: Thu Nov 29 07:30:42 2012
> New Revision: 243668
> URL: http://svnweb.freebsd.org/changeset/base/243668
>
> Log:
>   Using a long is the wrong type to represent the realmem and maxmbufmem
>   variable as they may overflow on i386/PAE and i386 with > 2GB RAM.
>   
>   Use 64bit quad_t instead.  It has broader kernel infrastructure support
>   with TUNABLE_QUAD_FETCH() and qmin/qmax() than other available types.
>   
>   Pointed out by:	alc, bde
>
> Modified:
>   head/sys/kern/subr_param.c
>   head/sys/sys/mbuf.h
>
> Modified: head/sys/kern/subr_param.c
> ==============================================================================
> --- head/sys/kern/subr_param.c	Thu Nov 29 06:26:42 2012	(r243667)
> +++ head/sys/kern/subr_param.c	Thu Nov 29 07:30:42 2012	(r243668)
> @@ -93,7 +93,7 @@ int	ncallout;			/* maximum # of timer ev
>  int	nbuf;
>  int	ngroups_max;			/* max # groups per process */
>  int	nswbuf;
> -long	maxmbufmem;			/* max mbuf memory */
> +quad_t	maxmbufmem;			/* max mbuf memory */
>  pid_t	pid_max = PID_MAX;
>  long	maxswzone;			/* max swmeta KVA storage */
>  long	maxbcache;			/* max buffer cache KVA storage */
> @@ -271,7 +271,7 @@ init_param1(void)
>  void
>  init_param2(long physpages)
>  {
> -	long realmem;
> +	quad_t realmem;
>  
>  	/* Base parameters */
>  	maxusers = MAXUSERS;
> @@ -332,10 +332,10 @@ init_param2(long physpages)
>  	 * available kernel memory (physical or kmem).
>  	 * At most it can be 3/4 of available kernel memory.
>  	 */
> -	realmem = lmin(physpages * PAGE_SIZE,
> +	realmem = qmin(physpages * PAGE_SIZE,
>  			VM_MAX_KERNEL_ADDRESS - VM_MIN_KERNEL_ADDRESS);


"physpages" is a signed long.  Suppose it is 1,000,000.  On i386/PAE,
the product of 1,000,000 and PAGE_SIZE will be a negative number. 
Likewise, quad_t is a signed type.  So, the negative product of
1,000,000 and PAGE_SIZE will be sign extended to a 64-bit signed value
when it is passed to qmin(), and qmin() will return a negative number.


>  	maxmbufmem = realmem / 2;
> -	TUNABLE_LONG_FETCH("kern.maxmbufmem", &maxmbufmem);
> +	TUNABLE_QUAD_FETCH("kern.maxmbufmem", &maxmbufmem);
>  	if (maxmbufmem > (realmem / 4) * 3)
>  		maxmbufmem = (realmem / 4) * 3;
>  
>
> Modified: head/sys/sys/mbuf.h
> ==============================================================================
> --- head/sys/sys/mbuf.h	Thu Nov 29 06:26:42 2012	(r243667)
> +++ head/sys/sys/mbuf.h	Thu Nov 29 07:30:42 2012	(r243668)
> @@ -395,7 +395,7 @@ struct mbstat {
>   *
>   * The rest of it is defined in kern/kern_mbuf.c
>   */
> -extern long		maxmbufmem;
> +extern quad_t		maxmbufmem;
>  extern uma_zone_t	zone_mbuf;
>  extern uma_zone_t	zone_clust;
>  extern uma_zone_t	zone_pack;
>




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50C4F5F7.7080101>