Re: [PATCH] selinux: remove duplicated LABEL_INITIALIZED check routine

[Austin Kim <austindh.kim@gmail.com>]

2021년 6월 2일 (수) 오후 11:32, Paul Moore <paul@paul-moore.com>님이 작성:
>
> On Wed, Jun 2, 2021 at 1:48 AM Austin Kim <austindh.kim@gmail.com> wrote:
> >
> > The 'isec->initialized == LABEL_INITIALIZED' is checked twice in a row,
> > since selinux was mainlined from Linux-2.6.12-rc2.
> >
> > Since 'isec->initialized' is protected using spin_lock(&isec->lock)
> > within various APIs, it had better remove first exceptional routine.
> >
> > With this commit, the code look simpler, easier to read and maintain.
> >
> > Signed-off-by: Austin Kim <austindh.kim@gmail.com>
> > ---
> >  security/selinux/hooks.c | 3 ---
> >  1 file changed, 3 deletions(-)
>
> This is a common pattern when dealing with lock protected variables:
> first check the variable before taking the lock (fast path) and if
> necessary take the lock and re-check the variable when we know we have
> exclusive access.
>
> In the majority of cases the SELinux inode initialization value goes
> from LABEL_INVALID to LABEL_INITIALIZED and stays there; while there
> is an invalidation function/hook that is used by some
> network/distributed filesystems, it isn't a common case to the best of
> my knowledge.  With that understanding it makes perfect sense to do a
> quick check to first see if the inode is initialized in
> inode_doinit_with_dentry() and return quickly, without taking a lock,
> if it is already initialized.  In the case where the inode has not
> been previously initialized, or has been invalidated, we take the
> spinlock to guarantee we are not racing with another task and re-check
> the initialization value to ensure that another task hasn't
> initialized the inode and act accordingly.
>
> The existing code is correct.
>

Understood, after looking into all routines related to 'isec->initialized' again
where 'isec->initialized' statement is not always protected
spin_lock(&isec->lock) during initialization progress.

Thanks for valuable feedback.

> --
> paul moore
> www.paul-moore.com