linux-kernel - Re: [PATCH 2/2] selinux: make __inode_security

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAEjxPJ6xCZLotyjfF-SBKbxFUur4=0bFbUpSZgbOkF_BMaAd4g@mail.gmail.com>
Date: Mon, 25 Aug 2025 08:47:12 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: Yugansh Mittal <mittalyugansh1@...il.com>
Cc: paul@...l-moore.com, omosnace@...hat.com, selinux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] selinux: make __inode_security_revalidate non-sleeping

On Sun, Aug 24, 2025 at 9:01 AM Yugansh Mittal <mittalyugansh1@...il.com> wrote:
>
> Replace the blocking revalidation logic in __inode_security_revalidate()
> with a fast, RCU-safe check of the inode security struct.
>
> Previously, the function could invoke inode_doinit_with_dentry() when
> may_sleep was true, which might block. With this change we always avoid
> sleeping and return -ECHILD if the inode label is invalid, forcing the
> caller to retry in a sleepable context.

If you look at the callers of __inode_security_revalidate(), you will
see that not all are capable of propagating -ECHILD to their callers
and forcing a retry; IIRC this is only truly possible during rcu path
walk. Hence, this change will produce situations where the inode may
be left with a stale or unlabeled context.
Your patch was marked as 2/2 but I did not see a 1/2 patch.

>
> This ensures that __inode_security_revalidate() can safely run in
> non-sleepable contexts while preserving correct retry semantics.
>
> Signed-off-by: Yugansh Mittal <mittalyugansh1@...il.com>
> ---
>  security/selinux/hooks.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874b..2bb94794e 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -282,19 +282,15 @@ static int __inode_security_revalidate(struct inode *inode,
>         if (!selinux_initialized())
>                 return 0;
>
> -       if (may_sleep)
> -               might_sleep();
> -       else
> -               return -ECHILD;
> -
> -       /*
> -        * Check to ensure that an inode's SELinux state is valid and try
> -        * reloading the inode security label if necessary.  This will fail if
> -        * @dentry is NULL and no dentry for this inode can be found; in that
> -        * case, continue using the old label.
> -        */
> -       inode_doinit_with_dentry(inode, dentry);
> -       return 0;
> +       rcu_read_lock();
> +        isec = selinux_inode(inode);
> +        if (unlikely(!isec || is_label_invalid(isec))) {
> +                rcu_read_unlock();
> +                return -ECHILD;  /* force caller to handle reload elsewhere */
> +        }
> +        rcu_read_unlock();
> +
> +       return 0; /* valid and no sleeping done */
>  }
>
>  static struct inode_security_struct *inode_security_novalidate(struct inode *inode)
> --
> 2.43.0
>