On Tue, Nov 3, 2020 at 8:14 AM Sven Schnelle <svens@xxxxxxxxxxxxx> wrote: > Paul Moore <paul@xxxxxxxxxxxxxx> writes: > > > On Thu, Oct 8, 2020 at 9:37 PM <rentianyue@xxxxxxxxxxxxx> wrote: > >> From: Tianyue Ren <rentianyue@xxxxxxxxxx> > >> > >> Mark the inode security label as invalid if we cannot find > >> a dentry so that we will retry later rather than marking it > >> initialized with the unlabeled SID. > >> > >> Fixes: 9287aed2ad1f ("selinux: Convert isec->lock into a spinlock") > >> Signed-off-by: Tianyue Ren <rentianyue@xxxxxxxxxx> > >> --- > >> security/selinux/hooks.c | 19 ++++++++++++++++--- > >> 1 file changed, 16 insertions(+), 3 deletions(-) > > > > Merged into selinux/next with some minor tweaks to the comments. > > Thanks for your help! > > This seems to break booting on s390: > > Welcome to Fedora 32 (Thirty Two)! > > [ 1.434571] systemd[1]: Set hostname to <xxx.xxx> > [ 1.436839] audit: type=1400 audit(1604408868.681:4): avc: denied { write } for pid=1 comm="systemd" dev="cgroup2" ino=2 scontext=system_u:sys > tem_r:init_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=file permissive=0 > [ 1.436840] systemd[1]: Failed to create /init.scope control group: Permission denied > [ 1.438039] systemd[1]: Failed to allocate manager object: Permission denied > [ [0;1;31m!!!!!! [0m] Failed to allocate manager object. > [ 1.438281] systemd[1]: Freezing execution. > > Any ideas? If i revert 83370b31a915493231e5b9addc72e4bef69f8d31 from > linux-next-20201103 it works fine... Thanks for the report. Looking at this again, I'm thinking that setting the isec->initialized field outside of the spinlock is probably a bad idea. My guess is that your system is racing on inode_doinit_with_dentry() and the initialized field is getting messed up. Any chance you could try the attached (completely untested) patch? -- paul moore www.paul-moore.com
selinux: fix inode_doinit_with_dentry() error case locking From: Paul Moore <paul@xxxxxxxxxxxxxx> XXX - testing only patch, work in progress Fixes: 83370b31a915 ("selinux: fix error initialization in inode_doinit_with_dentry()") Reported-by: Sven Schnelle <svens@xxxxxxxxxxxxx> Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx> --- security/selinux/hooks.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 158fc47d8620..0294da2aaacd 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1451,13 +1451,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * inode_doinit with a dentry, before these inodes could * be used again by userspace. */ - isec->initialized = LABEL_INVALID; - /* - * There is nothing useful to jump to the "out" - * label, except a needless spin lock/unlock - * cycle. - */ - return 0; + goto out_invalid; } rc = inode_doinit_use_xattr(inode, dentry, sbsec->def_sid, @@ -1513,15 +1507,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * inode_doinit() with a dentry, before these inodes * could be used again by userspace. */ - if (!dentry) { - isec->initialized = LABEL_INVALID; - /* - * There is nothing useful to jump to the "out" - * label, except a needless spin lock/unlock - * cycle. - */ - return 0; - } + if (!dentry) + goto out_invalid; rc = selinux_genfs_get_sid(dentry, sclass, sbsec->flags, &sid); if (rc) { @@ -1546,11 +1533,10 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent out: spin_lock(&isec->lock); if (isec->initialized == LABEL_PENDING) { - if (!sid || rc) { + if (rc) { isec->initialized = LABEL_INVALID; goto out_unlock; } - isec->initialized = LABEL_INITIALIZED; isec->sid = sid; } @@ -1558,6 +1544,13 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent out_unlock: spin_unlock(&isec->lock); return rc; + +out_invalid: + spin_lock(&isec->lock); + if (isec->initialized == LABEL_PENDING) + isec->initialized = LABEL_INVALID; + spin_unlock(&isec->lock); + return 0; } /* Convert a Linux signal to an access vector. */