From: YASUOKA Masahiko Subject: Re: diff: setting M_BCAST flag To: alexander.bluhm@gmx.net Cc: tech@openbsd.org Date: Wed, 12 Mar 2025 09:44:53 +0900 On Tue, 11 Mar 2025 14:48:28 +0100 Alexander Bluhm wrote: > On Tue, Mar 11, 2025 at 05:42:30PM +0900, YASUOKA Masahiko wrote: >> On Mon, 10 Mar 2025 19:28:25 +0100 >> Alexander Bluhm wrote: >> > On Sat, Mar 08, 2025 at 12:54:35PM +0900, YASUOKA Masahiko wrote: >> >> Hello, >> >> >> >> Our ip_input() doesn't set M_BCAST flag for 0.0.0.0 or 255.255.255.255 >> >> if it's not set by the lower layer. The flag must be set since it is >> >> used by the upper layers. (eg. TCP use the flag to drop the packet.) >> >> >> >> The modification on in_ouraddr() is to set M_BCAST flag consistently >> >> if the packet is for directed broadcast. Previous the flag isn't set >> >> if the packet is diverted by pf(4). >> >> >> >> (eg. on the host that runs a transparent proxy for 80/tcp, when it >> >> receives a packet for directed broadcast but its destination mac >> >> address is not ff:ff:ff:ff:ff:ff (eg. wrong netmask), TCP wil not drop >> >> it and the connection will be established.) >> >> >> >> ok? comments? >> > >> > M_BCAST looks like a layer 2 flag to me. I we want to prevent layer >> > 3 broadcast addresses in TCP, tcp_input() should care about it. >> >> Actually, sys/sys/mbuf.h says the flags is for link-level >> >> #define M_BCAST 0x0100 /* sent/received as link-level broadcast */ >> >> But, see the commit below, > > You are right. Removing the check in UDP, but not adding it in IP > was wrong. At least the broadcast flag in ip_input() was forgotten. > > The comment /* Make sure M_BCAST is set */ does not make sense. It > just describes the simple statement witten in the C code. Either > remove the comment, or explain why M_BCAST has to be set. > > Do not move pf_ouraddr() after route_mpath(). That counteracts the > idea of this function. If pf is smart and knows the packet is for > us, we don't need a route lookup. I see. The judgment by pf is prior to the other IP layer logic. Then I'd like to move checking for 255.255.255.255 after the pf_ouraddr() so that setting the flag is always done after the judgment by pf. ok? Index: sys/netinet/ip_input.c =================================================================== RCS file: /cvs/src/sys/netinet/ip_input.c,v diff -u -p -r1.404 ip_input.c --- sys/netinet/ip_input.c 2 Mar 2025 21:28:32 -0000 1.404 +++ sys/netinet/ip_input.c 12 Mar 2025 00:21:42 -0000 @@ -506,12 +506,6 @@ ip_input_if(struct mbuf **mp, int *offp, goto bad; } - if (ip->ip_dst.s_addr == INADDR_BROADCAST || - ip->ip_dst.s_addr == INADDR_ANY) { - nxt = ip_ours(mp, offp, nxt, af, ns); - goto out; - } - if (ns == NULL) { ro = &iproute; ro->ro_rt = NULL; @@ -864,6 +858,12 @@ in_ouraddr(struct mbuf *m, struct ifnet #endif ip = mtod(m, struct ip *); + + if (ip->ip_dst.s_addr == INADDR_BROADCAST || + ip->ip_dst.s_addr == INADDR_ANY) { + m->m_flags |= M_BCAST; + return (1); + } rt = route_mpath(ro, &ip->ip_dst, &ip->ip_src, m->m_pkthdr.ph_rtableid); if (rt != NULL) {