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().

> 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.

> ok? comments?

Can you try to remove the goto sendit instead?

> Index: sys/netinet/ip_output.c
> ===================================================================
> RCS file: /disk/cvs/openbsd/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	10 Mar 2025 01:40:17 -0000
> @@ -246,8 +246,14 @@ reroute:
>  	if (IN_MULTICAST(ip->ip_dst.s_addr) ||
>  	    (ip->ip_dst.s_addr == INADDR_BROADCAST)) {
>  
> -		m->m_flags |= (ip->ip_dst.s_addr == INADDR_BROADCAST) ?
> -			M_BCAST : M_MCAST;
> +		if (ip->ip_dst.s_addr == INADDR_BROADCAST) {
> +			if ((flags & IP_ALLOWBROADCAST) == 0) {
> +				error = EACCES;
> +				goto bad;
> +			}
> +			m->m_flags |= M_BCAST;
> +		} else
> +			m->m_flags |= M_MCAST;
>  
>  		/*
>  		 * IP destination address is multicast.  Make sure "dst"