* Ralf Horstmann <ralf-li...@ackstorm.de> [2020-11-05 11:02]:
> * Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com> [2020-11-05 09:24]:
> > Hello Ralf,
> >
> > On Thu, Nov 05, 2020 at 07:59:18AM +0100, Ralf Horstmann wrote:
> > > Hi Sashan,
> > >
> > > * Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com> [2020-11-05 00:25]:
> > > > Hello Ralf,
> > > >
> > > > I think we should rather give a try diff below. I would rather add
> > > > a return after m_freem(). Will diff below work for you too?
> > >
> > > Without understanding the full background I went with ignoring the return
> > > value
> >
> > I'm sorry I have not provide better explanation in my first email.
> > this is the code after applying diff I've suggested earlier:
> >
> > 2277 void
> > 2278 carp_transmit(struct carp_softc *sc, struct ifnet *ifp0, struct mbuf
> > *m)
> > 2279 {
> > 2280 struct ifnet *ifp = &sc->sc_if;
> > 2281
> > 2282 #if NBPFILTER > 0
> > 2283 {
> > 2284 caddr_t if_bpf = ifp->if_bpf;
> > 2285 if (if_bpf) {
> > 2286 if (bpf_mtap_ether(if_bpf, m,
> > BPF_DIRECTION_OUT)) {
> > 2287 m_freem(m);
> > 2288 return;
> > 2289 }
> > 2290 }
> > 2291 }
> > 2292 #endif /* NBPFILTER > 0 */
> > 2293
> > 2294 if (!ISSET(ifp0->if_flags, IFF_RUNNING)) {
> > 2295 counters_inc(ifp->if_counters, ifc_oerrors);
> > 2296 m_freem(m);
> > 2297 return;
> > 2298 }
> >
> >
> > the proposed change adds a 'return;' to line 2288, which is also
> > consistent
> > with code, we can see at following line 2297.
>
> Yes, that part is clear, there either needs to be a return or the bpf return
> code needs to be ignored and the m_freem removed.
>
> I'm just wondering why other callers of bpf_mtap_ether with BPF_DIRECTION_OUT
> across the tree are ignoring the return code, and why we need to observe it
> in carp. E.g. if_vlan.c:
>
> void
> vlan_transmit(struct vlan_softc *sc, struct ifnet *ifp0, struct mbuf *m)
> {
> struct ifnet *ifp = &sc->sc_if;
> int txprio = sc->sc_txprio;
> uint8_t prio;
>
> #if NBPFILTER > 0
> if (ifp->if_bpf)
> bpf_mtap_ether(ifp->if_bpf, m, BPF_DIRECTION_OUT);
> #endif /* NBPFILTER > 0 */
>
> Since I somehow managed to trigger the case that the drop action was returned
> from bpf, I'm suspecting that with your patch packets would be dropped that
> should rather be sent out. Maybe the dhcpd installed BPF code needs to be
> adjusted as well then.
I added some debug output to dump the packets that have been processed by BPF
in carp_transmit(). Especially the packets that result in BPF "drop". This
made preconditions for the kernel panic more clear:
- OpenBSD 6.8 or newer with ip_carp.c 1.344 or newer
- dhcpd needs to run on a carp interface
- DHCP requests need to be forwarded to dhcpd by a DHCP gateway. In my case
the DHCP gateway is a Cisco router with "ip helper-address".
- when dhcpd responds with an DHCPOFFER to the DHCP gateway , the
bpf_mtap_ether() result was 1 (== drop), which triggered the use-after-free
This means that the BPF wfilter installed by dhcpd may be too strict and does
not pass DHCP response packets sent to a DHCP gateway. The only differences to
normal DHCP response packets are that both source and destination port are 67
and the giaddr field is populated. Why exactly the write filter wants to drop
these packets is unclear, the dhcpd BPF program looks like it should accept
them.
It's also interesting that for the packets where bpf_mtap_ether() returns 1
(== drop), the return value of the dhcpd wfilter BPF is (u_int)-1, which
according to the comment in dhcpd/bpf.c is supposed to mean "pass".
Both diffs do prevent the kernel panic. But the diff that drops packets
depending on bpf_mtap_ether() result may cause problems when using DHCP
gateways.
Regards,
Ralf
