Re: benign bug in src/sys/kern/kern_resource.c:limcopy() ?

2002-07-08 Thread Bruce Evans

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 -  1.106
+++ kern_resource.c 8 Jul 2002 17:04:16 -
@@ -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 [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: benign bug in src/sys/kern/kern_resource.c:limcopy() ?

2002-07-08 Thread Mike Makonnen

On Mon, 08 Jul 2002 08:20:42 +0100
David Malone <[EMAIL PROTECTED]> wrote:

> 
> bcopy(&(lim->pl_rlimit[0]), &(copy->pl_rlimit[0]),
> sizeof(lim->pl_rlimit));
> 
> rather than just copying the first limit? It might be better to just
> bcopy the whole struct plimit and make a note that other fields need
> to be reset.
> 

yeah, you're right. I replaced one benign bug with a worse one. Alfred 
emailed me about it earlier, but I wasn't paying attention. Sorry :(


Cheers,
Mike Makonnen

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: benign bug in src/sys/kern/kern_resource.c:limcopy() ?

2002-07-08 Thread David Malone

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);

or maybe:

bcopy(&(lim->pl_rlimit[0]), &(copy->pl_rlimit[0]), sizeof(lim->pl_rlimit));

rather than just copying the first limit? It might be better to just
bcopy the whole struct plimit and make a note that other fields need
to be reset.

David.

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message