svn commit: r271458 - head/sys/netpfil/pf

[Gleb Smirnoff]

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);
 }