Author: hselasky
Date: Tue Dec 15 09:02:05 2015
New Revision: 292254
URL: https://svnweb.freebsd.org/changeset/base/292254

Log:
  Properly drain callouts in the IPFW subsystem to avoid use after free
  panics when unloading the dummynet and IPFW modules:
  
  - The callout drain function can sleep and should not be called having
  a non-sleepable lock locked. Remove locks around "ipfw_dyn_uninit(0)".
  
  - Add a new "dn_gone" variable to prevent asynchronous restart of
  dummynet callouts when unloading the dummynet kernel module.
  
  - Call "dn_reschedule()" locked so that "dn_gone" can be set and
  checked atomically with regard to starting a new callout.
  
  Reviewed by:  hiren
  MFC after:    1 week
  Differential Revision:        https://reviews.freebsd.org/D3855

Modified:
  head/sys/netpfil/ipfw/ip_dn_io.c
  head/sys/netpfil/ipfw/ip_dummynet.c
  head/sys/netpfil/ipfw/ip_fw2.c

Modified: head/sys/netpfil/ipfw/ip_dn_io.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_dn_io.c    Tue Dec 15 06:01:02 2015        
(r292253)
+++ head/sys/netpfil/ipfw/ip_dn_io.c    Tue Dec 15 09:02:05 2015        
(r292254)
@@ -711,8 +711,8 @@ dummynet_task(void *context, int pending
                dn_drain_queue();
        }
 
-       DN_BH_WUNLOCK();
        dn_reschedule();
+       DN_BH_WUNLOCK();
        if (q.head != NULL)
                dummynet_send(q.head);
        CURVNET_RESTORE();

Modified: head/sys/netpfil/ipfw/ip_dummynet.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_dummynet.c Tue Dec 15 06:01:02 2015        
(r292253)
+++ head/sys/netpfil/ipfw/ip_dummynet.c Tue Dec 15 09:02:05 2015        
(r292254)
@@ -75,6 +75,7 @@ struct schk_new_arg {
 
 /*---- callout hooks. ----*/
 static struct callout dn_timeout;
+static int dn_gone;
 static struct task     dn_task;
 static struct taskqueue        *dn_tq = NULL;
 
@@ -90,6 +91,8 @@ void
 dn_reschedule(void)
 {
 
+       if (dn_gone != 0)
+               return;
        callout_reset_sbt(&dn_timeout, tick_sbt, 0, dummynet, NULL,
            C_HARDCLOCK | C_DIRECT_EXEC);
 }
@@ -2179,9 +2182,11 @@ ip_dn_init(void)
 static void
 ip_dn_destroy(int last)
 {
-       callout_drain(&dn_timeout);
-
        DN_BH_WLOCK();
+       /* ensure no more callouts are started */
+       dn_gone = 1;
+
+       /* check for last */
        if (last) {
                ND("removing last instance\n");
                ip_dn_ctl_ptr = NULL;
@@ -2190,6 +2195,8 @@ ip_dn_destroy(int last)
 
        dummynet_flush();
        DN_BH_WUNLOCK();
+
+       callout_drain(&dn_timeout);
        taskqueue_drain(dn_tq, &dn_task);
        taskqueue_free(dn_tq);
 

Modified: head/sys/netpfil/ipfw/ip_fw2.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw2.c      Tue Dec 15 06:01:02 2015        
(r292253)
+++ head/sys/netpfil/ipfw/ip_fw2.c      Tue Dec 15 09:02:05 2015        
(r292254)
@@ -2814,11 +2814,10 @@ vnet_ipfw_uninit(const void *unused)
 
        IPFW_UH_WLOCK(chain);
        IPFW_UH_WUNLOCK(chain);
-       IPFW_UH_WLOCK(chain);
 
-       IPFW_WLOCK(chain);
        ipfw_dyn_uninit(0);     /* run the callout_drain */
-       IPFW_WUNLOCK(chain);
+
+       IPFW_UH_WLOCK(chain);
 
        reap = NULL;
        IPFW_WLOCK(chain);
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to