[PATCH 2/2] selinux: make __inode_security_revalidate non-sleeping

[PATCH 2/2] selinux: make __inode_security_revalidate non-sleeping

[Yugansh Mittal]

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.

This ensures that __inode_security_revalidate() can safely run in
non-sleepable contexts while preserving correct retry semantics.

Signed-off-by: Yugansh Mittal <mittalyugansh1@gmail.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

Re: [PATCH 2/2] selinux: make __inode_security_revalidate non-sleeping

[Stephen Smalley]

On Sun, Aug 24, 2025 at 9:01 AM Yugansh Mittal <mittalyugansh1@gmail.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@gmail.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
>

[PATCH v2] [V2] selinux: restore sleepable revalidation; keep fast no-sleep check

[Yugansh Mittal]

The prior change made __inode_security_revalidate() always return
-ECHILD when the inode label appears invalid, avoiding the potential
sleep in inode_doinit_with_dentry(). However, not all callers can
propagate -ECHILD; only RCU path walk reliably can. This caused
cases where the inode could be left with a stale/unlabeled context.

Fix by:
  * Keeping an RCU-safe, non-blocking validity check fast path.
  * Returning -ECHILD only when may_sleep == false.
  * When may_sleep == true, performing the blocking revalidation via
    inode_doinit_with_dentry() as before.

This preserves non-sleeping behavior in atomic/RCU contexts while
maintaining correct reload semantics elsewhere.

Signed-off-by: Yugansh Mittal <mittalyugansh1@gmail.com>
---
 security/selinux/hooks.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c95a5874b..170ae6d65 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -279,20 +279,34 @@ static int __inode_security_revalidate(struct inode *inode,
 				       struct dentry *dentry,
 				       bool may_sleep)
 {
+	struct inode_security_struct *isec;
+
 	if (!selinux_initialized())
 		return 0;
 
-	if (may_sleep)
-		might_sleep();
-	else
-		return -ECHILD;
+	/* Fast, non-blocking validity check first */
+	rcu_read_lock();
+	isec = selinux_inode(inode);
+	if (likely(isec && !is_label_invalid(isec))) {
+		rcu_read_unlock();
+		return 0;   /* valid and no sleeping done */
+	}
+	rcu_read_unlock();
 
 	/*
-	 * 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.
-	 */
+	* Label looks invalid. If we can't sleep, signal caller that a
+	* retry in a sleepable context is required. Only contexts like
+	* RCU path walk are expected to propagate -ECHILD.
+	*/
+	if (!may_sleep)
+	return -ECHILD;
+
+	/*
+	* Sleepable context: reload the label. This may block.
+	* If @dentry is NULL and no dentry can be found we'll continue
+	* using the old label, consistent with prior behavior.
+	*/
+	might_sleep();
 	inode_doinit_with_dentry(inode, dentry);
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v2] [V2] selinux: restore sleepable revalidation; keep fast no-sleep check

[Stephen Smalley]

On Tue, Aug 26, 2025 at 1:24 PM Yugansh Mittal <mittalyugansh1@gmail.com> wrote:
>
> The prior change made __inode_security_revalidate() always return

The prior change wasn't accepted so we wouldn't normally reference it
in a patch description,
just in a changelog that would go after the diffstat and not be
included into the commit message.

> -ECHILD when the inode label appears invalid, avoiding the potential
> sleep in inode_doinit_with_dentry(). However, not all callers can
> propagate -ECHILD; only RCU path walk reliably can. This caused
> cases where the inode could be left with a stale/unlabeled context.
>
> Fix by:

No need to fix that which is not broken.

>   * Keeping an RCU-safe, non-blocking validity check fast path.
>   * Returning -ECHILD only when may_sleep == false.
>   * When may_sleep == true, performing the blocking revalidation via
>     inode_doinit_with_dentry() as before.
>
> This preserves non-sleeping behavior in atomic/RCU contexts while
> maintaining correct reload semantics elsewhere.
>
> Signed-off-by: Yugansh Mittal <mittalyugansh1@gmail.com>

It is unclear what problem you are trying to solve with the current
code, but your current patch doesn't compile alone (seems to have a
dependency on some other patch you haven't posted anywhere I can see).
Also doesn't pass muster with the ./scripts/checkpatch.pl script.
See https://github.com/SELinuxProject/selinux-kernel/wiki/Getting-Started
for tips on how to get started with SELinux development and to prepare
patches that will be acceptable.

> ---
>  security/selinux/hooks.c | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874b..170ae6d65 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -279,20 +279,34 @@ static int __inode_security_revalidate(struct inode *inode,
>                                        struct dentry *dentry,
>                                        bool may_sleep)
>  {
> +       struct inode_security_struct *isec;
> +
>         if (!selinux_initialized())
>                 return 0;
>
> -       if (may_sleep)
> -               might_sleep();
> -       else
> -               return -ECHILD;
> +       /* Fast, non-blocking validity check first */
> +       rcu_read_lock();

Why do we need rcu_read_lock() here? We don't take it elsewhere that
we access isec.

> +       isec = selinux_inode(inode);
> +       if (likely(isec && !is_label_invalid(isec))) {

isec cannot be NULL for an inode when SELinux is enabled, so no need
to test for it.
is_label_invalid() is not defined in this patch and no other patch
from you appears to have been posted.

> +               rcu_read_unlock();
> +               return 0;   /* valid and no sleeping done */
> +       }
> +       rcu_read_unlock();
>
>         /*
> -        * 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.
> -        */
> +       * Label looks invalid. If we can't sleep, signal caller that a
> +       * retry in a sleepable context is required. Only contexts like
> +       * RCU path walk are expected to propagate -ECHILD.
> +       */
> +       if (!may_sleep)
> +       return -ECHILD;

Indentation problem, checkpatch.pl would have caught it.

> +
> +       /*
> +       * Sleepable context: reload the label. This may block.
> +       * If @dentry is NULL and no dentry can be found we'll continue
> +       * using the old label, consistent with prior behavior.
> +       */
> +       might_sleep();
>         inode_doinit_with_dentry(inode, dentry);
>         return 0;
>  }

What makes your patch better than the code that was in place before it?
Have you compared the resulting assembly and/or run any benchmarks?

> --
> 2.43.0
>

Re: [PATCH v2] [V2] selinux: restore sleepable revalidation; keep fast no-sleep check

[kernel test robot]

Hi Yugansh,

kernel test robot noticed the following build errors:

[auto build test ERROR on pcmoore-selinux/next]
[also build test ERROR on linus/master v7.2-rc7 next-20260814]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yugansh-Mittal/selinux-restore-sleepable-revalidation-keep-fast-no-sleep-check/20260815-164746
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link:    https://lore.kernel.org/r/20250826172330.44006-1-mittalyugansh1%40gmail.com
patch subject: [PATCH v2] [V2] selinux: restore sleepable revalidation; keep fast no-sleep check
config: alpha-allmodconfig (https://download.01.org/0day-ci/archive/20260815/202608152259.U1jKjeeX-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260815/202608152259.U1jKjeeX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202608152259.U1jKjeeX-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/build_bug.h:5,
                    from include/linux/init.h:5,
                    from security/selinux/hooks.c:24:
   security/selinux/hooks.c: In function '__inode_security_revalidate':
>> security/selinux/hooks.c:294:29: error: implicit declaration of function 'is_label_invalid' [-Wimplicit-function-declaration]
     294 |         if (likely(isec && !is_label_invalid(isec))) {
         |                             ^~~~~~~~~~~~~~~~
   include/linux/compiler.h:76:45: note: in definition of macro 'likely'
      76 | # define likely(x)      __builtin_expect(!!(x), 1)
         |                                             ^


vim +/is_label_invalid +294 security/selinux/hooks.c

   275	
   276	/*
   277	 * Try reloading inode security labels that have been marked as invalid.  The
   278	 * @may_sleep parameter indicates when sleeping and thus reloading labels is
   279	 * allowed; when set to false, returns -ECHILD when the label is
   280	 * invalid.  The @dentry parameter should be set to a dentry of the inode.
   281	 */
   282	static int __inode_security_revalidate(struct inode *inode,
   283					       struct dentry *dentry,
   284					       bool may_sleep)
   285	{
   286		struct inode_security_struct *isec;
   287	
   288		if (!selinux_initialized())
   289			return 0;
   290	
   291		/* Fast, non-blocking validity check first */
   292		rcu_read_lock();
   293		isec = selinux_inode(inode);
 > 294		if (likely(isec && !is_label_invalid(isec))) {
   295			rcu_read_unlock();
   296			return 0;   /* valid and no sleeping done */
   297		}
   298		rcu_read_unlock();
   299	
   300		/*
   301		* Label looks invalid. If we can't sleep, signal caller that a
   302		* retry in a sleepable context is required. Only contexts like
   303		* RCU path walk are expected to propagate -ECHILD.
   304		*/
   305		if (!may_sleep)
   306		return -ECHILD;
   307	
   308		/*
   309		* Sleepable context: reload the label. This may block.
   310		* If @dentry is NULL and no dentry can be found we'll continue
   311		* using the old label, consistent with prior behavior.
   312		*/
   313		might_sleep();
   314		inode_doinit_with_dentry(inode, dentry);
   315		return 0;
   316	}
   317	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki