On Tue, 11 Mar 2025 14:48:28 +0100
Alexander Bluhm <alexander.bluhm@gmx.net> 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 <bluhm@openbsd.org> 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) {