security/selinux/hooks.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
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.0On 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
>
© 2016 - 2025 Red Hat, Inc.