From owner-freebsd-security@FreeBSD.ORG  Sat Oct 21 08:18:23 2006
Return-Path: <owner-freebsd-security@FreeBSD.ORG>
X-Original-To: freebsd-security@freebsd.org
Delivered-To: freebsd-security@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id AC00016A417
	for <freebsd-security@freebsd.org>;
	Sat, 21 Oct 2006 08:18:23 +0000 (UTC) (envelope-from bde@zeta.org.au)
Received: from mailout1.pacific.net.au (mailout1-3.pacific.net.au [61.8.2.210])
	by mx1.FreeBSD.org (Postfix) with ESMTP id EB37843D6E
	for <freebsd-security@freebsd.org>;
	Sat, 21 Oct 2006 08:18:17 +0000 (GMT) (envelope-from bde@zeta.org.au)
Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au
	[61.8.2.163])
	by mailout1.pacific.net.au (Postfix) with ESMTP id 1800861FFB7;
	Sat, 21 Oct 2006 18:18:16 +1000 (EST)
Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246])
	by mailproxy2.pacific.net.au (Postfix) with ESMTP id 269742740C;
	Sat, 21 Oct 2006 18:18:15 +1000 (EST)
Date: Sat, 21 Oct 2006 18:18:14 +1000 (EST)
From: Bruce Evans <bde@zeta.org.au>
X-X-Sender: bde@delplex.bde.org
To: Padma Bhooma <pbhooma@panasas.com>
In-Reply-To: <453969CC.6060809@panasas.com>
Message-ID: <20061021175029.J84514@delplex.bde.org>
References: <453969CC.6060809@panasas.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Cc: freebsd-security@freebsd.org
Subject: Re: [patch] Memory leak from namei_zone in an error path in
	nfsrv_rename
X-BeenThere: freebsd-security@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: "Security issues \[members-only posting\]"
	<freebsd-security.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-security>, 
	<mailto:freebsd-security-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-security>
List-Post: <mailto:freebsd-security@freebsd.org>
List-Help: <mailto:freebsd-security-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-security>, 
	<mailto:freebsd-security-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sat, 21 Oct 2006 08:18:23 -0000

> --- nfs_serv.c  2005-11-25 06:32:38.000000000 -0800
> +++ /tmp/nfs_serv.c     2006-09-22 14:41:39.000000000 -0700
> @@ -2514,26 +2514,26 @@
>             /*
>              * The VOP_RENAME function releases all vnode references &
>              * locks prior to returning so we need to clear the pointers
>              * to bypass cleanup code later on.
>              */
>             error = VOP_RENAME(fromnd.ni_dvp, fromnd.ni_vp, &fromnd.ni_cnd,
>                                tond.ni_dvp, tond.ni_vp, &tond.ni_cnd);
>             fromnd.ni_dvp = NULL;
>             fromnd.ni_vp = NULL;
>             tond.ni_dvp = NULL;
>             tond.ni_vp = NULL;
>             if (error) {
> -                       fromnd.ni_cnd.cn_flags &= ~HASBUF;
> -                       tond.ni_cnd.cn_flags &= ~HASBUF;
> +                       NDFREE(&fromnd, NDF_ONLY_PNBUF);
> +                       NDFREE(&tond, NDF_ONLY_PNBUF);
>             }
>     } else {
>             if (error == -1)
>                     error = 0;
>     }
>     /* fall through */
>
>
> I will be happy to answer any questions wrt this. Please provide me some 
> feedback on this fix.

Seems about right, but why does it clear HASBUF at all?  Rev.1.79 added
a lot of similar clearings of HASBUF, but rev.1.91 converted all instances
of HASBUF except the above 2 above and 1 in a comment into NDFREE().  I
think associated changes also moved the VOP_ABORTUP() calls into the vfs
layer and out of the HASBUF conditionals.  It looks like the leak was
in rev.1.90 and rev.1.91 tried too hard not to change the logic by leaving
the 2 buggy HASBUF clearings untouched.  The comment about HASBUF is now
bogus -- _we_ now mostly don't use HASBUF to track the clearing of the name
buffer -- now namei iinternals do that and we only (?) use it to implement
the leak :-).

Bruce