From owner-freebsd-bugs  Wed Jul 19 06:47:29 1995
Return-Path: bugs-owner
Received: (from majordom@localhost)
          by freefall.cdrom.com (8.6.11/8.6.6) id GAA24294
          for bugs-outgoing; Wed, 19 Jul 1995 06:47:29 -0700
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.34])
          by freefall.cdrom.com (8.6.11/8.6.6) with ESMTP id GAA24284
          for <bugs@freebsd.org>; Wed, 19 Jul 1995 06:47:15 -0700
Received: (from bde@localhost) by godzilla.zeta.org.au (8.6.9/8.6.9) id XAA22543; Wed, 19 Jul 1995 23:44:44 +1000
Date: Wed, 19 Jul 1995 23:44:44 +1000
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199507191344.XAA22543@godzilla.zeta.org.au>
To: bugs@freebsd.org, dillon@blob.best.net
Subject: Re: probable race condition in ufs/ffs/ffs_vfsops.c:ffs_vget()
Sender: bugs-owner@freebsd.org
Precedence: bulk

>	panic: ffs_valloc: dup alloc

>    I believe I have found the problem... a race condition in ffs_vget().  
>    Here's a synopsis:

>    (1) lookup (dev,ino) in hash table, return on success

>    (2) allocate new vnode and new inode structure MALLOC(..., M_WAITOK) 
>	for the inode

>    (3) enter new inode into hash table.

>    The problem is that MALLOC() can block.  If it does, you can potentially 
>    have TWO processes attempt to lookup an uncached inode simultaniously 
>    in a low memory situation.  The MALLOC() blocks until memory is available, 

getnewvnode() can block in the same way :-(.  This problem makes using the
M_WAITOK flag difficult.  It can only be used in suroutines if all callers
are known to not care if the subroutine blocks.

I wonder how common it is for malloc() to block?  Several drivers complain
if malloc(..., M_NOWAIT) fails and a few drivers don't check if
malloc(..., M_NOWAIT) fails, but there haven't been many bug reports about
this.

>    The solution, as far as I can tell, is to check the hash table after 
>    MALLOC returns as well as before to determine if another process beat 
>    us to it.  I put the following code just before the ufs_ihashins().  I 
>    do NOT know whether this code fixes the problem yet or even if the 
>    code is valid in terms of freeing the right stuff before returning... 
>    (I'll tell you in a few days re: the crashes... I'll either get 
>    more panics or I will not).

>#if 1
>        if ((*vpp = ufs_ihashget(dev, ino)) != NULL) {
>                vp->v_data = NULL;

I think it clobbers the in-use v_data here.  I think you should lock
the vnode before possibly blocking.

Bruce