Subject: Re: setrlimit(2) strange behaviour under compat_netbsd32
To: Quentin Garnier <cube@cubidou.net>
From: Nicolas Joly <njoly@pasteur.fr>
List: current-users
Date: 11/21/2006 14:59:00
--45Z9DzgjV8m4Oswq
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Nov 21, 2006 at 12:48:59PM +0100, Quentin Garnier wrote:
> On Tue, Nov 21, 2006 at 12:17:24PM +0100, Nicolas Joly wrote:
> > On Mon, Nov 20, 2006 at 08:42:53PM +0100, Nicolas Joly wrote:
> > > 
> > > I just noticed a strange setrlimit(2) behaviour under compat_netbsd32
> > > (and compat_linux32) on my -current amd64 workstation ...
> > > 
> > > It seems that 32bits programs, running under compat_netbsd32, using
> > > setrlimit force all other programs to have their maximum data size
> > > fixed at 3GB, where native 64bits apps used 8GB previously.
> > 
> > I tracked this one to the `netbsd32_adjust_limits()' function (called
> > when creating a new process under compat_netbsd32), where data and
> > stack limits are set without checking for shared `p_limit' structure
> > (p_limit->p_refcnt > 1). This explain the side effect where processes
> > have their limits changed when a compat_netbsd32 (or compat_linux32)
> > program is run.
> > 
> > What about the attached patch, that use `dosetrlimit()' to ensure the
> > needed copy-on-write behaviour for shared structure.
> 
> What bugs me here is that dosetrlimit() does a lot of stuff we don't
> need at that point (like, say, returning a value), or more importantly,
> don't want:  the kauth test (it uses the process's creds).

Ok.

> Why don't you just grab the refcnt test from dosetrlimit() before you
> do the adjustments?

Like this one ?

-- 
Nicolas Joly

Biological Software and Databanks.
Institut Pasteur, Paris.

--45Z9DzgjV8m4Oswq
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="netbsd-netbsd32limits2.diff"

Index: sys/compat/netbsd32/netbsd32_netbsd.c
===================================================================
RCS file: /cvsroot/src/sys/compat/netbsd32/netbsd32_netbsd.c,v
retrieving revision 1.112
diff -u -r1.112 netbsd32_netbsd.c
--- sys/compat/netbsd32/netbsd32_netbsd.c	14 Nov 2006 13:34:29 -0000	1.112
+++ sys/compat/netbsd32/netbsd32_netbsd.c	21 Nov 2006 13:11:53 -0000
@@ -2314,21 +2314,41 @@
 void
 netbsd32_adjust_limits(struct proc *p)
 {
-	rlim_t *valp;
+	rlim_t valp;
+	struct plimit *oldplim;
 
-	valp = &p->p_rlimit[RLIMIT_DATA].rlim_cur;
-	if (*valp != RLIM_INFINITY && *valp > MAXDSIZ32)
-		*valp = MAXDSIZ32;
-	valp = &p->p_rlimit[RLIMIT_DATA].rlim_max;
-	if (*valp != RLIM_INFINITY && *valp > MAXDSIZ32)
-		*valp = MAXDSIZ32;
-
-	valp = &p->p_rlimit[RLIMIT_STACK].rlim_cur;
-	if (*valp != RLIM_INFINITY && *valp > MAXSSIZ32)
-		*valp = MAXSSIZ32;
-	valp = &p->p_rlimit[RLIMIT_STACK].rlim_max;
-	if (*valp != RLIM_INFINITY && *valp > MAXSSIZ32)
-		*valp = MAXSSIZ32;
+#define LIMCHKCOPY() \
+	do {                                                        \
+		if (p->p_limit->p_refcnt > 1 &&                     \
+		    (p->p_limit->p_lflags & PL_SHAREMOD) == 0) {    \
+			p->p_limit = limcopy(oldplim = p->p_limit); \
+			limfree(oldplim);                           \
+		}                                                   \
+	} while (/*CONSTCOND*/0)
+
+	valp = p->p_rlimit[RLIMIT_DATA].rlim_cur;
+	if (valp != RLIM_INFINITY && valp > MAXDSIZ32) {
+		LIMCHKCOPY();
+		p->p_rlimit[RLIMIT_DATA].rlim_cur = MAXDSIZ32;
+	}
+	valp = p->p_rlimit[RLIMIT_DATA].rlim_max;
+	if (valp != RLIM_INFINITY && valp > MAXDSIZ32) {
+		LIMCHKCOPY();
+		p->p_rlimit[RLIMIT_DATA].rlim_max = MAXDSIZ32;
+	}
+
+	valp = p->p_rlimit[RLIMIT_STACK].rlim_cur;
+	if (valp != RLIM_INFINITY && valp > MAXSSIZ32) {
+		LIMCHKCOPY();
+		p->p_rlimit[RLIMIT_STACK].rlim_cur = MAXSSIZ32;
+	}
+	valp = p->p_rlimit[RLIMIT_STACK].rlim_max;
+	if (valp != RLIM_INFINITY && valp > MAXSSIZ32) {
+		LIMCHKCOPY();
+		p->p_rlimit[RLIMIT_STACK].rlim_max = MAXSSIZ32;
+	}
+
+#undef LIMCHKCOPY
 }
 
 int

--45Z9DzgjV8m4Oswq--