Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Jun 2002 01:15:38 -0700
From:      Jon Mini <baka@elvis.mu.org>
To:        Peter Jeremy <peter.jeremy@alcatel.com.au>
Cc:        alfred@freebsd.org, current@freebsd.org
Subject:   Re: (forw) proc-args (M_PARGS) leakage
Message-ID:  <20020621081538.GC82982@elvis.mu.org>
In-Reply-To: <20020617044612.GD67925@elvis.mu.org>
References:  <20020617044612.GD67925@elvis.mu.org>

next in thread | previous in thread | raw e-mail | index | archive | help
> From: Peter Jeremy <peter.jeremy@alcatel.com.au>
> To: freebsd-current@freebsd.org
> Subject: proc-args (M_PARGS) leakage
> Date: Mon, 17 Jun 2002 13:15:03 +1000
> Message-id: <20020617131502.O680@gsmx07.alcatel.com.au>
> User-Agent: Mutt/1.2.5.1i
> Sender: owner-freebsd-current@FreeBSD.ORG
> 
> This is -CURRENT from 7th May so it's possible the bug has been
> fixed, though there's nothing obvious in either the CVS commit
> logs or by diffing the relevant files.
> 
> Having noticed that my system is paging far more than I would have
> expected, I went looking and found that the 'proc-args' pool was
> far larger than I expected.  And is growing over time:
> 
> gsmx07# vmstat -m|grep proc-args
>     proc-args701802 70634K  70634K  1589264  16,32,64,128,256
> [about 10 minutes delay]
> gsmx07# vmstat -m|grep proc-args;vmstat -m|grep proc-args 
>     proc-args702048 70652K  70652K  1589557  16,32,64,128,256
>     proc-args702047 70652K  70652K  1589558  16,32,64,128,256
> gsmx07# 
> 
> Unfortunately, M_PARGS is not the easiest pool to track allocations
> and de-allocations.  Having gone through the references to pargs_*()
> and p_args, I can't see any obvious cause of this.

This is definately failure to drop a reference.

> Whilst I'm fairly certain it's not my problem, sysctl_kern_proc_args()
> (1.136) looks dubious:

  [ ... ]

> (And later code shows pa dead at this point).  I don't follow this.
> pargs_drop(pa) deletes a single reference count - which matches the
> line "p->p_args = NULL;" - but I don't see anything to match the
> pargs_hold(pa) above.

You are correct, One reference too few is dropped when a proc's pargs
is changed. This would mean that every call to setproctitle(3) would
cause a leak.

Please let me know if this fixes your problem:

Index: kern_proc.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_proc.c,v
retrieving revision 1.136
diff -u -r1.136 kern_proc.c
--- kern_proc.c	7 Jun 2002 05:41:27 -0000	1.136
+++ kern_proc.c	21 Jun 2002 08:04:17 -0000
@@ -1024,10 +1024,9 @@
 	if (req->oldptr && pa != NULL) {
 		error = SYSCTL_OUT(req, pa->ar_args, pa->ar_length);
 	}
-	if (req->newptr == NULL) {
-		pargs_drop(pa);
+	pargs_drop(pa);
+	if (req->newptr == NULL)
 		return (error);
-	}
 
 	PROC_LOCK(p);
 	pa = p->p_args;

> Additionally, whilst I'm certain it's not my problem,
> fill_kinfo_proc() copys a reference to pargs, but doesn't increment
> the reference counter (using pargs_hold()).

You are correct; this is intentional. The kinfo stuff is terribly broken
currently. Many fields, pargs being one of them, are broken. A kinfo
struct is passed to userland, so it can't hold any references to kernel
objects. Really, kinfo should contain a copy of the string, not a pargs
pointer.

-- 
Jonathan Mini <mini@freebsd.org>
http://www.freebsd.org/

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?20020621081538.GC82982>