On 09.12.2012 21:35, Alan Cox wrote:
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.
Thank you taking a second look it.
To be honest I got a bit confused on which automatic type expansion
applies here.
qmax() is defined as this in libkern.h:
static __inline quad_t qmax(quad_t a, quad_t b) { return (a > b ? a : b); }
Wouldn't physpages be expanded to quad_t? Hmm, no, only the result of the
computation, which happens in long space, is passed on. This is a function,
not a macro. Dang...
This change will force the computation to be in quad_t space:
- realmem = qmin(physpages * PAGE_SIZE,
+ realmem = qmin((quad_t)physpages * PAGE_SIZE,
VM_MAX_KERNEL_ADDRESS - VM_MIN_KERNEL_ADDRESS);
VM_[MAX|MIN]_KERNEL_ADDRESS is safe as it is of the unsigned vm_offset_t
type.
--
Andre
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;
D
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
