Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 9 Jul 2002 03:11:36 +1000 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        David Malone <dwmalone@maths.tcd.ie>
Cc:        Mike Makonnen <makonnen@pacbell.net>, <freebsd-current@FreeBSD.ORG>
Subject:   Re: benign bug in src/sys/kern/kern_resource.c:limcopy() ?
Message-ID:  <20020709025555.M2958-100000@gamplex.bde.org>
In-Reply-To: <20020708072042.GA49370@walton.maths.tcd.ie>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 8 Jul 2002, David Malone wrote:

> On Sun, Jul 07, 2002 at 03:28:51PM -0700, Mike Makonnen wrote:
> >  	MALLOC(copy, struct plimit *, sizeof(struct plimit),
> >  	    M_SUBPROC, M_WAITOK);
> > -	bcopy(lim->pl_rlimit, copy->pl_rlimit, sizeof(struct plimit));
> > +	bcopy(lim->pl_rlimit, copy->pl_rlimit, sizeof(struct rlimit));
>
> Since pl_rlimit is an array of struct rlimits, don't we want:
>
> bcopy(lim->pl_rlimit, copy->pl_rlimit, sizeof(struct rlimit)*RLIM_NLIMITS);

That's what we had in revs 1.1 through 1.36 (modulo style bugs).  It was
hacked on in rev.1.37 to copy p_cpulimit (p_cpulimit needs to be copied
but is not in the array of rlimits).

> or maybe:
>
> bcopy(&(lim->pl_rlimit[0]), &(copy->pl_rlimit[0]), sizeof(lim->pl_rlimit));
>
> rather than just copying the first limit?

Same bug.

> It might be better to just
> bcopy the whole struct plimit and make a note that other fields need
> to be reset.

That is better (without the note), and is what we do now but in doubly
obfuscated (we use bcopy() instead of struct assignment, and confuse the
pointers to the structs with pointers to the first member of the structs).
De-obfuscation gives:

Index: kern_resource.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_resource.c,v
retrieving revision 1.106
diff -u -2 -r1.106 kern_resource.c
--- kern_resource.c	29 Jun 2002 02:00:01 -0000	1.106
+++ kern_resource.c	8 Jul 2002 17:04:16 -0000
@@ -812,5 +825,5 @@
 	MALLOC(copy, struct plimit *, sizeof(struct plimit),
 	    M_SUBPROC, M_WAITOK);
-	bcopy(lim->pl_rlimit, copy->pl_rlimit, sizeof(struct plimit));
+	*copy = *lim;
 	copy->p_lflags = 0;
 	copy->p_refcnt = 1;

Revs 1.1 through 1.36 really did want to copy only part of the struct,
so struct assignment was not suitable for them.

Bruce


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020709025555.M2958-100000>