Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: John Johansen <john.johansen@canonical.com>
To: Stephen Smalley <sds@tycho.nsa.gov>, paul@paul-moore.com
Cc: keescook@chromium.org, casey@schaufler-ca.com,
	selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
	jmorris@namei.org, dhowells@redhat.com
Subject: Re: [PATCH] selinux: fix residual uses of current_security() for the SELinux blob
Date: Wed, 4 Sep 2019 09:46:50 -0700
Message-ID: <412bc898-d142-081b-b86d-523348994ece@canonical.com> (raw)
In-Reply-To: <38c27c97-d525-9afb-3f2d-9d3190444ae2@tycho.nsa.gov>

On 9/4/19 8:31 AM, Stephen Smalley wrote:
> On 9/4/19 10:32 AM, Stephen Smalley wrote:
>> We need to use selinux_cred() to fetch the SELinux cred blob instead
>> of directly using current->security or current_security().  There
>> were a couple of lingering uses of current_security() in the SELinux code
>> that were apparently missed during the earlier conversions. IIUC, this
>> would only manifest as a bug if multiple security modules including
>> SELinux are enabled and SELinux is not first in the lsm order.
> 
> To further qualify that, it would only manifest as a bug if multiple non-exclusive security modules that use the cred security blob are enabled and SELinux is not first.  I don't think that is possible today in existing upstream kernels since the cred blob-using modules are currently all exclusive and tomoyo switched to using the task security blob instead.  Obviously that will change if/when the stacking for AA support lands and may already be an issue in Ubuntu depending on what stacking patchsets and what lsm order it is using.

Yes currently SELinux needs to be first in the stack for a few reasons. I haven't really played a lot with full SELinux and AA policy at the host level, instead focusing on running an Ubuntu container on an selinux based host. With SELinux and AA competing for userspace interfaces it currently only makes sense to specify SELinux first. I may have run into this bug when messing with the selinux namespacing patches to experiment with flipping the container order but there are enough other problems with that series that I can't say for sure.


> 
>> After
>> this change, there appear to be no other users of current_security()
>> in-tree; perhaps we should remove it altogether.
>>
>> Fixes: bbd3662a8348 ("Infrastructure management of the cred security blob")
>> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
>> ---
>>   security/selinux/hooks.c          |  2 +-
>>   security/selinux/include/objsec.h | 20 ++++++++++----------
>>   2 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index d55571c585ff..f1b763eceef9 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -3435,7 +3435,7 @@ static int selinux_inode_copy_up_xattr(const char *name)
>>   static int selinux_kernfs_init_security(struct kernfs_node *kn_dir,
>>                       struct kernfs_node *kn)
>>   {
>> -    const struct task_security_struct *tsec = current_security();
>> +    const struct task_security_struct *tsec = selinux_cred(current_cred());
>>       u32 parent_sid, newsid, clen;
>>       int rc;
>>       char *context;
>> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
>> index 231262d8eac9..d2e00c7595dd 100644
>> --- a/security/selinux/include/objsec.h
>> +++ b/security/selinux/include/objsec.h
>> @@ -40,16 +40,6 @@ struct task_security_struct {
>>       u32 sockcreate_sid;    /* fscreate SID */
>>   };
>>   -/*
>> - * get the subjective security ID of the current task
>> - */
>> -static inline u32 current_sid(void)
>> -{
>> -    const struct task_security_struct *tsec = current_security();
>> -
>> -    return tsec->sid;
>> -}
>> -
>>   enum label_initialized {
>>       LABEL_INVALID,        /* invalid or not initialized */
>>       LABEL_INITIALIZED,    /* initialized */
>> @@ -188,4 +178,14 @@ static inline struct ipc_security_struct *selinux_ipc(
>>       return ipc->security + selinux_blob_sizes.lbs_ipc;
>>   }
>>   +/*
>> + * get the subjective security ID of the current task
>> + */
>> +static inline u32 current_sid(void)
>> +{
>> +    const struct task_security_struct *tsec = selinux_cred(current_cred());
>> +
>> +    return tsec->sid;
>> +}
>> +
>>   #endif /* _SELINUX_OBJSEC_H_ */
>>
> 


  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-04 14:32 Stephen Smalley
2019-09-04 15:16 ` Casey Schaufler
2019-09-04 15:31 ` Stephen Smalley
2019-09-04 16:46   ` John Johansen [this message]
2019-09-04 19:35 ` James Morris
2019-09-04 22:50 ` Paul Moore
2019-09-05 20:01   ` Stephen Smalley
2019-09-05 20:55     ` James Morris

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=412bc898-d142-081b-b86d-523348994ece@canonical.com \
    --to=john.johansen@canonical.com \
    --cc=casey@schaufler-ca.com \
    --cc=dhowells@redhat.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git