Re: git: c81dd8e5fe72 - main - bpf: Add IfAPI analogue for bpf_peers_present()

> On Oct 14, 2023, at 2:13 AM, Zhenlei Huang <zlei@FreeBSD.org> wrote:
> 
> 
> 
>> On Oct 14, 2023, at 1:22 AM, Justin Hibbits <jhibbits@freebsd.org> wrote:
>> 
>> The branch main has been updated by jhibbits:
>> 
>> URL: https://cgit.FreeBSD.org/src/commit/?id=c81dd8e5fe72d0c7ec055c8621bb2da3a3627abf
>> 
>> commit c81dd8e5fe72d0c7ec055c8621bb2da3a3627abf
>> Author:     Justin Hibbits <jhibbits@FreeBSD.org>
>> AuthorDate: 2023-10-04 20:56:52 +0000
>> Commit:     Justin Hibbits <jhibbits@FreeBSD.org>
>> CommitDate: 2023-10-13 17:12:44 +0000
>> 
>>   bpf: Add IfAPI analogue for bpf_peers_present()
>> 
>>   An interface's bpf could feasibly not exist, in which case
>>   bpf_peers_present() would panic from a NULL pointer dereference.  Solve
>>   this by adding a new IfAPI that includes a NULL check.  Since this API
> 
> The initial version DOES have NULL check. But that is redundant and removed
> from the committed version.

See https://reviews.freebsd.org/D42082 <https://reviews.freebsd.org/D42082> .

> 
>>   is used in only a handful of locations, it reduces the the NULL check
>>   scope over inserting the check into bpf_peers_present().
>> 
>>   Sponsored by:   Juniper Networks, Inc.
>>   MFC after:      1 week
>> ---
>> sys/dev/firewire/if_fwip.c    |  4 ++--
>> sys/dev/hyperv/netvsc/if_hn.c |  4 ++--
>> sys/dev/my/if_my.c            |  2 +-
>> sys/dev/usb/usb_pf.c          |  4 +---
>> sys/net/bpf.c                 | 14 ++++++++++++++
>> sys/net/bpf.h                 |  1 +
>> 6 files changed, 21 insertions(+), 8 deletions(-)
>> 
>> diff --git a/sys/dev/firewire/if_fwip.c b/sys/dev/firewire/if_fwip.c
>> index 5237c555d999..b698db6c9620 100644
>> --- a/sys/dev/firewire/if_fwip.c
>> +++ b/sys/dev/firewire/if_fwip.c
>> @@ -780,7 +780,7 @@ fwip_stream_input(struct fw_xferq *xferq)
>> 		 * Record the sender ID for possible BPF usage.
>> 		 */
>> 		src = ntohl(p[1]) >> 16;
>> -		if (bpf_peers_present(if_getbpf(ifp))) {
>> +		if (bpf_peers_present_if(ifp)) {
>> 			mtag = m_tag_alloc(MTAG_FIREWIRE,
>> 			    MTAG_FIREWIRE_SENDER_EUID,
>> 			    2*sizeof(uint32_t), M_NOWAIT);
>> @@ -880,7 +880,7 @@ fwip_unicast_input(struct fw_xfer *xfer)
>> 		goto done;
>> 	}
>> 
>> -	if (bpf_peers_present(if_getbpf(ifp))) {
>> +	if (bpf_peers_present_if(ifp)) {
>> 		/*
>> 		 * Record the sender ID for possible BPF usage.
>> 		 */
>> diff --git a/sys/dev/hyperv/netvsc/if_hn.c b/sys/dev/hyperv/netvsc/if_hn.c
>> index 7d8e1914163e..f6f885873a79 100644
>> --- a/sys/dev/hyperv/netvsc/if_hn.c
>> +++ b/sys/dev/hyperv/netvsc/if_hn.c
>> @@ -3262,7 +3262,7 @@ hn_txpkt(if_t ifp, struct hn_tx_ring *txr, struct hn_txdesc *txd)
>> 	int error, send_failed = 0, has_bpf;
>> 
>> again:
>> -	has_bpf = bpf_peers_present(if_getbpf(ifp));
>> +	has_bpf = bpf_peers_present_if(ifp);
>> 	if (has_bpf) {
>> 		/*
>> 		 * Make sure that this txd and any aggregated txds are not
>> @@ -5972,7 +5972,7 @@ hn_transmit(if_t ifp, struct mbuf *m)
>> 			omcast = (m->m_flags & M_MCAST) != 0;
>> 
>> 			if (sc->hn_xvf_flags & HN_XVFFLAG_ACCBPF) {
>> -				if (bpf_peers_present(if_getbpf(ifp))) {
>> +				if (bpf_peers_present_if(ifp)) {
>> 					m_bpf = m_copypacket(m, M_NOWAIT);
>> 					if (m_bpf == NULL) {
>> 						/*
>> diff --git a/sys/dev/my/if_my.c b/sys/dev/my/if_my.c
>> index 2bf4573d337b..631c38df9dca 100644
>> --- a/sys/dev/my/if_my.c
>> +++ b/sys/dev/my/if_my.c
>> @@ -1152,7 +1152,7 @@ my_rxeof(struct my_softc * sc)
>> 		 * broadcast packet, multicast packet, matches our ethernet
>> 		 * address or the interface is in promiscuous mode.
>> 		 */
>> -		if (bpf_peers_present(if_getbpf(ifp))) {
>> +		if (bpf_peers_present_if(ifp)) {
>> 			bpf_mtap_if(ifp, m);
>> 			if (if_getflags(ifp) & IFF_PROMISC &&
>> 			    (bcmp(eh->ether_dhost, if_getlladdr(sc->my_ifp),
>> diff --git a/sys/dev/usb/usb_pf.c b/sys/dev/usb/usb_pf.c
>> index 43e819684857..4da59419a7c6 100644
>> --- a/sys/dev/usb/usb_pf.c
>> +++ b/sys/dev/usb/usb_pf.c
>> @@ -408,9 +408,7 @@ usbpf_xfertap(struct usb_xfer *xfer, int type)
>> 	bus = xfer->xroot->bus;
>> 
>> 	/* sanity checks */
>> -	if (bus->ifp == NULL || if_getbpf(bus->ifp) == NULL)
>> -		return;
>> -	if (!bpf_peers_present(if_getbpf(bus->ifp)))
>> +	if (bus->ifp == NULL || !bpf_peers_present_if(bus->ifp))
>> 		return;
>> 
>> 	totlen = usbpf_xfer_precompute_size(xfer, type);
>> diff --git a/sys/net/bpf.c b/sys/net/bpf.c
>> index 8ca6e941e646..96420b709911 100644
>> --- a/sys/net/bpf.c
>> +++ b/sys/net/bpf.c
>> @@ -2879,6 +2879,14 @@ bpfdetach(struct ifnet *ifp)
>> 	BPF_UNLOCK();
>> }
>> 
>> +bool
>> +bpf_peers_present_if(struct ifnet *ifp)
>> +{
>> +	struct bpf_if *bp = ifp->if_bpf;
>> +
>> +	return (bpf_peers_present(bp) > 0);
>> +}
>> +
>> /*
>> * Get a list of available data link type of the interface.
>> */
>> @@ -3162,6 +3170,12 @@ bpfdetach(struct ifnet *ifp)
>> {
>> }
>> 
>> +bool
>> +bpf_peers_present_if(struct ifnet *ifp)
>> +{
>> +	return (false);
>> +}
>> +
>> u_int
>> bpf_filter(const struct bpf_insn *pc, u_char *p, u_int wirelen, u_int buflen)
>> {
>> diff --git a/sys/net/bpf.h b/sys/net/bpf.h
>> index 924dea5fc9f4..31968445aac1 100644
>> --- a/sys/net/bpf.h
>> +++ b/sys/net/bpf.h
>> @@ -428,6 +428,7 @@ void	 bpf_mtap2_if(struct ifnet *, void *, u_int, struct mbuf *);
>> void	 bpfattach(struct ifnet *, u_int, u_int);
>> void	 bpfattach2(struct ifnet *, u_int, u_int, struct bpf_if **);
>> void	 bpfdetach(struct ifnet *);
>> +bool	 bpf_peers_present_if(struct ifnet *);
>> #ifdef VIMAGE
>> int	 bpf_get_bp_params(struct bpf_if *, u_int *, u_int *);
>> #endif

Best regards,
Zhenlei