netinet: enforce broadcast mode for all-ones and all-zeroes destinations
Needs ReviewPublic
Actions

Authored by glebius on Mon, Feb 17, 11:38 PM.

Details

Reviewers

rstone
markj
ae

Group Reviewers

network

Summary

When a socket has SO_BROADCAST set and destination address is INADDR_ANY
or INADDR_BROADCAST, the kernel shall pick up first broadcast capable
interface and broadcast the packet out of it. Since this API is not
reliable on a machine with > 1 broadcast capable interfaces, all practical
software seems to use IP_ONESBCAST or other mechanisms to send broadcasts.
This has been broken at least since FreeBSD 6.0, see bug 99558. Back then
the problem was in the fact that in_broadcast() check was always done
against the gateway address, not the destination address. Later, with
90cc51a1ab4be, a second problem piled on top - we aren't checking for
INADDR_ANY and INADDR_BROADCAST at all.

Better late than never, fix that by checking destination address.

PR: 99558

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 62554
Build 59438: arc lint + arc unit

Event Timeline

glebius created this revision.Mon, Feb 17, 11:38 PM

Herald added subscribers: melifaro, imp. · View Herald TranscriptMon, Feb 17, 11:38 PM

glebius requested review of this revision.Mon, Feb 17, 11:38 PM

glebius added a parent revision: D49041: netinet: rename in_broadcast() to in_ifnet_broadcast().

glebius added a child revision: D49043: udp: make sendto(2) on unconnected UDP socket use public inpcb KPIs.

Harbormaster completed remote builds in B62494: Diff 151134.Mon, Feb 17, 11:38 PM

glebius added a reviewer: rstone.Mon, Feb 17, 11:38 PM

glebius mentioned this in D49041: netinet: rename in_broadcast() to in_ifnet_broadcast().

glebius added a reviewer: markj.Wed, Feb 19, 5:14 PM

glebius added a reviewer: ae.Thu, Feb 20, 5:20 AM

zlei added a subscriber: zlei.Thu, Feb 20, 6:22 AM

markj added inline comments.Thu, Feb 20, 6:35 PM

sys/netinet/in.h
690	Why add it to the header? Is there a follow-up patch which adds another consumer?
692	This is fine because these addresses have the same representation in both host and network order, but formally we should check ntohl(in.s_addr) instead.

glebius added inline comments.Thu, Feb 20, 8:11 PM

sys/netinet/in.h
690	I don't have a patch/review yet, but indeed there are few places which have exactly this check unrolled.
692	I'd rather add htonl() around the constants. Will do.