On Mon, Jan 6, 2020 at 4:25 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> On 12/19/19 2:22 PM, Paul Moore wrote:
> > Deprecate the CONFIG_SECURITY_SELINUX_DISABLE functionality. The
> > code was originally developed to make it easier for Linux
> > distributions to support architectures where adding parameters to the
> > kernel command line was difficult. Unfortunately, supporting runtime
> > disable meant we had to make some security trade-offs when it came to
> > the LSM hooks, as documented in the Kconfig help text:
> >
> > NOTE: selecting this option will disable the '__ro_after_init'
> > kernel hardening feature for security hooks. Please consider
> > using the selinux=0 boot parameter instead of enabling this
> > option.
> >
> > Fortunately it looks as if that the original motivation for the
> > runtime disable functionality is gone, and Fedora/RHEL appears to be
> > the only major distribution enabling this capability at build time
> > so we are now taking steps to remove it entirely from the kernel.
> > The first step is to mark the functionality as deprecated and print
> > an error when it is used (what this patch is doing). As Fedora/RHEL
> > makes progress in transitioning the distribution away from runtime
> > disable, we will introduce follow-up patches over several kernel
> > releases which will block for increasing periods of time when the
> > runtime disable is used. Finally we will remove the option entirely
> > once we believe all users have moved to the kernel cmdline approach.
> >
> > Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> > ---
> > security/selinux/Kconfig | 3 +++
> > security/selinux/selinuxfs.c | 6 ++++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
> > index 996d35d950f7..580ac24c7aa1 100644
> > --- a/security/selinux/Kconfig
> > +++ b/security/selinux/Kconfig
> > @@ -42,6 +42,9 @@ config SECURITY_SELINUX_DISABLE
> > using the selinux=0 boot parameter instead of enabling this
> > option.
> >
> > + WARNING: this option is deprecated and will be removed in a future
> > + kernel release.
> > +
> > If you are unsure how to answer this question, answer N.
> >
> > config SECURITY_SELINUX_DEVELOP
> > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> > index 278417e67b4c..adbe2dd35202 100644
> > --- a/security/selinux/selinuxfs.c
> > +++ b/security/selinux/selinuxfs.c
> > @@ -281,6 +281,12 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
> > int new_value;
> > int enforcing;
> >
> > + /* NOTE: we are now officially considering runtime disable as
> > + * deprecated, and using it will become increasingly painful
> > + * (e.g. sleeping/blocking) as we progress through future
> > + * kernel releases until eventually it is removed */
> > + pr_err("SELinux: Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n");
>
> Looking for examples of similar deprecations in the kernel, I notice
> that they often use pr_warn_once() or WARN_ONCE() to avoid spamming the
> kernel logs excessively. They also often include the current process
> name to identify the offending process. In this case, it probably
> matters little since this is only done (legitimately) by the init
> process and only once, so up to you whether you bother amending it.
Yes, I saw that too and decided we were better off printing something
each time since it really should only ever happen once on a well
behaved system.
> Also for some interfaces, they appear to document the intent to remove
> it in a file under Documentation/ABI/obsolete/ and then later move that
> to removed/ when fully removed.
Thanks, I wasn't aware of that, and couldn't find anything relevant
while grep'ing under Documentation/process. There used to be a
Documentation/feature-removal.txt (or a file with a similar name)
which tracked these things, but I guess it migrated over to
Documentation/ABI during the last Documentation shuffle a couple of
years ago.
I'll send out an updated patch in just a moment.
--
paul moore
www.paul-moore.com
