-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thu, 30 Oct 2014 18:20:01 +0100
Maxime Villard <[email protected]> wrote:

> 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 <[email protected]> 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?
> 

looks good to me.

lars
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlRYtzsACgkQcxuYqjT7GRZRogCfbFcVHO9Fcxb1uM7UY8MX0RuW
O5AAoNFDw5kQdfTHgatRWQksRO32wvxQ
=MX72
-----END PGP SIGNATURE-----

Reply via email to