From owner-freebsd-current Mon Jul 8 10: 8:50 2002 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D941637B400 for ; Mon, 8 Jul 2002 10:08:46 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id 02DFA43E42 for ; Mon, 8 Jul 2002 10:08:46 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id DAA14017; Tue, 9 Jul 2002 03:08:31 +1000 Date: Tue, 9 Jul 2002 03:11:36 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: David Malone Cc: Mike Makonnen , Subject: Re: benign bug in src/sys/kern/kern_resource.c:limcopy() ? In-Reply-To: <20020708072042.GA49370@walton.maths.tcd.ie> Message-ID: <20020709025555.M2958-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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