diff mbox series

[3/3] selinux: log missing anonclass in debug configuration

Message ID 20230718184921.112786-3-cgzones@googlemail.com (mailing list archive)
State Rejected
Delegated to: Paul Moore
Headers show
Series [1/3] selinux: introduce SECURITY_SELINUX_DEBUG configuration | expand

Commit Message

Christian Göttsche July 18, 2023, 6:49 p.m. UTC
Log under the SELinux debug configuration when a caller to the LSM hook
inode_init_security_anon does not pass a anonymous inode class name.
The class name allows policy writers to transition the anonymous inode
into a private type via a name based type transition.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/hooks.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Paul Moore July 20, 2023, 8:21 p.m. UTC | #1
On Jul 18, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Log under the SELinux debug configuration when a caller to the LSM hook
> inode_init_security_anon does not pass a anonymous inode class name.
> The class name allows policy writers to transition the anonymous inode
> into a private type via a name based type transition.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/hooks.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Is this really a problem?  There are two callers in v6.5-rc2 and both
properly populate the @name parameter.

Considering how easy it is to look up the callers in the kernel source
and ensure they are passing a valid @name parameter I'm inclined to
leave this patch unmerged.

Thoughts?

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index b8a8a4f0f2ad..f6ffab9958b6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2918,6 +2918,17 @@ static int selinux_inode_init_security_anon(struct inode *inode,
>  	if (unlikely(!selinux_initialized()))
>  		return 0;
>  
> +#ifdef CONFIG_SECURITY_SELINUX_DEBUG
> +	/*
> +	 * Allow policy writers to transition the anonymous inode into
> +	 * a private type via a name based type transition.
> +	 */
> +	if (!name) {
> +		pr_debug("SELinux:  no class given for anonymous inode\n");
> +		dump_stack();
> +	}
> +#endif
> +
>  	isec = selinux_inode(inode);
>  
>  	/*
> -- 
> 2.40.1

--
paul-moore.com
Christian Göttsche July 28, 2023, 3:30 p.m. UTC | #2
On Thu, 20 Jul 2023 at 22:21, Paul Moore <paul@paul-moore.com> wrote:
>
> On Jul 18, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> >
> > Log under the SELinux debug configuration when a caller to the LSM hook
> > inode_init_security_anon does not pass a anonymous inode class name.
> > The class name allows policy writers to transition the anonymous inode
> > into a private type via a name based type transition.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  security/selinux/hooks.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
>
> Is this really a problem?  There are two callers in v6.5-rc2 and both
> properly populate the @name parameter.
>
> Considering how easy it is to look up the callers in the kernel source
> and ensure they are passing a valid @name parameter I'm inclined to
> leave this patch unmerged.
>
> Thoughts?

True, there are currently only two callers.  The intention was to be
notified if in the future new callers are added and author being
unaware of the effects for SELinux and the patch not noticed by the
SELinux maintainers.  Also normal users/distributions will never see
this message.

If you aren't convinced then feel free to disregard this patch.

>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index b8a8a4f0f2ad..f6ffab9958b6 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2918,6 +2918,17 @@ static int selinux_inode_init_security_anon(struct inode *inode,
> >       if (unlikely(!selinux_initialized()))
> >               return 0;
> >
> > +#ifdef CONFIG_SECURITY_SELINUX_DEBUG
> > +     /*
> > +      * Allow policy writers to transition the anonymous inode into
> > +      * a private type via a name based type transition.
> > +      */
> > +     if (!name) {
> > +             pr_debug("SELinux:  no class given for anonymous inode\n");
> > +             dump_stack();
> > +     }
> > +#endif
> > +
> >       isec = selinux_inode(inode);
> >
> >       /*
> > --
> > 2.40.1
>
> --
> paul-moore.com
diff mbox series

Patch

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b8a8a4f0f2ad..f6ffab9958b6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2918,6 +2918,17 @@  static int selinux_inode_init_security_anon(struct inode *inode,
 	if (unlikely(!selinux_initialized()))
 		return 0;
 
+#ifdef CONFIG_SECURITY_SELINUX_DEBUG
+	/*
+	 * Allow policy writers to transition the anonymous inode into
+	 * a private type via a name based type transition.
+	 */
+	if (!name) {
+		pr_debug("SELinux:  no class given for anonymous inode\n");
+		dump_stack();
+	}
+#endif
+
 	isec = selinux_inode(inode);
 
 	/*