Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 24 Sep 2011 04:19:53 -0400
From:      Arnaud Lacombe <lacombar@gmail.com>
To:        zec@FreeBSD.org, Julian Elischer <julian@freebsd.org>
Cc:        freebsd-net@freebsd.org
Subject:   VNET mismanagement in bpf(4) ?
Message-ID:  <CACqU3MWaNxM3-N4qURzZs5NMVeETzXM59qu-24zW%2BEuqfY=2CQ@mail.gmail.com>
next in thread | raw e-mail | index | archive | help 

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

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACqU3MWaNxM3-N4qURzZs5NMVeETzXM59qu-24zW%2BEuqfY=2CQ>