All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: 任天悦 <rentianyue@tj.kylinos.cn>,
	"Eric Paris" <eparis@parisplace.org>,
	yangzhao <yangzhao@kylinos.cn>,
	"SElinux list" <selinux@vger.kernel.org>,
	"Tianyue Ren" <rentianyue@kylinos.cn>
Subject: Re: [PATCH v1 1/1] selinux: fix error initialization in inode_doinit_with_dentry()
Date: Thu, 1 Oct 2020 17:41:23 -0400	[thread overview]
Message-ID: <CAHC9VhTjrVtqbSAfpRSNYUSYx-mCur22bKvASHNY6fByOu+pFQ@mail.gmail.com> (raw)
In-Reply-To: <CAEjxPJ6z-ZsbsbdpDtOWxJ3_rJPjREKhNXkBn5bV_pYzW7AYnQ@mail.gmail.com>

On Tue, Sep 29, 2020 at 9:31 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Tue, Sep 29, 2020 at 8:54 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > On 9/27/20 5:42 AM, rentianyue@tj.kylinos.cn wrote:
> > > From: Tianyue Ren <rentianyue@kylinos.cn>
> > >
> > > 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.

Beyond the patch that has already been posted, I think the fix/clean
up is probably just to change the "!sid || rc" conditional in the
"out" jump target to simply "rc".  All of the code above that appears
to set "rc" correctly on error, which is really the only time (beyond
the posted patch) that we would need to set "isec->initizalized" to
"LABEL_INVALID".

-- 
paul moore
www.paul-moore.com

      parent reply	other threads:[~2020-10-01 21:41 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200927094243.43673-1-rentianyue@tj.kylinos.cn>
2020-09-28  3:23 ` [PATCH v1 0/1] selinux: fix error initialization in inode_doinit_with_dentry() Paul Moore
2020-09-28 13:41   ` Stephen Smalley
     [not found]     ` <tencent_489983C034412A8A6D8DF21D@qq.com>
2020-09-29 12:38       ` Stephen Smalley
     [not found] ` <20200927094243.43673-2-rentianyue@tj.kylinos.cn>
2020-09-29 12:54   ` [PATCH v1 1/1] " Stephen Smalley
2020-09-29 13:31     ` Stephen Smalley
2020-09-29 14:18       ` Stephen Smalley
2020-09-30  1:36         ` [PATCH v2 0/1] " rentianyue
2020-09-30  1:36           ` [PATCH v2 1/1] " rentianyue
2020-09-30 13:49             ` Stephen Smalley
2020-10-01 21:14               ` Paul Moore
2020-10-01 21:45             ` Paul Moore
2020-10-09  1:36               ` [PATCH v3 0/1] " rentianyue
2020-10-09  1:36                 ` [PATCH v3 1/1] selinux: " rentianyue
2020-10-28  2:17                   ` Paul Moore
2020-11-03 13:13                     ` Sven Schnelle
2020-11-03 17:11                       ` Paul Moore
2020-11-03 19:02                         ` Sven Schnelle
2020-11-04  2:42                           ` Paul Moore
2020-11-04  7:01                             ` Sven Schnelle
2020-11-04 20:40                               ` Paul Moore
2020-10-01 21:41       ` Paul Moore [this message]

Reply instructions:

You may reply publicly 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=CAHC9VhTjrVtqbSAfpRSNYUSYx-mCur22bKvASHNY6fByOu+pFQ@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=eparis@parisplace.org \
    --cc=rentianyue@kylinos.cn \
    --cc=rentianyue@tj.kylinos.cn \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=yangzhao@kylinos.cn \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.