On Mon, Nov 14, 2022 at 12:45 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Mon, Nov 14, 2022 at 6:19 AM Konstantin Meskhidze > <konstantin.meskhidze@xxxxxxxxxx> wrote: > > From: Alexander Kozhevnikov <alexander.kozhevnikov@xxxxxxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@xxxxxxxxxx> > > --- > > 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
