Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 16 Feb 2010 00:05:53 +0000 (UTC)
From:      Lawrence Stewart <lstewart@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r203939 - projects/tcp_cc_head/sys/netinet
Message-ID:  <201002160005.o1G05rG9054401@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: lstewart
Date: Tue Feb 16 00:05:53 2010
New Revision: 203939
URL: http://svn.freebsd.org/changeset/base/203939

Log:
  Add a simple refcount to helpers to resolve the possibility of use-after-free on
  unload of a module that still had data blocks in active use. Something better
  should probably be devised eventually.

Modified:
  projects/tcp_cc_head/sys/netinet/helper.c
  projects/tcp_cc_head/sys/netinet/helper.h

Modified: projects/tcp_cc_head/sys/netinet/helper.c
==============================================================================
--- projects/tcp_cc_head/sys/netinet/helper.c	Mon Feb 15 23:44:48 2010	(r203938)
+++ projects/tcp_cc_head/sys/netinet/helper.c	Tue Feb 16 00:05:53 2010	(r203939)
@@ -37,6 +37,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/malloc.h>
 #include <sys/module.h>
 #include <sys/queue.h>
+#include <sys/refcount.h>
 #include <sys/rwlock.h>
 #include <sys/systm.h>
 
@@ -81,11 +82,12 @@ init_helper_dblocks(struct helper_dblock
 	    M_NOWAIT | M_ZERO);
 
 	if (hdbs->blocks != NULL) {
-		printf("Malloced ptr %p for %d data blocks\n", hdbs->blocks,
-		    num_dblocks);
+		/*printf("Malloced ptr %p for %d data blocks\n", hdbs->blocks,
+		    num_dblocks);*/
 		STAILQ_FOREACH(h, &helpers, h_next) {
 			if (h->h_flags & HELPER_NEEDS_DBLOCK) {
 				dblock = hdbs->blocks+i;
+				/*printf("Current dblock ptr: %p\n", dblock);*/
 				dblock->hd_block = uma_zalloc(h->h_zone,
 				    M_NOWAIT);
 				/*
@@ -96,9 +98,10 @@ init_helper_dblocks(struct helper_dblock
 				}
 				*/
 				dblock->hd_id = h->h_id;
-				printf("dblock[%d]: id=%d, block=%p\n", i,
-				dblock->hd_id, dblock->hd_block);
+				/*printf("dblock[%d]: id=%d, block=%p\n", i,
+				    dblock->hd_id, dblock->hd_block);*/
 				i++;
+				refcount_acquire(&h->h_refcount);
 			}
 		}
 		hdbs->nblocks = i;
@@ -118,8 +121,12 @@ destroy_helper_dblocks(struct helper_dbl
 	HELPER_LIST_WLOCK();
 
 	for (nblocks--; nblocks >= 0; nblocks--) {
-		if ((h = get_helper(hdbs->blocks[nblocks].hd_id)) != NULL)
+		if ((h = get_helper(hdbs->blocks[nblocks].hd_id)) != NULL) {
+			refcount_release(&h->h_refcount);
+			/*printf("destroy() freeing hdbs->blocks[%d] with ptr %p\n",
+			    nblocks, hdbs->blocks[nblocks].hd_block);*/
 			uma_zfree(h->h_zone, hdbs->blocks[nblocks].hd_block);
+		}
 	}
 
 	HELPER_LIST_WUNLOCK();
@@ -130,23 +137,22 @@ destroy_helper_dblocks(struct helper_dbl
 int
 register_helper(struct helper *h)
 {
-	printf("Register helper %p\n", h);
-
 	HELPER_LIST_WLOCK();
-
 	if (h->h_flags | HELPER_NEEDS_DBLOCK)
 		num_dblocks++;
 
+	refcount_init(&h->h_refcount, 0);
 	h->h_id = helper_id++;
 	STAILQ_INSERT_TAIL(&helpers, h, h_next);
 	HELPER_LIST_WUNLOCK();
+	printf("Registered \"%s\" helper (mem %p)\n", h->h_name, h);
 	return (0);
 }
 
 int
 deregister_helper(struct helper *h)
 {
-	printf("Deregister helper %p\n", h);
+	int error = 0;
 
 	/*
 	HHOOK_WLOCK
@@ -155,11 +161,17 @@ deregister_helper(struct helper *h)
 	*/
 
 	HELPER_LIST_WLOCK();
-	STAILQ_REMOVE(&helpers, h, helper, h_next);
-	if (h->h_flags | HELPER_NEEDS_DBLOCK)
-		num_dblocks--;
+	if (h->h_refcount > 0)
+		error = EBUSY;
+	
+	if (!error) {
+		STAILQ_REMOVE(&helpers, h, helper, h_next);
+		if (h->h_flags | HELPER_NEEDS_DBLOCK)
+			num_dblocks--;
+		printf("Deregistered \"%s\" helper (mem %p)\n", h->h_name, h);
+	}
 	HELPER_LIST_WUNLOCK();
-	return (0);
+	return (error);
 }
 
 int32_t
@@ -244,9 +256,12 @@ helper_modevent(module_t mod, int event_
 
 		case MOD_QUIESCE:
 			error = deregister_helper(hmd->helper);
-			uma_zdestroy(hmd->helper->h_zone);
-			if (!error && hmd->helper->mod_destroy != NULL)
-				hmd->helper->mod_destroy();
+			if (!error) {
+				uma_zdestroy(hmd->helper->h_zone);
+				if (hmd->helper->mod_destroy != NULL)
+					hmd->helper->mod_destroy();
+			} else
+				printf("Helper's refcount != 0, can't unload\n");
 			break;
 
 		case MOD_SHUTDOWN:
@@ -254,7 +269,7 @@ helper_modevent(module_t mod, int event_
 			break;
 
 		default:
-			return EINVAL;
+			error = EINVAL;
 			break;
 	}
 

Modified: projects/tcp_cc_head/sys/netinet/helper.h
==============================================================================
--- projects/tcp_cc_head/sys/netinet/helper.h	Mon Feb 15 23:44:48 2010	(r203938)
+++ projects/tcp_cc_head/sys/netinet/helper.h	Tue Feb 16 00:05:53 2010	(r203939)
@@ -54,6 +54,7 @@ struct helper {
 	uint16_t	h_flags;
 	uint32_t	h_class;
 	int32_t		h_id;
+	volatile uint32_t	h_refcount;
 	STAILQ_ENTRY(helper) h_next;
 };

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201002160005.o1G05rG9054401>