Date: Sat, 4 Jun 2011 15:54:26 +0100 From: "Robert N. M. Watson" <rwatson@FreeBSD.org> To: Kristof Provost <kristof@sigsegv.be> Cc: "freebsd-current@FreeBSD.org Current" <freebsd-current@freebsd.org> Subject: Divert socket problem (was: Re: svn commit: r222488 - in head/sys: contrib/pf/net netinet netinet/ipfw netinet6) Message-ID: <64903CD6-7AED-4BF6-9FDF-B6748690EA8A@FreeBSD.org> In-Reply-To: <20110604143043.GE17764@nereid> References: <201105300943.p4U9htjI070096@svn.freebsd.org> <20110604143043.GE17764@nereid>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail-7-327068992 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii On 4 Jun 2011, at 15:30, Kristof Provost wrote: > div_bind probably also needs to surround the call to in_pcbbind with > INP_HASHW(UN)LOCK(...) >=20 > I'm currently running 222680. I've only now seen the issue, but I've = also > just now activated INVARIANTS. Hi Kristof: Thanks for the detailed report, and yes, it looks like that is exactly = what is required. Could you try the attached patch? Robert --Apple-Mail-7-327068992 Content-Disposition: attachment; filename=20110604-divert-fix.diff Content-Type: application/octet-stream; x-unix-mode=0644; name="20110604-divert-fix.diff" Content-Transfer-Encoding: 7bit IP divert sockets use their inpcbinfo for port reservation, although not for lookup. I missed its call to in_pcbbind() when preparing previous patches, which would lead to a lock assertion failure (although problem not an actual race condition due to global pcbinfo locks providing required synchronisation -- in this particular case only). This change adds the missing locking of the pcbhash lock. (Existing comments in the ipdivert code question the need for using the global hash to manage the namespace, as really it's a simple port namespace and not an address/port namespace. Also, although in_pcbbind is used to manage reservations, the hash tables aren't used for lookup. It might be a good idea to make them use hashed lookup, or to use a different reservation scheme.) Reviewed by: bz Reported by: Kristof Provost <kristof at sigsegv.be> Sponsored by: Juniper Networks Index: ip_divert.c =================================================================== --- ip_divert.c (revision 222672) +++ ip_divert.c (working copy) @@ -530,7 +530,9 @@ ((struct sockaddr_in *)nam)->sin_addr.s_addr = INADDR_ANY; INP_INFO_WLOCK(&V_divcbinfo); INP_WLOCK(inp); + INP_HASH_WLOCK(&V_divcbinfo); error = in_pcbbind(inp, nam, td->td_ucred); + INP_HASH_WUNLOCK(&V_divcbinfo); INP_WUNLOCK(inp); INP_INFO_WUNLOCK(&V_divcbinfo); return error; --Apple-Mail-7-327068992--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?64903CD6-7AED-4BF6-9FDF-B6748690EA8A>