Author: glebius
Date: Fri Sep 12 08:39:15 2014
New Revision: 271458
URL: http://svnweb.freebsd.org/changeset/base/271458

Log:
  - Provide a sleepable lock to protect against ioctl() vs ioctl() races.
  - Use the new lock to protect against simultaneous DIOCSTART and/or
    DIOCSTOP ioctls.
  
  Reported & tested by: jmallett
  Sponsored by:         Nginx, Inc.

Modified:
  head/sys/netpfil/pf/pf_ioctl.c

Modified: head/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- head/sys/netpfil/pf/pf_ioctl.c      Fri Sep 12 08:37:21 2014        
(r271457)
+++ head/sys/netpfil/pf/pf_ioctl.c      Fri Sep 12 08:39:15 2014        
(r271458)
@@ -191,6 +191,7 @@ static volatile VNET_DEFINE(int, pf_pfil
 VNET_DEFINE(int,               pf_end_threads);
 
 struct rwlock                  pf_rules_lock;
+struct sx                      pf_ioctl_lock;
 
 /* pfsync */
 pfsync_state_import_t          *pfsync_state_import_ptr = NULL;
@@ -1095,20 +1096,18 @@ pfioctl(struct cdev *dev, u_long cmd, ca
 
        switch (cmd) {
        case DIOCSTART:
-               PF_RULES_WLOCK();
+               sx_xlock(&pf_ioctl_lock);
                if (V_pf_status.running)
                        error = EEXIST;
                else {
                        int cpu;
 
-                       PF_RULES_WUNLOCK();
                        error = hook_pf();
                        if (error) {
                                DPFPRINTF(PF_DEBUG_MISC,
                                    ("pf: pfil registration failed\n"));
                                break;
                        }
-                       PF_RULES_WLOCK();
                        V_pf_status.running = 1;
                        V_pf_status.since = time_second;
 
@@ -1117,27 +1116,23 @@ pfioctl(struct cdev *dev, u_long cmd, ca
 
                        DPFPRINTF(PF_DEBUG_MISC, ("pf: started\n"));
                }
-               PF_RULES_WUNLOCK();
                break;
 
        case DIOCSTOP:
-               PF_RULES_WLOCK();
+               sx_xlock(&pf_ioctl_lock);
                if (!V_pf_status.running)
                        error = ENOENT;
                else {
                        V_pf_status.running = 0;
-                       PF_RULES_WUNLOCK();
                        error = dehook_pf();
                        if (error) {
                                V_pf_status.running = 1;
                                DPFPRINTF(PF_DEBUG_MISC,
                                    ("pf: pfil unregistration failed\n"));
                        }
-                       PF_RULES_WLOCK();
                        V_pf_status.since = time_second;
                        DPFPRINTF(PF_DEBUG_MISC, ("pf: stopped\n"));
                }
-               PF_RULES_WUNLOCK();
                break;
 
        case DIOCADDRULE: {
@@ -3261,6 +3256,8 @@ DIOCCHANGEADDR_error:
                break;
        }
 fail:
+       if (sx_xlocked(&pf_ioctl_lock))
+               sx_xunlock(&pf_ioctl_lock);
        CURVNET_RESTORE();
 
        return (error);
@@ -3734,6 +3731,7 @@ pf_load(void)
        VNET_LIST_RUNLOCK();
 
        rw_init(&pf_rules_lock, "pf rulesets");
+       sx_init(&pf_ioctl_lock, "pf ioctl");
 
        pf_dev = make_dev(&pf_cdevsw, 0, 0, 0, 0600, PF_NAME);
        if ((error = pfattach()) != 0)
@@ -3747,9 +3745,7 @@ pf_unload(void)
 {
        int error = 0;
 
-       PF_RULES_WLOCK();
        V_pf_status.running = 0;
-       PF_RULES_WUNLOCK();
        swi_remove(V_pf_swi_cookie);
        error = dehook_pf();
        if (error) {
@@ -3778,6 +3774,7 @@ pf_unload(void)
        PF_RULES_WUNLOCK();
        destroy_dev(pf_dev);
        rw_destroy(&pf_rules_lock);
+       sx_destroy(&pf_ioctl_lock);
 
        return (error);
 }
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to