kern/162174: [patch] rman_manage_region() error return path leaves mutex locked

[Ian Lepore]

>Number:         162174
>Category:       kern
>Synopsis:       [patch] rman_manage_region() error return path leaves mutex locked
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun Oct 30 19:50:11 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Ian Lepore <freebsd at damnhippie.dyndns.org>
>Release:        FreeBSD 8.2-STABLE
>Organization:
Symmetricom, Inc.
>Environment:
FreeBSD tflex 8.2-STABLE FreeBSD 8.2-STABLE #29: Tue Oct 11 13:32:35 UTC 2011     root at revolution.hippie.lan:/usr/obj/arm/usr/src/sys/TFLEX  arm

>Description:
If rman_manage_region() detects an overlapping region and returns EBUSY it
leaves the rman mutex locked, causing a panic on some future rman call.

>How-To-Repeat:

>Fix:
This patch was generated against 8.2-STABLE but applies cleanly to -current.
The error handling idiom in this module seems to be the "goto out" style so
I did this same way.

--- kern_subr.diff begins here ---
diff -r 96e180d3dc91 sys/kern/subr_rman.c
--- sys/kern/subr_rman.c.orig	Sat Oct 29 17:47:07 2011 -0600
+++ sys/kern/subr_rman.c	Sun Oct 30 13:23:14 2011 -0600
@@ -158,6 +158,7 @@ rman_init(struct rman *rm)
 int
 rman_manage_region(struct rman *rm, u_long start, u_long end)
 {
+	int rv;
 	struct resource_i *r, *s, *t;
 
 	DPRINTF(("rman_manage_region: <%s> request: start %#lx, end %#lx\n",
@@ -184,14 +185,16 @@ rman_manage_region(struct rman *rm, u_lo
 		TAILQ_INSERT_TAIL(&rm->rm_list, r, r_link);
 	} else {
 		/* Check for any overlap with the current region. */
-		if (r->r_start <= s->r_end && r->r_end >= s->r_start)
-			return EBUSY;
-
+		if (r->r_start <= s->r_end && r->r_end >= s->r_start) {
+			rv = EBUSY;
+			goto out;
+		}
 		/* Check for any overlap with the next region. */
 		t = TAILQ_NEXT(s, r_link);
-		if (t && r->r_start <= t->r_end && r->r_end >= t->r_start)
-			return EBUSY;
-
+		if (t && r->r_start <= t->r_end && r->r_end >= t->r_start) {
+			rv = EBUSY;
+			goto out;
+		}
 		/*
 		 * See if this region can be merged with the next region.  If
 		 * not, clear the pointer.
@@ -222,8 +225,10 @@ rman_manage_region(struct rman *rm, u_lo
 		}
 	}
 
+	rv = 0;
+out:
 	mtx_unlock(rm->rm_mtx);
-	return 0;
+	return (rv);
 }
 
 int
--- kern_subr.diff ends here ---

>Release-Note:
>Audit-Trail:
>Unformatted: