From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Smalley Subject: Re: [PATCH] SELinux: Create a common helper to determine an inode label [ver #2] Date: Thu, 18 Jun 2015 13:31:38 -0400 Message-ID: <5583007A.8040704@tycho.nsa.gov> References: <1509.1434645664@warthog.procyon.org.uk> <5582FB01.9080602@tycho.nsa.gov> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5582FB01.9080602@tycho.nsa.gov> Sender: linux-security-module-owner@vger.kernel.org To: David Howells Cc: linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-unionfs@vger.kernel.org, linux-kernel@vger.kernel.org, SELinux , Paul Moore List-Id: linux-unionfs@vger.kernel.org On 06/18/2015 01:08 PM, Stephen Smalley wrote: > On 06/18/2015 12:41 PM, David Howells wrote: >> SELinux: Create a common helper to determine an inode label >> >> Create a common helper function to determine the label for a new inode. >> This is then used by: >> >> - may_create() >> - selinux_dentry_init_security() >> - selinux_inode_init_security() >> >> This will change the behaviour of the functions slightly, bringing them all >> into line. >> >> Suggested-by: Stephen Smalley >> Signed-off-by: David Howells >> --- >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index ffa5a642629a..902113d71acd 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -1684,6 +1684,32 @@ out: >> return rc; >> } >> >> +/* >> + * Determine the label for an inode that might be unioned. >> + */ >> +static int selinux_determine_inode_label(const struct inode *dir, >> + const struct qstr *name, >> + u16 tclass, >> + u32 *_new_isid) >> +{ >> + const struct superblock_security_struct *sbsec = dir->i_sb->s_security; >> + const struct inode_security_struct *dsec = dir->i_security; >> + const struct task_security_struct *tsec = current_security(); >> + >> + if ((sbsec->flags & SE_SBINITIALIZED) && >> + (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) { >> + *_new_isid = sbsec->mntpoint_sid; >> + } else if ((sbsec->flags & SBLABEL_MNT) && >> + tsec->create_sid) { >> + *_new_isid = tsec->create_sid; >> + } else { >> + return security_transition_sid(tsec->sid, dsec->sid, tclass, >> + name, _new_isid); >> + } >> + >> + return 0; >> +} >> + >> /* Check whether a task can create a file. */ >> static int may_create(struct inode *dir, >> struct dentry *dentry, >> @@ -1700,7 +1726,6 @@ static int may_create(struct inode *dir, >> sbsec = dir->i_sb->s_security; >> >> sid = tsec->sid; >> - newsid = tsec->create_sid; >> >> ad.type = LSM_AUDIT_DATA_DENTRY; >> ad.u.dentry = dentry; >> @@ -1711,12 +1736,10 @@ static int may_create(struct inode *dir, >> if (rc) >> return rc; >> >> - if (!newsid || !(sbsec->flags & SBLABEL_MNT)) { >> - rc = security_transition_sid(sid, dsec->sid, tclass, >> - &dentry->d_name, &newsid); >> - if (rc) >> - return rc; >> - } >> + rc = selinux_determine_inode_label(dir, &dentry->d_name, tclass, >> + &newsid); >> + if (rc) >> + return rc; >> >> rc = avc_has_perm(sid, newsid, tclass, FILE__CREATE, &ad); >> if (rc) >> @@ -2723,32 +2746,14 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode, >> struct qstr *name, void **ctx, >> u32 *ctxlen) >> { >> - const struct cred *cred = current_cred(); >> - struct task_security_struct *tsec; >> - struct inode_security_struct *dsec; >> - struct superblock_security_struct *sbsec; >> - struct inode *dir = d_backing_inode(dentry->d_parent); >> u32 newsid; >> int rc; >> >> - tsec = cred->security; >> - dsec = dir->i_security; >> - sbsec = dir->i_sb->s_security; >> - >> - if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) { >> - newsid = tsec->create_sid; >> - } else { >> - rc = security_transition_sid(tsec->sid, dsec->sid, >> - inode_mode_to_security_class(mode), >> - name, >> - &newsid); >> - if (rc) { >> - printk(KERN_WARNING >> - "%s: security_transition_sid failed, rc=%d\n", >> - __func__, -rc); >> - return rc; >> - } >> - } >> + rc = selinux_determine_inode_label(d_inode(dentry), name, > > We want d_inode(dentry->d_parent) here. Not sure why the caller didn't just pass in the dir inode in the first place. > >> + inode_mode_to_security_class(mode), >> + &newsid); >> + if (rc) >> + return rc; >> >> return security_sid_to_context(newsid, (char **)ctx, ctxlen); >> } >> @@ -2771,22 +2776,12 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, >> sid = tsec->sid; >> newsid = tsec->create_sid; >> >> - if ((sbsec->flags & SE_SBINITIALIZED) && >> - (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) >> - newsid = sbsec->mntpoint_sid; >> - else if (!newsid || !(sbsec->flags & SBLABEL_MNT)) { >> - rc = security_transition_sid(sid, dsec->sid, >> - inode_mode_to_security_class(inode->i_mode), >> - qstr, &newsid); >> - if (rc) { >> - printk(KERN_WARNING "%s: " >> - "security_transition_sid failed, rc=%d (dev=%s " >> - "ino=%ld)\n", >> - __func__, >> - -rc, inode->i_sb->s_id, inode->i_ino); >> - return rc; >> - } >> - } >> + rc = selinux_determine_inode_label( >> + dir, qstr, >> + inode_mode_to_security_class(inode->i_mode), >> + &newsid); >> + if (rc) >> + return rc; >> >> /* Possibly defer initialization to selinux_complete_init. */ >> if (sbsec->flags & SE_SBINITIALIZED) { >> >> > From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <5583007A.8040704@tycho.nsa.gov> Date: Thu, 18 Jun 2015 13:31:38 -0400 From: Stephen Smalley MIME-Version: 1.0 To: David Howells Subject: Re: [PATCH] SELinux: Create a common helper to determine an inode label [ver #2] References: <1509.1434645664@warthog.procyon.org.uk> <5582FB01.9080602@tycho.nsa.gov> In-Reply-To: <5582FB01.9080602@tycho.nsa.gov> Content-Type: text/plain; charset=windows-1252 Cc: linux-unionfs@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, SELinux , linux-fsdevel@vger.kernel.org List-Id: "Security-Enhanced Linux \(SELinux\) mailing list" List-Post: List-Help: On 06/18/2015 01:08 PM, Stephen Smalley wrote: > On 06/18/2015 12:41 PM, David Howells wrote: >> SELinux: Create a common helper to determine an inode label >> >> Create a common helper function to determine the label for a new inode. >> This is then used by: >> >> - may_create() >> - selinux_dentry_init_security() >> - selinux_inode_init_security() >> >> This will change the behaviour of the functions slightly, bringing them all >> into line. >> >> Suggested-by: Stephen Smalley >> Signed-off-by: David Howells >> --- >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index ffa5a642629a..902113d71acd 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -1684,6 +1684,32 @@ out: >> return rc; >> } >> >> +/* >> + * Determine the label for an inode that might be unioned. >> + */ >> +static int selinux_determine_inode_label(const struct inode *dir, >> + const struct qstr *name, >> + u16 tclass, >> + u32 *_new_isid) >> +{ >> + const struct superblock_security_struct *sbsec = dir->i_sb->s_security; >> + const struct inode_security_struct *dsec = dir->i_security; >> + const struct task_security_struct *tsec = current_security(); >> + >> + if ((sbsec->flags & SE_SBINITIALIZED) && >> + (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) { >> + *_new_isid = sbsec->mntpoint_sid; >> + } else if ((sbsec->flags & SBLABEL_MNT) && >> + tsec->create_sid) { >> + *_new_isid = tsec->create_sid; >> + } else { >> + return security_transition_sid(tsec->sid, dsec->sid, tclass, >> + name, _new_isid); >> + } >> + >> + return 0; >> +} >> + >> /* Check whether a task can create a file. */ >> static int may_create(struct inode *dir, >> struct dentry *dentry, >> @@ -1700,7 +1726,6 @@ static int may_create(struct inode *dir, >> sbsec = dir->i_sb->s_security; >> >> sid = tsec->sid; >> - newsid = tsec->create_sid; >> >> ad.type = LSM_AUDIT_DATA_DENTRY; >> ad.u.dentry = dentry; >> @@ -1711,12 +1736,10 @@ static int may_create(struct inode *dir, >> if (rc) >> return rc; >> >> - if (!newsid || !(sbsec->flags & SBLABEL_MNT)) { >> - rc = security_transition_sid(sid, dsec->sid, tclass, >> - &dentry->d_name, &newsid); >> - if (rc) >> - return rc; >> - } >> + rc = selinux_determine_inode_label(dir, &dentry->d_name, tclass, >> + &newsid); >> + if (rc) >> + return rc; >> >> rc = avc_has_perm(sid, newsid, tclass, FILE__CREATE, &ad); >> if (rc) >> @@ -2723,32 +2746,14 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode, >> struct qstr *name, void **ctx, >> u32 *ctxlen) >> { >> - const struct cred *cred = current_cred(); >> - struct task_security_struct *tsec; >> - struct inode_security_struct *dsec; >> - struct superblock_security_struct *sbsec; >> - struct inode *dir = d_backing_inode(dentry->d_parent); >> u32 newsid; >> int rc; >> >> - tsec = cred->security; >> - dsec = dir->i_security; >> - sbsec = dir->i_sb->s_security; >> - >> - if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) { >> - newsid = tsec->create_sid; >> - } else { >> - rc = security_transition_sid(tsec->sid, dsec->sid, >> - inode_mode_to_security_class(mode), >> - name, >> - &newsid); >> - if (rc) { >> - printk(KERN_WARNING >> - "%s: security_transition_sid failed, rc=%d\n", >> - __func__, -rc); >> - return rc; >> - } >> - } >> + rc = selinux_determine_inode_label(d_inode(dentry), name, > > We want d_inode(dentry->d_parent) here. Not sure why the caller didn't just pass in the dir inode in the first place. > >> + inode_mode_to_security_class(mode), >> + &newsid); >> + if (rc) >> + return rc; >> >> return security_sid_to_context(newsid, (char **)ctx, ctxlen); >> } >> @@ -2771,22 +2776,12 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, >> sid = tsec->sid; >> newsid = tsec->create_sid; >> >> - if ((sbsec->flags & SE_SBINITIALIZED) && >> - (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) >> - newsid = sbsec->mntpoint_sid; >> - else if (!newsid || !(sbsec->flags & SBLABEL_MNT)) { >> - rc = security_transition_sid(sid, dsec->sid, >> - inode_mode_to_security_class(inode->i_mode), >> - qstr, &newsid); >> - if (rc) { >> - printk(KERN_WARNING "%s: " >> - "security_transition_sid failed, rc=%d (dev=%s " >> - "ino=%ld)\n", >> - __func__, >> - -rc, inode->i_sb->s_id, inode->i_ino); >> - return rc; >> - } >> - } >> + rc = selinux_determine_inode_label( >> + dir, qstr, >> + inode_mode_to_security_class(inode->i_mode), >> + &newsid); >> + if (rc) >> + return rc; >> >> /* Possibly defer initialization to selinux_complete_init. */ >> if (sbsec->flags & SE_SBINITIALIZED) { >> >> >