[PATCH selinux-next] selinux: Annotate lockdep for services locks

[peter enderborg]

On 02/20/2018 04:58 PM, Stephen Smalley wrote:
> On Tue, 2018-02-20 at 08:59 -0500, Stephen Smalley wrote:
>> On Mon, 2018-02-19 at 16:18 +0100, Peter Enderborg wrote:
>>> From: Peter <peter.enderborg at sony.com>
>>>
>>> The locks are moved to dynamic allocation, we need to
>>> help the lockdep system to classify the locks.
>>> This adds to lockdep annotation for the page mutex and
>>> for the ss lock.
>>>
>>> Signed-off-by: Peter Enderborg <peter.enderborg at sony.com>
>>> ---
>>> This is the rebase of suggested patches from selinuxns tree
>>> and are intended to be applyed on top of:
>>> selinux: wrap global selinux state
>>> from Stephen Smalley
>>>
>>>  security/selinux/ss/services.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/security/selinux/ss/services.c
>>> b/security/selinux/ss/services.c
>>> index 3698352213d7..a741552e22b5 100644
>>> --- a/security/selinux/ss/services.c
>>> +++ b/security/selinux/ss/services.c
>>> @@ -81,11 +81,15 @@ char
>>> *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
>>>  };
>>>  
>>>  static struct selinux_ss selinux_ss;
>>> +static struct lock_class_key selinux_ss_class_key;
>>> +static struct lock_class_key selinux_status_class_key;
>>>  
>>>  void selinux_ss_init(struct selinux_ss **ss)
>>>  {
>>>  	rwlock_init(&selinux_ss.policy_rwlock);
>>> +	lockdep_set_class(&selinux_ss.policy_rwlock,
>>> &selinux_ss_class_key);
>>>  	mutex_init(&selinux_ss.status_lock);
>>> +	lockdep_set_class(&selinux_ss.status_lock,
>>> &selinux_status_class_key);
>>>  	*ss = &selinux_ss;
>>>  }
>> Pardon my ignorance, but can you explain why we need an explicit call
>> to lockdep_set_class() here?  I see it used for e.g. the inode
>> i_lock,
>> but there the class is per-file_system_type.  It doesn't seem to be
>> always be used for all locks when they are dynamically initialized or
>> allocated, e.g. get_empty_filp does not call lockdep_set_class() for
>> struct file's f_owner.lock or f_lock even though they are dynamically
>> allocated and initialized.  What makes this case different?
> Also, your explanation in the patch description was because the locks
> are moved to dynamic allocation.  That was true of the original selinux
> namespace patch.  But it isn't true for the wrap global selinux state
> patch; selinux_ss is statically allocated and there is only one
> instance of it in this patch.  So do we need this lockdep annotation
> yet?
>
>
I think you are right. I dont get any warnings whey trying to use them, and lockdep
get a useful name for them.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html