From owner-svn-src-head@FreeBSD.ORG Sun Nov 6 10:51:20 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 66B5F106564A; Sun, 6 Nov 2011 10:51:20 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id 4F24E8FC14; Sun, 6 Nov 2011 10:51:18 +0000 (UTC) Received: by faar19 with SMTP id r19so6109418faa.13 for ; Sun, 06 Nov 2011 02:51:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=from:to:cc:subject:references:x-comment-to:sender:date:in-reply-to :message-id:user-agent:mime-version:content-type; bh=fmDTxO8feWZxv4f2uWLO8RdfwfumMXpidvQkYa/wKGc=; b=cULbNPO9CWVIjCajg/fezC0JAW7Llrp1nsZSSv5kGbKAS61+5eRMagB+uXP2JiQX98 Xtibcnnw8REPuIRtET1F4GHJS0ko83jCeViM6alonTLXig2TW/kkMwfOYGrVGKhUOSs8 PSft07u5yIg1Hhuz1d4jheBOsMIsX6k+BUui0= Received: by 10.223.30.149 with SMTP id u21mr38663224fac.18.1320576678083; Sun, 06 Nov 2011 02:51:18 -0800 (PST) Received: from localhost ([95.69.173.122]) by mx.google.com with ESMTPS id x19sm26051655fag.5.2011.11.06.02.51.15 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 06 Nov 2011 02:51:16 -0800 (PST) From: Mikolaj Golub To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201111061047.pA6AlKnc017568@svn.freebsd.org> X-Comment-To: Mikolaj Golub Sender: Mikolaj Golub Date: Sun, 06 Nov 2011 12:51:13 +0200 In-Reply-To: <201111061047.pA6AlKnc017568@svn.freebsd.org> (Mikolaj Golub's message of "Sun, 6 Nov 2011 10:47:20 +0000 (UTC)") Message-ID: <86r51lcyn2.fsf@kopusha.home.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Robert Watson Subject: Re: svn commit: r227207 - in head/sys: netinet netinet6 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 06 Nov 2011 10:51:20 -0000 On Sun, 6 Nov 2011 10:47:20 +0000 (UTC) Mikolaj Golub wrote: MG> Author: trociny MG> Date: Sun Nov 6 10:47:20 2011 MG> New Revision: 227207 MG> URL: http://svn.freebsd.org/changeset/base/227207 MG> Log: MG> Cache SO_REUSEPORT socket option in inpcb-layer in order to avoid MG> inp_socket->so_options dereference when we may not acquire the lock on MG> the inpcb. MG> MG> This fixes the crash due to NULL pointer dereference in MG> in_pcbbind_setup() when inp_socket->so_options in a pcb returned by MG> in_pcblookup_local() was checked. MG> MG> Reported by: dave jones , Arnaud Lacombe MG> Suggested by: rwatson MG> Glanced by: rwatson MG> Tested by: dave jones This commit fixes the panic reported by Dave for 9.0 triggered by named. Robert has helped very much suggesting the solution and looking at the patches. Unfortunately being saturated on free time he couldn't do thorough review of the final version confirming only that presumably the approach was correct. I made an effort to check that there was no regression and SO_REUSEADDR worked the same way as it had worked before. But I can't be 100% confident that I haven't broken something. Because of this I am going to MFC only after the release. Here is the initial discussion of the issue: http://lists.freebsd.org/pipermail/freebsd-net/2011-September/029858.html MG> Modified: MG> head/sys/netinet/in_pcb.c MG> head/sys/netinet/in_pcb.h MG> head/sys/netinet/ip_output.c MG> head/sys/netinet6/in6_pcb.c MG> head/sys/netinet6/ip6_output.c MG> Modified: head/sys/netinet/in_pcb.c MG> ============================================================================== MG> --- head/sys/netinet/in_pcb.c Sun Nov 6 09:29:52 2011 (r227206) MG> +++ head/sys/netinet/in_pcb.c Sun Nov 6 10:47:20 2011 (r227207) MG> @@ -575,8 +575,7 @@ in_pcbbind_setup(struct inpcb *inp, stru MG> ntohl(t->inp_faddr.s_addr) == INADDR_ANY) && MG> (ntohl(sin->sin_addr.s_addr) != INADDR_ANY || MG> ntohl(t->inp_laddr.s_addr) != INADDR_ANY || MG> - (t->inp_socket->so_options & MG> - SO_REUSEPORT) == 0) && MG> + (t->inp_flags2 & INP_REUSEPORT) == 0) && MG> (inp->inp_cred->cr_uid != MG> t->inp_cred->cr_uid)) MG> return (EADDRINUSE); MG> @@ -594,15 +593,15 @@ in_pcbbind_setup(struct inpcb *inp, stru MG> if (tw == NULL || MG> (reuseport & tw->tw_so_options) == 0) MG> return (EADDRINUSE); MG> - } else if (t && MG> - (reuseport & t->inp_socket->so_options) == 0) { MG> + } else if (t && (reuseport == 0 || MG> + (t->inp_flags2 & INP_REUSEPORT) == 0)) { MG> #ifdef INET6 MG> if (ntohl(sin->sin_addr.s_addr) != MG> INADDR_ANY || MG> ntohl(t->inp_laddr.s_addr) != MG> INADDR_ANY || MG> - INP_SOCKAF(so) == MG> - INP_SOCKAF(t->inp_socket)) MG> + (inp->inp_vflag & INP_IPV6PROTO) == 0 || MG> + (t->inp_vflag & INP_IPV6PROTO) == 0) MG> #endif MG> return (EADDRINUSE); MG> } MG> Modified: head/sys/netinet/in_pcb.h MG> ============================================================================== MG> --- head/sys/netinet/in_pcb.h Sun Nov 6 09:29:52 2011 (r227206) MG> +++ head/sys/netinet/in_pcb.h Sun Nov 6 10:47:20 2011 (r227207) MG> @@ -540,6 +540,7 @@ void inp_4tuple_get(struct inpcb *inp, MG> #define INP_LLE_VALID 0x00000001 /* cached lle is valid */ MG> #define INP_RT_VALID 0x00000002 /* cached rtentry is valid */ MG> #define INP_PCBGROUPWILD 0x00000004 /* in pcbgroup wildcard list */ MG> +#define INP_REUSEPORT 0x00000008 /* SO_REUSEPORT option is set */ MG> MG> /* MG> * Flags passed to in_pcblookup*() functions. MG> Modified: head/sys/netinet/ip_output.c MG> ============================================================================== MG> --- head/sys/netinet/ip_output.c Sun Nov 6 09:29:52 2011 (r227206) MG> +++ head/sys/netinet/ip_output.c Sun Nov 6 10:47:20 2011 (r227207) MG> @@ -895,12 +895,43 @@ ip_ctloutput(struct socket *so, struct s MG> MG> error = optval = 0; MG> if (sopt->sopt_level != IPPROTO_IP) { MG> - if ((sopt->sopt_level == SOL_SOCKET) && MG> - (sopt->sopt_name == SO_SETFIB)) { MG> - inp->inp_inc.inc_fibnum = so->so_fibnum; MG> - return (0); MG> + error = EINVAL; MG> + MG> + if (sopt->sopt_level == SOL_SOCKET && MG> + sopt->sopt_dir == SOPT_SET) { MG> + switch (sopt->sopt_name) { MG> + case SO_REUSEADDR: MG> + INP_WLOCK(inp); MG> + if (IN_MULTICAST(ntohl(inp->inp_laddr.s_addr))) { MG> + if ((so->so_options & MG> + (SO_REUSEADDR | SO_REUSEPORT)) != 0) MG> + inp->inp_flags2 |= INP_REUSEPORT; MG> + else MG> + inp->inp_flags2 &= ~INP_REUSEPORT; MG> + } MG> + INP_WUNLOCK(inp); MG> + error = 0; MG> + break; MG> + case SO_REUSEPORT: MG> + INP_WLOCK(inp); MG> + if ((so->so_options & SO_REUSEPORT) != 0) MG> + inp->inp_flags2 |= INP_REUSEPORT; MG> + else MG> + inp->inp_flags2 &= ~INP_REUSEPORT; MG> + INP_WUNLOCK(inp); MG> + error = 0; MG> + break; MG> + case SO_SETFIB: MG> + INP_WLOCK(inp); MG> + inp->inp_inc.inc_fibnum = so->so_fibnum; MG> + INP_WUNLOCK(inp); MG> + error = 0; MG> + break; MG> + default: MG> + break; MG> + } MG> } MG> - return (EINVAL); MG> + return (error); MG> } MG> MG> switch (sopt->sopt_dir) { MG> Modified: head/sys/netinet6/in6_pcb.c MG> ============================================================================== MG> --- head/sys/netinet6/in6_pcb.c Sun Nov 6 09:29:52 2011 (r227206) MG> +++ head/sys/netinet6/in6_pcb.c Sun Nov 6 10:47:20 2011 (r227207) MG> @@ -207,8 +207,8 @@ in6_pcbbind(register struct inpcb *inp, MG> IN6_IS_ADDR_UNSPECIFIED(&t->in6p_faddr)) && MG> (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr) || MG> !IN6_IS_ADDR_UNSPECIFIED(&t->in6p_laddr) || MG> - (t->inp_socket->so_options & SO_REUSEPORT) MG> - == 0) && (inp->inp_cred->cr_uid != MG> + (t->inp_flags2 & INP_REUSEPORT) == 0) && MG> + (inp->inp_cred->cr_uid != MG> t->inp_cred->cr_uid)) MG> return (EADDRINUSE); MG> #ifdef INET MG> @@ -267,12 +267,10 @@ in6_pcbbind(register struct inpcb *inp, MG> INP_IPV6PROTO) == MG> (t->inp_vflag & INP_IPV6PROTO)))) MG> return (EADDRINUSE); MG> - } MG> - else if (t && MG> - (reuseport & t->inp_socket->so_options) MG> - == 0 && (ntohl(t->inp_laddr.s_addr) != MG> - INADDR_ANY || INP_SOCKAF(so) == MG> - INP_SOCKAF(t->inp_socket))) MG> + } else if (t && (reuseport == 0 || MG> + (t->inp_flags2 & INP_REUSEPORT) == 0) && MG> + (ntohl(t->inp_laddr.s_addr) != INADDR_ANY || MG> + (t->inp_vflag & INP_IPV6PROTO) == 0)) MG> return (EADDRINUSE); MG> } MG> #endif MG> Modified: head/sys/netinet6/ip6_output.c MG> ============================================================================== MG> --- head/sys/netinet6/ip6_output.c Sun Nov 6 09:29:52 2011 (r227206) MG> +++ head/sys/netinet6/ip6_output.c Sun Nov 6 10:47:20 2011 (r227207) MG> @@ -1421,7 +1421,38 @@ ip6_ctloutput(struct socket *so, struct MG> optval = 0; MG> uproto = (int)so->so_proto->pr_protocol; MG> MG> - if (level == IPPROTO_IPV6) { MG> + if (level != IPPROTO_IPV6) { MG> + error = EINVAL; MG> + MG> + if (sopt->sopt_level == SOL_SOCKET && MG> + sopt->sopt_dir == SOPT_SET) { MG> + switch (sopt->sopt_name) { MG> + case SO_REUSEADDR: MG> + INP_WLOCK(in6p); MG> + if (IN_MULTICAST(ntohl(in6p->inp_laddr.s_addr))) { MG> + if ((so->so_options & MG> + (SO_REUSEADDR | SO_REUSEPORT)) != 0) MG> + in6p->inp_flags2 |= INP_REUSEPORT; MG> + else MG> + in6p->inp_flags2 &= ~INP_REUSEPORT; MG> + } MG> + INP_WUNLOCK(in6p); MG> + error = 0; MG> + break; MG> + case SO_REUSEPORT: MG> + INP_WLOCK(in6p); MG> + if ((so->so_options & SO_REUSEPORT) != 0) MG> + in6p->inp_flags2 |= INP_REUSEPORT; MG> + else MG> + in6p->inp_flags2 &= ~INP_REUSEPORT; MG> + INP_WUNLOCK(in6p); MG> + error = 0; MG> + break; MG> + default: MG> + break; MG> + } MG> + } MG> + } else { /* level == IPPROTO_IPV6 */ MG> switch (op) { MG> MG> case SOPT_SET: MG> @@ -2044,8 +2075,6 @@ do { \ MG> } MG> break; MG> } MG> - } else { /* level != IPPROTO_IPV6 */ MG> - error = EINVAL; MG> } MG> return (error); MG> } -- Mikolaj Golub