2023-03-23 05:13:46

by Lukas Bulwahn

[permalink] [raw]
Subject: [PATCH] selinux: clean up dead code after removing runtime disable

Commit f22f9aaf6c3d ("selinux: remove the runtime disable functionality")
removes the config SECURITY_SELINUX_DISABLE. This results in some dead code
in lsm_hooks.h and a reference in the ABI documentation leading nowhere as
the help text is simply gone.

Remove the dead code and dead reference.

Signed-off-by: Lukas Bulwahn <[email protected]>
---
Paul, please pick this minor cleanup patch on top of your commit above.

.../ABI/removed/sysfs-selinux-disable | 3 ---
include/linux/lsm_hooks.h | 23 -------------------
2 files changed, 26 deletions(-)

diff --git a/Documentation/ABI/removed/sysfs-selinux-disable b/Documentation/ABI/removed/sysfs-selinux-disable
index cb783c64cab3..1ae9587231e1 100644
--- a/Documentation/ABI/removed/sysfs-selinux-disable
+++ b/Documentation/ABI/removed/sysfs-selinux-disable
@@ -24,6 +24,3 @@ Description:
SELinux at runtime. Fedora is in the process of removing the
selinuxfs "disable" node and once that is complete we will start the
slow process of removing this code from the kernel.
-
- More information on /sys/fs/selinux/disable can be found under the
- CONFIG_SECURITY_SELINUX_DISABLE Kconfig option.
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 2b04f94a31bd..ab2b2fafa4a4 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -117,29 +117,6 @@ extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
__used __section(".early_lsm_info.init") \
__aligned(sizeof(unsigned long))

-#ifdef CONFIG_SECURITY_SELINUX_DISABLE
-/*
- * Assuring the safety of deleting a security module is up to
- * the security module involved. This may entail ordering the
- * module's hook list in a particular way, refusing to disable
- * the module once a policy is loaded or any number of other
- * actions better imagined than described.
- *
- * The name of the configuration option reflects the only module
- * that currently uses the mechanism. Any developer who thinks
- * disabling their module is a good idea needs to be at least as
- * careful as the SELinux team.
- */
-static inline void security_delete_hooks(struct security_hook_list *hooks,
- int count)
-{
- int i;
-
- for (i = 0; i < count; i++)
- hlist_del_rcu(&hooks[i].list);
-}
-#endif /* CONFIG_SECURITY_SELINUX_DISABLE */
-
extern int lsm_inode_alloc(struct inode *inode);

#endif /* ! __LINUX_LSM_HOOKS_H */
--
2.17.1


2023-03-23 15:07:26

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] selinux: clean up dead code after removing runtime disable

On Thu, Mar 23, 2023 at 1:12 AM Lukas Bulwahn <[email protected]> wrote:
>
> Commit f22f9aaf6c3d ("selinux: remove the runtime disable functionality")
> removes the config SECURITY_SELINUX_DISABLE. This results in some dead code
> in lsm_hooks.h and a reference in the ABI documentation leading nowhere as
> the help text is simply gone.
>
> Remove the dead code and dead reference.
>
> Signed-off-by: Lukas Bulwahn <[email protected]>
> ---
> Paul, please pick this minor cleanup patch on top of your commit above.

Hi Lukas, thanks for catching this and sending a patch! For future
reference, you don't need to add a note asking me to pick up this
patch, as long as you send it to the right mailing list - you did -
I'll see it and you'll either get a quick reply when I merge it or a
longer reply with comments/feedback.

One comment below ...

> diff --git a/Documentation/ABI/removed/sysfs-selinux-disable b/Documentation/ABI/removed/sysfs-selinux-disable
> index cb783c64cab3..1ae9587231e1 100644
> --- a/Documentation/ABI/removed/sysfs-selinux-disable
> +++ b/Documentation/ABI/removed/sysfs-selinux-disable
> @@ -24,6 +24,3 @@ Description:
> SELinux at runtime. Fedora is in the process of removing the
> selinuxfs "disable" node and once that is complete we will start the
> slow process of removing this code from the kernel.
> -
> - More information on /sys/fs/selinux/disable can be found under the
> - CONFIG_SECURITY_SELINUX_DISABLE Kconfig option.

When I moved the deprecation notice from the "obsolete" to the
"removed" directory I added a note at the top which read:

"REMOVAL UPDATE: The SELinux checkreqprot functionality was
removed in March 2023, the original deprecation notice is
shown below."

My goal was to preserve the original notice as much as possible,
including the references to the now defunct Kconfig option, to help
people who are trying to understand how things worked prior to the
removal.

If you can remove this part of your patch and resubmit I'll happily
merge it into the selinux/next tree.

Thanks!

--
paul-moore.com

2023-03-24 09:34:43

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] selinux: clean up dead code after removing runtime disable

On Thu, Mar 23, 2023 at 3:55 PM Paul Moore <[email protected]> wrote:
>
> On Thu, Mar 23, 2023 at 1:12 AM Lukas Bulwahn <[email protected]> wrote:
> >
> > Commit f22f9aaf6c3d ("selinux: remove the runtime disable functionality")
> > removes the config SECURITY_SELINUX_DISABLE. This results in some dead code
> > in lsm_hooks.h and a reference in the ABI documentation leading nowhere as
> > the help text is simply gone.
> >
> > Remove the dead code and dead reference.
> >
> > Signed-off-by: Lukas Bulwahn <[email protected]>
> > ---
> > Paul, please pick this minor cleanup patch on top of your commit above.
>
> Hi Lukas, thanks for catching this and sending a patch! For future
> reference, you don't need to add a note asking me to pick up this
> patch, as long as you send it to the right mailing list - you did -
> I'll see it and you'll either get a quick reply when I merge it or a
> longer reply with comments/feedback.
>
> One comment below ...
>
> > diff --git a/Documentation/ABI/removed/sysfs-selinux-disable b/Documentation/ABI/removed/sysfs-selinux-disable
> > index cb783c64cab3..1ae9587231e1 100644
> > --- a/Documentation/ABI/removed/sysfs-selinux-disable
> > +++ b/Documentation/ABI/removed/sysfs-selinux-disable
> > @@ -24,6 +24,3 @@ Description:
> > SELinux at runtime. Fedora is in the process of removing the
> > selinuxfs "disable" node and once that is complete we will start the
> > slow process of removing this code from the kernel.
> > -
> > - More information on /sys/fs/selinux/disable can be found under the
> > - CONFIG_SECURITY_SELINUX_DISABLE Kconfig option.
>
> When I moved the deprecation notice from the "obsolete" to the
> "removed" directory I added a note at the top which read:
>
> "REMOVAL UPDATE: The SELinux checkreqprot functionality was
> removed in March 2023, the original deprecation notice is
> shown below."
>
> My goal was to preserve the original notice as much as possible,
> including the references to the now defunct Kconfig option, to help
> people who are trying to understand how things worked prior to the
> removal.
>
> If you can remove this part of your patch and resubmit I'll happily
> merge it into the selinux/next tree.
>

Okay, I reworked the patch as requested and sent out a PATCH v2:

https://lore.kernel.org/all/[email protected]/T/#u

Thanks,

Lukas