Date: Sun, 06 Nov 2011 12:51:13 +0200 From: Mikolaj Golub <trociny@freebsd.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Cc: Robert Watson <rwatson@freebsd.org> Subject: Re: svn commit: r227207 - in head/sys: netinet netinet6 Message-ID: <86r51lcyn2.fsf@kopusha.home.net> In-Reply-To: <201111061047.pA6AlKnc017568@svn.freebsd.org> (Mikolaj Golub's message of "Sun, 6 Nov 2011 10:47:20 %2B0000 (UTC)") References: <201111061047.pA6AlKnc017568@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <s.dave.jones@gmail.com>, Arnaud Lacombe <lacombar@gmail.com> MG> Suggested by: rwatson MG> Glanced by: rwatson MG> Tested by: dave jones <s.dave.jones@gmail.com> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86r51lcyn2.fsf>