Re: rwlock issue in secmodel

[Maxime Villard <max%M00nBSD.net@localhost>]

Le 27/10/2014 21:21, Lars Heidieker a écrit :
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On Mon, 27 Oct 2014 16:42:15 +0100
> Maxime Villard <max%M00nBSD.net@localhost> wrote:
> 
>> Hi,
>> I think there's a rwlock issue in secmodel/secmodel.c:
>>
>> 174	if (sm == NULL) {
>> 175		error = EFAULT;
>> 176		goto out;
>> 177	}
>> 178
>> 179	/* Check if the secmodel is already present. */
>> 180	rw_enter(&secmodels_lock, RW_WRITER);
>> ...
>>   out:
>> 194	/* Unlock the secmodels list. */
>> 195	rw_exit(&secmodels_lock);
>> 196
>> 197	return error;
>>
>> This goto out will release secmodels_lock while it is not yet held,
>> right?
>>
>> The same in secmodel_unplug().
>>
> 
> Yes, seems that way to me.
> secmodel_plug is only called from secmodel_register, where it would
> crash anyway if sm is NULL so it should be an assert, right?

yes, but in case it gets used in the future...

> (and for unplug the check should be in secmodel_deregister)
> 
> Lars
> 

Index: secmodel.c
===================================================================
RCS file: /cvsroot/src/sys/secmodel/secmodel.c,v
retrieving revision 1.1
diff -u -r1.1 secmodel.c
--- secmodel.c	4 Dec 2011 19:24:59 -0000	1.1
+++ secmodel.c	30 Oct 2014 17:17:36 -0000
@@ -171,10 +171,8 @@
 	secmodel_t tsm;
 	int error = 0;
 
-	if (sm == NULL) {
-		error = EFAULT;
-		goto out;
-	}
+	if (sm == NULL)
+		return EFAULT;
 
 	/* Check if the secmodel is already present. */
 	rw_enter(&secmodels_lock, RW_WRITER);
@@ -203,10 +201,8 @@
 	secmodel_t tsm;
 	int error = 0;
 
-	if (sm == NULL) {
-		error = EFAULT;
-		goto out;
-	}
+	if (sm == NULL)
+		return EFAULT;
 
 	/* Make sure the secmodel is present. */
 	rw_enter(&secmodels_lock, RW_WRITER);


Ok?