On Tue, Sep 29, 2020 at 8:54 AM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
>
> On 9/27/20 5:42 AM, rentianyue@xxxxxxxxxxxxx wrote:
>
> > From: Tianyue Ren <rentianyue@xxxxxxxxxx>
> >
> > Fix to initialize isec->class with SECINITSID_UNLABELED other
> > than the from the xattr label when then dentry is NULL when
> > the filesystem is remounted before the policy loading.
>
> Looks like this was broken by commit
> 9287aed2ad1ff1bde5eb190bcd6dccd5f1cf47d3 ("selinux: Convert isec->lock
> into a spinlock").
It appears that the broken commit assumed (wrongly) that isec->sid is
0 initially, sets sid = isec->sid, and then in the out: path, if (!sid
|| rc) it sets isec->initialized to LABEL_INVALID. In fact, isec->sid
is SECINITSID_UNLABELED initially upon selinux_inode_alloc_security(),
so that !sid test never evaluates to true. And changing it to compare
with SECINITSID_UNLABELED wouldn't be safe either since it is possible
to end up with SECINITSID_UNLABELED without it being invalid. I think
your fix resolves the issue with ensuring that we retry upon
subsequent attempts to access the inode but we should likely fix up
this code.
Acked-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
>
> >
> > Signed-off-by: Tianyue Ren <rentianyue@xxxxxxxxxx>
> > ---
> > security/selinux/hooks.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index bf8328adad8f..da7295a546e0 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1499,6 +1499,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;
> > goto out;
> > }
> >
> > @@ -1553,8 +1554,10 @@ 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)
> > + if (!dentry) {
> > + isec->initialized = LABEL_INVALID;
> > goto out;
> > + }
> > rc = selinux_genfs_get_sid(dentry, sclass,
> > sbsec->flags, &sid);
> > if (rc) {
