[PATCH] [RFC] SELINUX: Remove obsolete deferred inode security init list.

[Paul Moore]

On Mon, Nov 14, 2022 at 12:45 PM Paul Moore <paul at paul-moore.com> wrote:
>
> On Mon, Nov 14, 2022 at 6:19 AM Konstantin Meskhidze
> <konstantin.meskhidze at huawei.com> wrote:
> > From: Alexander Kozhevnikov <alexander.kozhevnikov at huawei-partners.com>
> >
> > This patch is a proposed code optimization for SELinux:
> >
> > 1) Each inode has SELinux security structure attached
> >    to it, this one need to be initialized at some point.
> > 2) This initialization is done by the function
> >    inode_doinit_with_dentry ( ).
> > 3) In the kernel releases started from some point in the past
> >    this function (2) is always called normally from function
> >    __inode_security_revalidate ( ).
> > 4) Which in turn is always called  from inode_security ( ), which
> >    is a base point for any selinux calls and always called on
> >    any access to any inode except a few special cases when
> >    _inode_security_novalidate ( ) is used.
> > 5) Inode security structure initialization can be done only after
> >    SELinux is fully initialized and policy is loaded.
> > 6) So, for this purpose there was a special defeferred inode security
> >    initialization list protected by a spinlock implemented, which was
> >    populated instead of isec initialization in function
> >    inode_doinit_with_dentry ( ), if it was called before SELinux full
> >    initialization, and processed at the time when SELinux policy load
> >    occurred by calling again inode_doinit_with_dentry ( ) on each inode
> >    in this list.
> > 7) This list was a part of a default initialization logic before (3) was
> >    implemented, but now, taking into account new mechanism implemented
> >    with current approach of inode security revalidation on each access
> >    (4)-(3)-(2), it looks obsolete and not needed anymore.
> > 8) So deferred initialization, this list and code associated with it can
> >    be safely removed now, as anyway, if inode isec was not initialized
> >    before it will be processed on any next inode access.
> > 9) There are two possible positive consequences from this removal:
> >      a. More clean and simple code, less memory consumption;
> >      b. This deferred initialization in some cases (for example SELinux
> >         was switched on manually after system was up quite a long time)
> >         could take some significant time to process, i.e. system looks
> >         hung for some notable time. And now this is avoided.
> >
> > Signed-off-by: Alexander Kozhevnikov <alexander.kozhevnikov at huawei-partners.com>
> > Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze at huawei.com>
> > ---
> >  security/selinux/hooks.c          | 70 ++++---------------------------
> >  security/selinux/include/objsec.h |  3 --
> >  2 files changed, 7 insertions(+), 66 deletions(-)
>
> Hi Konstantin, Alexander,
>
> A few comments below, but can you share what testing you've done with
> this?  Specifically what you've done to ensure that inodes allocated
> before the policy is loaded are properly initialized/validated after
> the policy is loaded?

To be more specific, I'm curious about the cases where
__inode_security_revalidate() is called without the ability to sleep;
in those cases it is not possible to call inode_doinit_with_dentry()
to revalidate the inode's label.  With the current solution that is
not so much of an issue as sb_finish_set_opts() can block, but in your
proposed solution I worry this may be an issue.

--
paul-moore.com