From owner-freebsd-bugs@FreeBSD.ORG Mon Jun 24 00:10:01 2013 Return-Path: Delivered-To: freebsd-bugs@smarthost.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 84B9A7D2 for ; Mon, 24 Jun 2013 00:10:01 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) by mx1.freebsd.org (Postfix) with ESMTP id 667EE1309 for ; Mon, 24 Jun 2013 00:10:01 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.7/8.14.7) with ESMTP id r5O0A1oA062963 for ; Mon, 24 Jun 2013 00:10:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.7/8.14.7/Submit) id r5O0A11U062962; Mon, 24 Jun 2013 00:10:01 GMT (envelope-from gnats) Resent-Date: Mon, 24 Jun 2013 00:10:01 GMT Resent-Message-Id: <201306240010.r5O0A11U062962@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Michael Gmelin Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 4D6797C7 for ; Mon, 24 Jun 2013 00:08:19 +0000 (UTC) (envelope-from freebsd@grem.de) Received: from mail.grem.de (outcast.grem.de [213.239.217.27]) by mx1.freebsd.org (Postfix) with SMTP id 745451302 for ; Mon, 24 Jun 2013 00:08:17 +0000 (UTC) Received: (qmail 83299 invoked by uid 0); 24 Jun 2013 00:02:38 -0000 Message-Id: <20130624000238.83298.qmail@mail.grem.de> Date: 24 Jun 2013 00:02:38 -0000 From: Michael Gmelin To: FreeBSD-gnats-submit@freebsd.org X-Send-Pr-Version: 3.113 Subject: kern/179901: [patch] Multicast SO_REUSEADDR handled incorrectly X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Jun 2013 00:10:01 -0000 >Number: 179901 >Category: kern >Synopsis: [patch] Multicast SO_REUSEADDR handled incorrectly >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Jun 24 00:10:00 UTC 2013 >Closed-Date: >Last-Modified: >Originator: Michael Gmelin >Release: FreeBSD 9.1-RELEASE-p2 amd64 >Organization: Grem Equity GmbH >Environment: System: FreeBSD box.grem.de 9.1-RELEASE-p2 FreeBSD 9.1-RELEASE-p2 #5 r249052M: Fri May 31 17:50:16 UTC >Description: Traditionally SO_REUSEADDR is used to indicate binding to the same address and port is permitted in the case of multicast addresses. BSD introduced the special sockopt SO_REUSEPORT and some point, but SO_REUSEADDR still implied SO_REUSEPORT automatically, which is important for portability reasons - client software expects SO_REUSEADDR to be sufficient and the kernel indicates that this is the way it should be for multicast addresses. While debugging an issue causing a unit test of the port devel/ice to fail, I realized that this is not handled correctly anymore [1]. As part of r227207, which was MFC'd in r227428 [2], handling of sockopts was slightly changed and setting of the implicit SO_REUSEPORT flag was moved to ip_output.c, which sets INP_REUSEPORT on inp->inp_flags2, so that these flags can be checked when binding additional sockets in in_pcb.c. Unfortunately this approach doesn't work, since these socket options are set before bind is called, and therefore checking IN_MULTICAST returns false at this point and the required INP_REUSEPORT option is never set implicitely. As a result, attempts to bind additional sockets to the same multicast address and port fail with EADDRINUSE ("Address already in use"), which breaks existing software. As a workaround, applications can be fixed by either using SO_REUSEPORT, or by calling setsockopt SO_REUSEADDR again after calling bind (because at that point the address is known and can be detected as multicast by ip_output.c). Changing application code is not desireable for obvious reasons. [1] http://lists.freebsd.org/pipermail/freebsd-ports/2013-June/084480.html [2] http://svnweb.freebsd.org/base?view=revision&revision=227428 >How-To-Repeat: Build the following program, which is also available at http://blog.grem.de/multicast.c # cc -o multicast -c multicast.c SNIP #include #include #include #include #include #include #include #include #include struct testdata { int port; int flag; int settwice; int expectOk; }; #define TDSIZE 20 #define BASEPORT 5555 const struct testdata td[TDSIZE] = { {0, SO_REUSEADDR, 0, 1}, {0, SO_REUSEADDR, 0, 1}, {0, SO_REUSEPORT, 0, 1}, {1, SO_REUSEPORT, 0, 1}, {1, SO_REUSEPORT, 0, 1}, {1, SO_REUSEADDR, 0, 1}, {1, SO_REUSEPORT, 0, 1}, {2, SO_REUSEADDR, 1, 1}, {2, SO_REUSEADDR, 1, 1}, {2, SO_REUSEPORT, 0, 1}, {2, SO_REUSEADDR, 0, 1}, {2, SO_REUSEPORT, 0, 1}, {3, 0, 0, 1}, {3, SO_REUSEADDR, 0, 0}, {3, SO_REUSEPORT, 0, 0}, {4, SO_REUSEADDR, 0, 1}, {4, 0, 0, 0}, {5, SO_REUSEPORT, 0, 1}, {5, SO_REUSEPORT, 0, 1}, {5, 0, 0, 0}, }; int main(int argc, char *argv[]) { int reuse = 1; struct sockaddr_in localSock; int sd; int i; int lastport = 0; int port; memset((char *) &localSock, 0, sizeof(localSock)); localSock.sin_family = AF_INET; localSock.sin_addr.s_addr = inet_addr("239.1.1.1"); for (i = 0; i < TDSIZE; ++i) { port = BASEPORT + td[i].port; localSock.sin_port = htons(port); sd = socket(AF_INET, SOCK_DGRAM, 0); if (lastport != port) { printf("Port %i:\n", port); lastport = port; } if (td[i].flag) { printf(" Bind using SO_REUSE%s%s...", td[i].flag == SO_REUSEADDR ? "ADDR" : "PORT", td[i].settwice?" x 2":"...."); setsockopt(sd, SOL_SOCKET, td[i].flag, (char *)&reuse, sizeof(reuse)); } else { printf(" Bind without socketopts......."); } if (bind(sd, (struct sockaddr*)&localSock, sizeof(localSock))) { printf("FAIL (%sexpected): %s\n", td[i].expectOk?"NOT ":"", strerror(errno)); close(sd); } else { printf("OK (%sexpected)\n", td[i].expectOk?"":"NOT "); } if (td[i].settwice && td[i].flag) setsockopt(sd, SOL_SOCKET, td[i].flag, (char *)&reuse, sizeof(reuse)); } } SNAP Running this on 9.1-RELEASE gives the following result: Port 5555: Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEADDR.......FAIL (NOT expected): Address already in use Bind using SO_REUSEPORT.......FAIL (NOT expected): Address already in use Port 5556: Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......FAIL (NOT expected): Address already in use Port 5557: Bind using SO_REUSEADDR x 2...OK (expected) Bind using SO_REUSEADDR x 2...OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......FAIL (NOT expected): Address already in use Port 5558: Bind without socketopts.......OK (expected) Bind using SO_REUSEADDR.......FAIL (expected): Address already in use Bind using SO_REUSEPORT.......FAIL (expected): Address already in use Port 5559: Bind using SO_REUSEADDR.......OK (expected) Bind without socketopts.......FAIL (expected): Address already in use Port 5560: Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind without socketopts.......FAIL (expected): Address already in use Out of curiosity, I ran the same code on an outdated version of FreeBSD (7.2-RELEASE): Port 5555: Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......FAIL (NOT expected): Address already in use Port 5556: Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......FAIL (NOT expected): Address already in use Port 5557: Bind using SO_REUSEADDR x 2...OK (expected) Bind using SO_REUSEADDR x 2...OK (expected) Bind using SO_REUSEPORT.......FAIL (NOT expected): Address already in use Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......FAIL (NOT expected): Address already in use Port 5558: Bind without socketopts.......OK (expected) Bind using SO_REUSEADDR.......FAIL (expected): Address already in use Bind using SO_REUSEPORT.......FAIL (expected): Address already in use Port 5559: Bind using SO_REUSEADDR.......OK (expected) Bind without socketopts.......FAIL (expected): Address already in use Port 5560: Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind without socketopts.......FAIL (expected): Address already in use This shows that there were issues with this code before r227207 as well - but only when mixing SO_REUSEADDR and SO_REUSEPORT, which hardly ever happens in practice. Nevertheless, this code didn't behave like it should either - at least in my understanding. >Fix: Disclaimer: The patch needs to be reviewed by somebody who has a better understanding of the code in question. The idea of the attached patch is to introduce a new flag called INP_REUSEADDR in in_pcb.h, which is set on inp->inp_flags2 in ip_output.c whenever SO_REUSEADDR is set on a socket. This way there is an explicit record of the setsockopt call which can be utilized, even though we don't know that the socket will be bound to a multicast address yet. The last step is to modify in_pcb.c (line 597), so that the condition for emitting EADDRINUSE only matches if it's not a multicast address or if it is a multicast address and neither INP_REUSEADDR nor INP_REUSEPORT are set. After applying the patch and rebuilding the kernel, multicast.c runs as expected: Port 5555: Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Port 5556: Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Port 5557: Bind using SO_REUSEADDR x 2...OK (expected) Bind using SO_REUSEADDR x 2...OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEADDR.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Port 5558: Bind without socketopts.......OK (expected) Bind using SO_REUSEADDR.......FAIL (expected): Address already in use Bind using SO_REUSEPORT.......FAIL (expected): Address already in use Port 5559: Bind using SO_REUSEADDR.......OK (expected) Bind without socketopts.......FAIL (expected): Address already in use Port 5560: Bind using SO_REUSEPORT.......OK (expected) Bind using SO_REUSEPORT.......OK (expected) Bind without socketopts.......FAIL (expected): Address already in use The patch is applied using: # cd /usr/src # patch < /path/to/freebsd-multicast.patch --- freebsd-multicast.patch begins here --- --- sys/netinet/in_pcb.c.orig 2013-06-23 20:52:10.000000000 +0000 +++ sys/netinet/in_pcb.c 2013-06-23 21:14:45.000000000 +0000 @@ -594,7 +594,11 @@ (reuseport & tw->tw_so_options) == 0) return (EADDRINUSE); } else if (t && (reuseport == 0 || - (t->inp_flags2 & INP_REUSEPORT) == 0)) { + (!IN_MULTICAST(ntohl(sin->sin_addr.s_addr)) && + (t->inp_flags2 & INP_REUSEPORT) == 0) || + (IN_MULTICAST(ntohl(sin->sin_addr.s_addr)) && + (t->inp_flags2 & + (INP_REUSEADDR | INP_REUSEPORT)) == 0))) { #ifdef INET6 if (ntohl(sin->sin_addr.s_addr) != INADDR_ANY || --- sys/netinet/in_pcb.h.orig 2013-06-23 21:02:42.000000000 +0000 +++ sys/netinet/in_pcb.h 2013-06-23 21:03:40.000000000 +0000 @@ -542,6 +542,7 @@ #define INP_RT_VALID 0x00000002 /* cached rtentry is valid */ #define INP_PCBGROUPWILD 0x00000004 /* in pcbgroup wildcard list */ #define INP_REUSEPORT 0x00000008 /* SO_REUSEPORT option is set */ +#define INP_REUSEADDR 0x00000010 /* SO_REUSEADDR option is set */ /* * Flags passed to in_pcblookup*() functions. --- sys/netinet/ip_output.c.orig 2013-06-23 21:03:51.000000000 +0000 +++ sys/netinet/ip_output.c 2013-06-23 21:04:58.000000000 +0000 @@ -915,6 +915,10 @@ else inp->inp_flags2 &= ~INP_REUSEPORT; } + if ((so->so_options & SO_REUSEADDR) != 0) + inp->inp_flags2 |= INP_REUSEADDR; + else + inp->inp_flags2 &= ~INP_REUSEADDR; INP_WUNLOCK(inp); error = 0; break; --- freebsd-multicast.patch ends here --- >Release-Note: >Audit-Trail: >Unformatted: