[patch] Memory leak from namei_zone in an error path in nfsrv_rename

Bruce Evans bde at zeta.org.au
Sat Oct 21 01:18:23 PDT 2006


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


More information about the freebsd-security mailing list