Created attachment 183567 [details] lock access to INADDR_TO_IFP in the ipfw There are several places in kernel sources where code performs unlocked access to the hash of host's IP addresses. If adresses change often enough, a panic may occur. For example, part of kgdb script of latest panic of my mpd server: 1642 case O_IP_DST_ME: 1643 if (is_ipv4) { 1644 struct ifnet *tif; 1645 1646 INADDR_TO_IFP(dst_ip, tif); (kgdb) p ia $3 = (struct in_ifaddr *) 0xdeadc0dedeadc0de Attached patches add needed locking to several such places including ipfw.
Created attachment 183568 [details] lock access to INADDR_TO_IFP in the in_mcast.c
Created attachment 183569 [details] lock access to INADDR_HASH in the stf(4)
Created attachment 183570 [details] lock access to INADDR_HASH in the ip_input()
Created attachment 183590 [details] Proposed patch (untested) For ipfw(4) I prefer to use this patch. I believe it has a bit less overhead. Can you test it?
(In reply to Andrey V. Elsukov from comment #4) I've just rebuilt the kernel with your version of patch instead of old one and restarted my tests. New version does basically same thing, so I do not expect problems. The system exhibited no problems at all during a day with my stress test using all patches involving (for kernel, libc and mpd itself). Unless something bad happens next day, I'm going to put it in "testing production".
(In reply to Eugene Grosbein from comment #5) Eugene, should i apply this one too?
(In reply to Cassiano Peixoto from comment #6) This PR presents several patches for several kernel subsystems. Apply patches for ip_mcast.c and ip_input(). The patch for ipfw is only needed if you use ipfw and have a rule with "me" keyword like "allow ip from any to me". Better use version by ae@ (https://bugs.freebsd.org/bugzilla/attachment.cgi?id=183590). The patch for stf(4) is only needed if you use stf(4).
(In reply to Eugene Grosbein from comment #7) Do i need to recompile the world or only the kernel?
(In reply to Cassiano Peixoto from comment #8) Kernel only.
(In reply to Eugene Grosbein from comment #9) mcast has been reject: # patch < mcast.patch Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |--- sys/netinet/in_mcast.c.orig 2017-04-20 15:01:10.786840000 +0700 |+++ sys/netinet/in_mcast.c 2017-06-17 18:24:34.034823000 +0700 -------------------------- Patching file sys/netinet/in_mcast.c using Plan A... Hunk #1 succeeded at 1340 (offset 2 lines). Hunk #2 succeeded at 1378 (offset 2 lines). Hunk #3 succeeded at 1878 (offset 2 lines). Hunk #4 succeeded at 1895 (offset 2 lines). Hunk #5 succeeded at 2229 (offset 2 lines). Hunk #6 failed at 2288. Hunk #7 succeeded at 2452 (offset 2 lines). Hunk #8 succeeded at 2491 (offset 2 lines). 1 out of 8 hunks failed--saving rejects to sys/netinet/in_mcast.c.rej done Exit 1 # cat sys/netinet/in_mcast.c.rej @@ -2283,9 +2288,11 @@ * XXX NOTE WELL: The RFC 3678 API is preferred because * using an IPv4 address as a key is racy. */ - if (!in_nullhost(mreqs.imr_interface)) + if (!in_nullhost(mreqs.imr_interface)) { + IN_IFADDR_RLOCK(&in_ifa_tracker); INADDR_TO_IFP(mreqs.imr_interface, ifp); - + IN_IFADDR_RUNLOCK(&in_ifa_tracker); + } CTR3(KTR_IGMPV3, "%s: imr_interface = 0x%08x, ifp = %p", __func__, ntohl(mreqs.imr_interface.s_addr), ifp);
(In reply to Cassiano Peixoto from comment #10) You have revision earlier than 315456 that changed context for this patch: - CTR3(KTR_IGMPV3, "%s: imr_interface = %s, ifp = %p", - __func__, inet_ntoa(mreqs.imr_interface), ifp); + CTR3(KTR_IGMPV3, "%s: imr_interface = 0x%08x, ifp = %p", + __func__, ntohl(mreqs.imr_interface.s_addr), ifp); That's not big deal. Either apply this small chunk manually, or update your system to latest stable/11.
(In reply to Cassiano Peixoto from comment #10) You can also just skip this patch for now if your system does not process any multicast traffic including routing protocols like OSPF or RIPv2.
(In reply to Eugene Grosbein from comment #11) There is a reference for CTR3() 3 times in the code. Should i replace all of them? Thanks.
(In reply to Cassiano Peixoto from comment #13) You do not need to change context of the patch. That is, do not touch lines referring to CTR3. Just make supposed change manually, e.g. add lines with IN_IFADDR_RLOCK/IN_IFADDR_RUNLOCK and fix braces.
(In reply to Eugene Grosbein from comment #14) Ok got it. Patch applied fine. Thanks.
(In reply to Andrey V. Elsukov from comment #4) Andrey, there is no problems with your patch for ipfw. Please commit.
*** Bug 162558 has been marked as a duplicate of this bug. ***
A commit references this bug: Author: ae Date: Wed Sep 20 22:35:28 UTC 2017 New revision: 323839 URL: https://svnweb.freebsd.org/changeset/base/323839 Log: Use in_localip() function instead of unlocked access to addresses hash to determine that an address is our local. PR: 220078 MFC after: 1 week Changes: head/sys/netpfil/ipfw/ip_fw2.c
My PR.
A commit references this bug: Author: ae Date: Wed Sep 27 01:47:54 UTC 2017 New revision: 324047 URL: https://svnweb.freebsd.org/changeset/base/324047 Log: MFC r323839: Use in_localip() function instead of unlocked access to addresses hash to determine that an address is our local. PR: 220078 Changes: _U stable/11/ stable/11/sys/netpfil/ipfw/ip_fw2.c
Created attachment 187688 [details] stable/10: lock access to INADDR_HASH in the ip_input() Fix for ip_input() adjusted to stable/10 code base.
Created attachment 187689 [details] stable/10: lock access to INADDR_TO_IFP in the in_mcast.c Fix for in_mcast.c adjusted to stable/10 code base.
*** Bug 222617 has been marked as a duplicate of this bug. ***
Also https://reviews.freebsd.org/D12457
*** Bug 195102 has been marked as a duplicate of this bug. ***
A commit references this bug: Author: eugen Date: Sat Oct 27 04:45:28 UTC 2018 New revision: 339806 URL: https://svnweb.freebsd.org/changeset/base/339806 Log: Prevent stf(4) from panicing due to unprotected access to INADDR_HASH. PR: 220078 MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: head/sys/net/if_stf.c
A commit references this bug: Author: eugen Date: Sat Oct 27 04:53:26 UTC 2018 New revision: 339807 URL: https://svnweb.freebsd.org/changeset/base/339807 Log: Prevent multicast code from panicing due to unprotected access to INADDR_HASH. PR: 220078 MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: head/sys/netinet/in_mcast.c
A commit references this bug: Author: eugen Date: Sat Oct 27 04:59:36 UTC 2018 New revision: 339808 URL: https://svnweb.freebsd.org/changeset/base/339808 Log: Prevent ip_input() from panicing due to unprotected access to INADDR_HASH. PR: 220078 MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: head/sys/netinet/ip_input.c
A commit references this bug: Author: eugen Date: Mon Nov 26 10:47:01 UTC 2018 New revision: 340934 URL: https://svnweb.freebsd.org/changeset/base/340934 Log: MFC r339807: Prevent multicast code from panicing due to unprotected access to INADDR_HASH. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: _U stable/12/ stable/12/sys/netinet/in_mcast.c
A commit references this bug: Author: eugen Date: Mon Nov 26 10:50:50 UTC 2018 New revision: 340935 URL: https://svnweb.freebsd.org/changeset/base/340935 Log: MFC r339807: Prevent multicast code from panicing due to unprotected access to INADDR_HASH. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: _U stable/11/ stable/11/sys/netinet/in_mcast.c
A commit references this bug: Author: eugen Date: Mon Nov 26 11:15:59 UTC 2018 New revision: 340950 URL: https://svnweb.freebsd.org/changeset/base/340950 Log: MFC r339806: Prevent stf(4) from panicing due to unprotected access to INADDR_HASH. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: _U stable/12/ stable/12/sys/net/if_stf.c
A commit references this bug: Author: eugen Date: Mon Nov 26 11:17:12 UTC 2018 New revision: 340951 URL: https://svnweb.freebsd.org/changeset/base/340951 Log: MFC r339806: Prevent stf(4) from panicing due to unprotected access to INADDR_HASH. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: _U stable/11/ stable/11/sys/net/if_stf.c
A commit references this bug: Author: eugen Date: Mon Nov 26 11:51:44 UTC 2018 New revision: 340957 URL: https://svnweb.freebsd.org/changeset/base/340957 Log: Prevent multicast code from panicing due to unprotected access to INADDR_HASH. This is direct commit to stable/10 instead of MFC r339807 due to significant difference in code base. PR: 220078 Tested-by: Cassiano Peixoto and others Changes: stable/10/sys/netinet/in_mcast.c
A commit references this bug: Author: eugen Date: Mon Nov 26 12:19:31 UTC 2018 New revision: 340958 URL: https://svnweb.freebsd.org/changeset/base/340958 Log: Prevent stf(4) from panicing due to unprotected access to INADDR_HASH. This is direct commit to stable/10 instead of MFC r339806 due to significant differences in code base. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: stable/10/sys/net/if_stf.c
A commit references this bug: Author: eugen Date: Mon Nov 26 12:39:57 UTC 2018 New revision: 340959 URL: https://svnweb.freebsd.org/changeset/base/340959 Log: MFC r339808: Prevent ip_input() from panicing due to unprotected access to INADDR_HASH. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: _U stable/12/ stable/12/sys/netinet/ip_input.c
A commit references this bug: Author: eugen Date: Mon Nov 26 12:41:50 UTC 2018 New revision: 340960 URL: https://svnweb.freebsd.org/changeset/base/340960 Log: MFC r339808: Prevent ip_input() from panicing due to unprotected access to INADDR_HASH. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: _U stable/11/ stable/11/sys/netinet/ip_input.c
A commit references this bug: Author: eugen Date: Mon Nov 26 12:47:13 UTC 2018 New revision: 340961 URL: https://svnweb.freebsd.org/changeset/base/340961 Log: Prevent ip_input() from panicing due to unprotected access to INADDR_HASH. This is direct commit to stable/10 instead of MFC r339808 due to significant differences in code base. PR: 220078 Differential Revision: https://reviews.freebsd.org/D12457 Tested-by: Cassiano Peixoto and others Changes: stable/10/sys/netinet/ip_input.c
Fixed and merged to all supported branches: stable/10, stable/11 and stable/12.