From owner-freebsd-hackers  Tue Jun 15 11: 9:57 1999
Delivered-To: freebsd-hackers@freebsd.org
Received: from gvr.gvr.org (gvr.gvr.org [194.151.74.97])
	by hub.freebsd.org (Postfix) with ESMTP
	id 4B584152CA; Tue, 15 Jun 1999 11:09:49 -0700 (PDT)
	(envelope-from guido@gvr.org)
Received: by gvr.gvr.org (Postfix, from userid 657)
	id C35605B98; Tue, 15 Jun 1999 20:09:47 +0200 (MET DST)
Message-ID: <19990615200947.B3803@gvr.org>
Date: Tue, 15 Jun 1999 20:09:47 +0200
From: Guido van Rooij <guido@gvr.org>
To: Matthew Dillon <dillon@apollo.backplane.com>,
	Matthew Jacob <mjacob@feral.com>
Cc: freebsd-hackers@FreeBSD.ORG, peter@FreeBSD.ORG
Subject: Re: D'oh!
References: <Pine.BSF.4.05.9906151030470.5752-100000@semuta.feral.com> <199906151737.KAA19611@apollo.backplane.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-Mailer: Mutt 0.93.2i
In-Reply-To: <199906151737.KAA19611@apollo.backplane.com>; from Matthew Dillon on Tue, Jun 15, 1999 at 10:37:18AM -0700
Sender: owner-freebsd-hackers@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

On Tue, Jun 15, 1999 at 10:37:18AM -0700, Matthew Dillon wrote:
> 
>     Sounds good to me!  If someone on -hackers has easy access to the OpenBSD
>     source, it would be nice if he could check whether the OpenBSD code
>     has the same problem and notify the OpenBSD folks if it does.

they dont seem to have the nfs_node_hash_lock at all.

Our code in nfs_nget():
loop:
        for (np = nhpp->lh_first; np != 0; np = np->n_hash.le_next) {
                if (mntp != NFSTOV(np)->v_mount || np->n_fhsize != fhsize ||
                    bcmp((caddr_t)fhp, (caddr_t)np->n_fhp, fhsize))
                        continue;
                vp = NFSTOV(np);
                if (vget(vp, 1))
                        goto loop;
                *npp = np;
                return(0); 
        }
        /*
         * Obtain a lock to prevent a race condition if the getnewvnode()
         * or MALLOC() below happens to block.
         */
        if (nfs_node_hash_lock) {
                while (nfs_node_hash_lock) {
                        nfs_node_hash_lock = -1;
                        tsleep(&nfs_node_hash_lock, PVM, "nfsngt", 0);
                }
        nfs_node_hash_lock = 1;

        /*
         * Do the MALLOC before the getnewvnode since doing so afterward
         * might cause a bogus v_data pointer to get dereferenced
         * elsewhere if MALLOC should block.
         */
        MALLOC(np, struct nfsnode *, sizeof *np, M_NFSNODE, M_WAITOK);
               
        error = getnewvnode(VT_NFS, mntp, nfsv2_vnodeop_p, &nvp);

Their code:
loop:
        for (np = nhpp->lh_first; np != 0; np = np->n_hash.le_next) {
                if (mntp != NFSTOV(np)->v_mount || np->n_fhsize != fhsize ||
                    bcmp((caddr_t)fhp, (caddr_t)np->n_fhp, fhsize))
                        continue;
                vp = NFSTOV(np);
                if (vget(vp, LK_EXCLUSIVE, p))
                        goto loop;
                *npp = np;
                return(0);
        }
        error = getnewvnode(VT_NFS, mntp, nfsv2_vnodeop_p, &nvp);
        if (error) {
                *npp = 0;
                return (error);
        }
        vp = nvp;
        MALLOC(np, struct nfsnode *, sizeof *np, M_NFSNODE, M_WAITOK);

I have not checked if they have fixed this otherwise though.

-Guido


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message