Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 5 Aug 1996 15:00:05 -0600 (MDT)
From:      Nate Williams <nate@mt.sri.com>
To:        Gary Jennejohn <Gary.Jennejohn@munich.netsurf.de>
Cc:        Michael Smith <msmith@atrad.adelaide.edu.au>, freebsd-current@freebsd.org
Subject:   Re: question about grinding system (Linux guru attn. reqd) 
Message-ID:  <199608052100.PAA23721@rocky.mt.sri.com>
In-Reply-To: <199608052205.WAA00720@peedub.gj.org>
References:  <199608050444.OAA10882@genesis.atrad.adelaide.edu.au> <199608052205.WAA00720@peedub.gj.org>

next in thread | previous in thread | raw e-mail | index | archive | help
> >Watching the output of vmstat -m, I notice that the numbers for the 
> >'temp' category keep growing, and that there's about 8M allocated to it
> >at this point in time.
> >
> >Following this and the suspicion that the problem is linux-emulation related
> >I went looking for M_TEMP allocations in i386/linux, and I'd like someone
> >who's better qualified than I to look over the exit conditions in
> >linux_emul_find() in linux_util.c because I'm fairly sure that it's
> >possible for this function to return without freeing its buffer,
> >and this would lead to the sort of leak that I'm seeing.

> sure looks like the places where a "goto done" happens fail to free the
> buffer.

I concur.  And, if you look at the sources from the NetBSD OS emulation
code in /sys/compat/common/compat_util.c, it's even more apparent.  The
FreeBSD code now reads:

        vrele(nd.ni_vp);
        if (!cflag)
                vrele(ndroot.ni_vp);
        return error;

Where the code in NetBSD reads:

        vrele(nd.ni_vp);
        if (!cflag)
                vrele(ndroot.ni_vp);
        return error;

bad3:
        vrele(ndroot.ni_vp);
bad2:
        vrele(nd.ni_vp);
bad:
        free(buf, M_TEMP);
        return error;

Note that we must release the buffer in an error condition, and that
things are handled differently if everything succeeds vs. a failure.  I
chose to solve it the same way the NetBSD sources did, just to be
consistant with them, so our sources are almost exactly the same again.

The context diff is appended below, which is at least a partial solution
to Michael's problem, if not the entire thing.


Nate
------
Index: linux_util.c
===================================================================
RCS file: /home/ncvs/src/sys/i386/linux/linux_util.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -c -r1.1 -r1.2
*** linux_util.c	1996/03/02 19:38:02	1.1
--- linux_util.c	1996/08/05 20:52:30	1.2
***************
*** 27,33 ****
   * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
   *
   *	from: svr4_util.c,v 1.5 1995/01/22 23:44:50 christos Exp
!  *	$Id: linux_util.c,v 1.1 1996/03/02 19:38:02 peter Exp $
   */
  
  #include <sys/param.h>
--- 27,33 ----
   * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
   *
   *	from: svr4_util.c,v 1.5 1995/01/22 23:44:50 christos Exp
!  *	$Id: linux_util.c,v 1.2 1996/08/05 20:52:30 nate Exp $
   */
  
  #include <sys/param.h>
***************
*** 146,163 ****
  		}
  
  		if ((error = VOP_GETATTR(nd.ni_vp, &vat, p->p_ucred, p)) != 0) {
! 			goto done;
  		}
  
  		if ((error = VOP_GETATTR(ndroot.ni_vp, &vatroot, p->p_ucred, p))
  		    != 0) {
! 			goto done;
  		}
  
  		if (vat.va_fsid == vatroot.va_fsid &&
  		    vat.va_fileid == vatroot.va_fileid) {
  			error = ENOENT;
! 			goto done;
  		}
  
  	}
--- 146,163 ----
  		}
  
  		if ((error = VOP_GETATTR(nd.ni_vp, &vat, p->p_ucred, p)) != 0) {
! 			goto bad;
  		}
  
  		if ((error = VOP_GETATTR(ndroot.ni_vp, &vatroot, p->p_ucred, p))
  		    != 0) {
! 			goto bad;
  		}
  
  		if (vat.va_fsid == vatroot.va_fsid &&
  		    vat.va_fileid == vatroot.va_fileid) {
  			error = ENOENT;
! 			goto bad;
  		}
  
  	}
***************
*** 170,179 ****
  		free(buf, M_TEMP);
  	}
  
- 
- done:
  	vrele(nd.ni_vp);
  	if (!cflag)
  		vrele(ndroot.ni_vp);
  	return error;
  }
--- 170,183 ----
  		free(buf, M_TEMP);
  	}
  
  	vrele(nd.ni_vp);
  	if (!cflag)
  		vrele(ndroot.ni_vp);
+ 	return error;
+ 
+ bad:
+ 	vrele(ndroot.ni_vp);
+ 	vrele(nd.ni_vp);
+ 	free(buf, M_TEMP);
  	return error;
  }



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