From owner-freebsd-current@FreeBSD.ORG  Sat May 17 00:27:57 2003
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP
	id 48F0237B401; Sat, 17 May 2003 00:27:57 -0700 (PDT)
Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id B00DB43F3F; Sat, 17 May 2003 00:27:56 -0700 (PDT)
	(envelope-from truckman@FreeBSD.org)
Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2])
	by gw.catspoiler.org (8.12.9/8.12.9) with ESMTP id h4H7RjM7059853;
	Sat, 17 May 2003 00:27:49 -0700 (PDT)
	(envelope-from truckman@FreeBSD.org)
Message-Id: <200305170727.h4H7RjM7059853@gw.catspoiler.org>
Date: Sat, 17 May 2003 00:27:45 -0700 (PDT)
From: Don Lewis <truckman@FreeBSD.org>
To: tlambert2@mindspring.com
In-Reply-To: <3EC5BFF2.9359D22F@mindspring.com>
MIME-Version: 1.0
Content-Type: TEXT/plain; charset=us-ascii
cc: jeff@FreeBSD.org
cc: current@FreeBSD.org
Subject: Re: CFR: fifo_open()/fifo_close() patch
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
	<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sat, 17 May 2003 07:27:57 -0000

On 16 May, Terry Lambert wrote:
> 
> 
> Don Lewis wrote:
>> 
>> There are a few problems in the fifo_open() and fifo_close()
>> implementations.
>> 
>>         fifo_open() calls VOP_CLOSE() with the vnode locked, whereas
>>         VOP_CLOSE() should be called with the vnode unlocked.
> 
> This is actually pretty bogus.  All VOP's, except those that
> return (locked) vnodes, or dispose (locked) vnodes that are
> managed by the FS itself, should have locked vnodes.  There's
> a nasty race condition that occurs because of the VOP_CLOSE()
> being called without the vnode locked.

It does look like v_writecount is somewhat inconsistently locked. The
comment in <sys/vnode.h> indicates that it should be protected by the
vnode lock, but some of the INVARIANTS, DIAGNOSTIC, and KASSERT code
protects it with the vnode interlock, and vn_close() totally fails to
protect the manipulation of v_writecount.

I'd toss in calls to vn_lock() and VOP_UNLOCK(), but it looks like while
most callers of vn_close() call it with the vnode locked, not all do.
I'm not feeling ambitious enough to track them all down.