From owner-freebsd-bugs@FreeBSD.ORG  Tue Mar  9 00:36:50 2004
Return-Path: <owner-freebsd-bugs@FreeBSD.ORG>
Delivered-To: freebsd-bugs@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id 3BAC216A4CE
	for <freebsd-bugs@freebsd.org>; Tue,  9 Mar 2004 00:36:50 -0800 (PST)
Received: from ice.42.org (ice.42.org [194.77.3.162])
	by mx1.FreeBSD.org (Postfix) with ESMTP id CF8AB43D54
	for <freebsd-bugs@freebsd.org>; Tue,  9 Mar 2004 00:36:49 -0800 (PST)
	(envelope-from sec@42.org)
Received: by ice.42.org (Postfix, from userid 1000)
	id 0E2E11C918; Tue,  9 Mar 2004 09:36:48 +0100 (CET)
Date: Tue, 9 Mar 2004 09:36:47 +0100
From: Stefan `Sec` Zehl <sec@42.org>
To: Brooks Davis <brooks@one-eyed-alien.net>
Message-ID: <20040309083647.GA65980@ice.42.org>
X-Current-Backlog: 1272 messages
References: <200403090050.i290oEuG015002@freefall.freebsd.org>
	<20040309012438.GA20988@Odin.AC.HMC.Edu>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20040309012438.GA20988@Odin.AC.HMC.Edu>
User-Agent: Mutt/1.4.1i
I-love-doing-this: really
X-Modeline: vim:set ts=8 sw=4 smarttab tw=72 si noic notitle:
Accept-Languages: de, en
X-URL: http://sec.42.org/
cc: freebsd-bugs@freebsd.org
Subject: Re: kern/63772: tap device / exclusive open problem
X-BeenThere: freebsd-bugs@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: Bug reports <freebsd-bugs.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>,
	<mailto:freebsd-bugs-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-bugs>
List-Post: <mailto:freebsd-bugs@freebsd.org>
List-Help: <mailto:freebsd-bugs-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>,
	<mailto:freebsd-bugs-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 09 Mar 2004 08:36:50 -0000

On Mon, Mar 08, 2004 at 17:24 -0800, Brooks Davis wrote:
> The problem is very simple.  Since !(tp->tap_flags & TAP_OPEN) should
> never happen (and results in a panic in the KASSERT if INVARIANTS is
> enabled) your "fix" effectivly removes the entire rest of this
> function.  It may be that the rest of that code is junk, but if that's
> the case, it should be removed not bypassed by a bogus conditional.

Aww. I see. I feel so stupid. Somehow I didn't realize that the
condition inside the KASSERT was the exact opposite of my added
condition. Thanks for pointing out the obvious.

However, you made a mistake, too :)

> With this patch and INVARIANTS, you end up with:
> 
> function()
> {
> 	...
> 	if (!A)
> 		panic;
> 	if (A)
> 		return (EBUSY)
> }

No. Kassert panics if the argument is false, not if it is true.

|     KASSERT(!(tp->tap_flags & TAP_OPEN), 
| 	    ("%s flags is out of sync", tp->tap_if.if_xname));
| 
|     if (tp->tap_flags & TAP_OPEN)
| 	    return (EBUSY);

and from sys/systm.h:

| #define KASSERT(exp,msg) do { \
|         if (__predict_false(!(exp))) \
|                 panic msg; \
| } while (0)

Lucky for me, i didn't have INVARIANTS defined :)

If I'm correct, a kernel with INVARIANTS and if_tap loaded can be
panic()ed by a simple "cat /dev/tun0 & cat /dev/tun0".

After my embarrassing dsplay of stupidity, i will however test this
tonight on a test box, and report back on my findings.

CU,
    Sec
-- 
The problem with troubleshooting is that trouble shoots back.