From owner-freebsd-net@FreeBSD.ORG  Sat Sep 24 08:19:55 2011
Return-Path: <owner-freebsd-net@FreeBSD.ORG>
Delivered-To: freebsd-net@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 1BFE6106566C
	for <freebsd-net@freebsd.org>; Sat, 24 Sep 2011 08:19:55 +0000 (UTC)
	(envelope-from lacombar@gmail.com)
Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com
	[74.125.82.182])
	by mx1.freebsd.org (Postfix) with ESMTP id AE93B8FC14
	for <freebsd-net@freebsd.org>; Sat, 24 Sep 2011 08:19:53 +0000 (UTC)
Received: by wyj26 with SMTP id 26so2083419wyj.13
	for <multiple recipients>; Sat, 24 Sep 2011 01:19:53 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;
	h=mime-version:date:message-id:subject:from:to:cc:content-type;
	bh=UaygTDl3jBzLBUuu66lnLqf8EzMXmToJJygxGCZiWRM=;
	b=CI0a9zltBRIut9T43jNoW3mBoQNRsUUzylrHvAX2+8J3Jp+rMA5Gq/53+VTdRWlovs
	C2cyVuqjUHbiHY0w6O+UIABrI/hPWpghRiIKShYhW0lpGumDThjP+R1Zu74PEltuHBTJ
	ARH28ih9xCraYvbnOfqsvx47KVTW1F/LKZtAo=
MIME-Version: 1.0
Received: by 10.227.175.77 with SMTP id w13mr4456642wbz.53.1316852393201; Sat,
	24 Sep 2011 01:19:53 -0700 (PDT)
Received: by 10.180.95.169 with HTTP; Sat, 24 Sep 2011 01:19:53 -0700 (PDT)
Date: Sat, 24 Sep 2011 04:19:53 -0400
Message-ID: <CACqU3MWaNxM3-N4qURzZs5NMVeETzXM59qu-24zW+EuqfY=2CQ@mail.gmail.com>
From: Arnaud Lacombe <lacombar@gmail.com>
To: zec@FreeBSD.org, Julian Elischer <julian@freebsd.org>
Content-Type: text/plain; charset=ISO-8859-1
Cc: freebsd-net@freebsd.org
Subject: VNET mismanagement in bpf(4) ?
X-BeenThere: freebsd-net@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Networking and TCP/IP with FreeBSD <freebsd-net.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
	<mailto:freebsd-net-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net>
List-Post: <mailto:freebsd-net@freebsd.org>
List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
	<mailto:freebsd-net-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sat, 24 Sep 2011 08:19:55 -0000

Hi,

It seems to me there is a VNET mismanagement in `net/bpf.c'; we have:

static void
bpf_detachd(struct bpf_d *d)
{
[...]
    if (d->bd_promisc) {
        d->bd_promisc = 0;
        CURVNET_SET(ifp->if_vnet);
        error = ifpromisc(ifp, 0);
        CURVNET_RESTORE();
[...]
}

which is called by either bpfdetach() or bpf_setdlt(). Here is the
relevant code block of the latter:

static int
bpf_setdlt(struct bpf_d *d, u_int dlt)
{
[...]
    if (bp != NULL) {
        opromisc = d->bd_promisc;
        bpf_detachd(d);
        bpf_attachd(d, bp);
        BPFD_LOCK(d);
        reset_d(d);
        BPFD_UNLOCK(d);
        if (opromisc) {
            error = ifpromisc(bp->bif_ifp, 1);
            if (error)
                if_printf(bp->bif_ifp,
                    "bpf_setdlt: ifpromisc failed (%d)\n",
                    error);
            else
                d->bd_promisc = 1;
        }
[...]
}

I would guess that there is no need to `CURVNET_SET(ifp->if_vnet);'
before calling `ifpromisc()' because it has already been done in
`bpfioctl()':

static  int
bpfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags,
    struct thread *td)
{
[...]
    CURVNET_SET(TD_TO_VNET(td));
[...]
    /*
     * Set data link type.
     */
    case BIOCSDLT:
        if (d->bd_bif == NULL)
            error = EINVAL;
        else
            error = bpf_setdlt(d, *(u_int *)addr);
        break;
[...]
    CURVNET_RESTORE();
    return (error);
}

However, the following call path:

bpfioctl() ->
  bpf_setdlt() ->
    bpf_detachd() ->
    [ret]
  bpf_setdlt() ->
  [ret]
bpfioctl()
[ret]

will end up in the following sequence of VNET setting/restore:

bpfioctl():CURVNET_SET(TD_TO_VNET(td)) ->
  bpf_detachd():CURVNET_SET(ifp->if_vnet) ->
  bpf_detachd():CURVNET_RESTORE() ->
bpfioctl():CURVNET_RESTORE()

leading to the lost of the original VNET, from bpfioctl().

Am I missing something ?

Thanks,
 - Arnaud