On Tue, Mar 11, 2025 at 03:48:41PM +0900, YASUOKA Masahiko wrote:
> 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.

OK bluhm@

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