On Mon, 10 Mar 2025 19:13:13 +0100
Alexander Bluhm <bluhm@openbsd.org> wrote:
> On Mon, Mar 10, 2025 at 10:55:24AM +0900, YASUOKA Masahiko wrote:
>> Hello,
>> 
>> I think SO_BROADCAST flag is needed to send a packet to a broadcast
>> address.  But actually, our stack doesn't require the flag.  The
>> behavior was introduced by the the following change on
>> sys/netinet/ip_output.c .
>> 
>> ---
>> revision 1.103
>> date: 2001/06/14 18:00:02;  author: provos;  state: Exp;  lines: +9 -4;
>> limited broadcast 255.255.255.255 was not recognized correctly, reported
>> by crh@ubiqx.mn.org, fix from NetBSD; okay angelos@
>> ---
>> (https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/netinet/ip_output.c#rev1.103)
> 
> In NetBSD it was this commit that changed behavior.
> 
> ----------------------------
> revision 1.64
> date: 1999-12-13 17:04:11 +0000;  author: is;  state: Exp;  lines: +9 -4;
> Handle packets to 255.255.255.255 like multicast packets. Fixes PR 7682 by
> Darren Reed.
> ----------------------------
> 
> The problematic logic seems to be the goto sendit.  This skips the
> existing check for IP_ALLOWBROADCAST.  Should we just remove the
> goto sendit to get even more checks?  This was the behavior before
> the NetBSD change.
> 
> The goto sendit looks like a shortcut to avoid expensive in_broadcast()
> function which we don't call in ip_output().

Ah, it likely was so.

>> To test the behavior, run tcpdump on the interface of the default
>> gateway and send a packet to 255.255.255.255 by a command like the
>> following.
>> 
>>   echo 'AAAA' | nc -u 255.255.255.255 65535
> 
> I can confirm that broadcast packets are send without SO_BROADCAST
> option.  Question is if a fix breaks userland assumtions afer 25
> years of misbehavior.  FreeBSD uses the original 4.4BSD logic and
> is not affected.  So I think we are safe to fix it.

Thanks.  I also think we are safe.

>> ok? comments?
> 
> Can you try to remove the goto sendit instead?

Yes.

Index: sys/netinet/ip_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_output.c,v
diff -u -p -r1.406 ip_output.c
--- sys/netinet/ip_output.c	2 Mar 2025 21:28:32 -0000	1.406
+++ sys/netinet/ip_output.c	11 Mar 2025 03:47:16 -0000
@@ -348,8 +348,6 @@ reroute:
 		 */
 		if (ip->ip_ttl == 0 || (ifp->if_flags & IFF_LOOPBACK) != 0)
 			goto bad;
-
-		goto sendit;
 	}
 
 	/*
@@ -377,7 +375,6 @@ reroute:
 	} else
 		m->m_flags &= ~M_BCAST;
 
-sendit:
 	/*
 	 * If we're doing Path MTU discovery, we need to set DF unless
 	 * the route's MTU is locked.