Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201028020615.8789-1-chenjun102@huawei.com> <c2eba2fb-79f8-eb48-ddd1-77fbc205ebff@schaufler-ca.com>
 <c9e1646e-f242-4d76-f482-f6281585860e@huawei.com> <70f2c67a-eb8a-5c28-9f78-838b397370b3@schaufler-ca.com>
In-Reply-To: <70f2c67a-eb8a-5c28-9f78-838b397370b3@schaufler-ca.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Fri, 30 Oct 2020 12:14:40 -0400
Message-ID: <CAHC9VhR_mCQ=amJbVRC_F=B+aCwRkjirYKQ+9ehktKy1Ji9Sng@mail.gmail.com>
Subject: Re: [RFC PATCH v2] selinux: Fix kmemleak after disabling selinux runtime
To:     Casey Schaufler <casey@schaufler-ca.com>
Cc:     Hou Tao <houtao1@huawei.com>, Chen Jun <chenjun102@huawei.com>,
        linux-kernel@vger.kernel.org, selinux@vger.kernel.org,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>, rui.xiang@huawei.com,
        guohanjun@huawei.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Oct 30, 2020 at 8:34 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/30/2020 12:57 AM, Hou Tao wrote:
> > Hi,
> >
> > On 2020/10/29 0:29, Casey Schaufler wrote:
> >> On 10/27/2020 7:06 PM, Chen Jun wrote:
> >>> From: Chen Jun <c00424029@huawei.com>
> >>>
> >>> Kmemleak will report a problem after using
> >>> "echo 1 > /sys/fs/selinux/disable" to disable selinux on runtime.
> >> Runtime disable of SELinux has been deprecated. It would be
> >> wasteful to make these changes in support of a facility that
> >> is going away.
> >>
> > But this sysfs file will still be present and workable on LTS kernel versions, so
> > is the proposed fixe OK for these LTS kernel versions ?
>
> It's not my call to make. Paul Moore has the voice that matters here.
> I think that the trivial memory leak here is inconsequential compared
> to the overhead you're introducing by leaving the NO_DEL hooks enabled.

Disabling SELinux at runtime is deprecated and will be removed in a
future release, check the
Documentation/ABI/obsolete/sysfs-selinux-disable in Linus' current
tree for details.  The recommended way to disable SELinux is at boot
using the kernel command line, as described in the deprecation text:

  The preferred method of disabling SELinux is via the "selinux=0" boot
  parameter, but the selinuxfs "disable" node was created to make it
  easier for systems with primitive bootloaders that did not allow for
  easy modification of the kernel command line.  Unfortunately, allowing
  for SELinux to be disabled at runtime makes it difficult to secure the
  kernel's LSM hooks using the "__ro_after_init" feature.

  Thankfully, the need for the SELinux runtime disable appears to be
  gone, the default Kconfig configuration disables this selinuxfs node,
  and only one of the major distributions, Fedora, supports disabling
  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.

Because of the upcoming removal as well as the drawbacks and minimal
gains provided by the patch in this thread, I would recommend against
merging this patch.  I would further recommend that distros and those
building their own kernels leave CONFIG_SECURITY_SELINUX_DISABLE
disabled and use the kernel command line instead.

NACK.

-- 
paul moore
www.paul-moore.com