From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: bpf use after free
To: David Gwynne <david@gwynne.id.au>
Cc: Vitaliy Makkoveev <mvs@openbsd.org>, tech@openbsd.org
Date: Fri, 13 Jun 2025 13:52:41 +0200

On Fri, Jun 13, 2025 at 01:53:13PM +1000, David Gwynne wrote:
> > > > if_input_local() checks ifp->if_bpf and bpfdetach() sets ifp->if_bpf
> > > > to NULL.  As if_input_local() runs with shared net lock, bpfdetach()
> > > > should be locked exclusively.  As bpf_mtap() may be called by
> > > > driver's RX interrupt, we also need splnet().
> 
> raising the spl only applies to the current cpu, it does not block
> interrupts on any other cpu, so splnet is not enough to block rx
> interrupts.

Yes, spl is for current CPU.  It works together with kernel lock
to prevent interrupts on other CPU.  That is what legacy drivers
use.

Anyway, your fix is much better than mine.

OK bluhm@

> Index: if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> diff -u -p -r1.734 if.c
> --- if.c	9 May 2025 03:12:36 -0000	1.734
> +++ if.c	13 Jun 2025 03:50:28 -0000
> @@ -1202,8 +1202,10 @@ if_detach(struct ifnet *ifp)
>  	ifp->if_qstart = if_detached_qstart;
>  
>  	/* Wait until the start routines finished. */
> -	ifq_barrier(&ifp->if_snd);
> -	ifq_clr_oactive(&ifp->if_snd);
> +	for (i = 0; i < ifp->if_nifqs; i++) {
> +		ifq_barrier(ifp->if_ifqs[i]);
> +		ifq_clr_oactive(ifp->if_ifqs[i]);
> +	}
>  
>  #if NBPFILTER > 0
>  	bpfdetach(ifp);