linux-kernel - Re: [PATCH 4/5] selinux: declare data arrays const

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Feb 2022 11:13:26 -0500
From:   Paul Moore <paul@...l-moore.com>
To:     Christian Göttsche <cgzones@...glemail.com>
Cc:     selinux@...r.kernel.org,
        Stephen Smalley <stephen.smalley.work@...il.com>,
        Eric Paris <eparis@...isplace.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Ondrej Mosnacek <omosnace@...hat.com>,
        Jeremy Kerr <jk@...econstruct.com.au>,
        "David S. Miller" <davem@...emloft.net>,
        Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>,
        Yang Li <yang.lee@...ux.alibaba.com>,
        Austin Kim <austin.kim@....com>, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH 4/5] selinux: declare data arrays const

On Thu, Feb 17, 2022 at 9:21 AM Christian Göttsche
<cgzones@...glemail.com> wrote:
>
> The arrays for the policy capability names, the initial sid identifiers
> and the class and permission names are not changed at runtime.  Declare
> them const to avoid accidental modification.
>
> The build time script genheaders needs to be exempted, since it converts
> the entries to uppercase.
>
> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>
> ---
>  scripts/selinux/genheaders/genheaders.c          | 2 ++
>  scripts/selinux/mdp/mdp.c                        | 4 ++--
>  security/selinux/avc.c                           | 2 +-
>  security/selinux/include/avc_ss.h                | 2 +-
>  security/selinux/include/classmap.h              | 8 +++++++-
>  security/selinux/include/initial_sid_to_string.h | 9 ++++++++-
>  security/selinux/include/policycap.h             | 2 +-
>  security/selinux/include/policycap_names.h       | 2 +-
>  security/selinux/ss/services.c                   | 4 ++--
>  9 files changed, 25 insertions(+), 10 deletions(-)

...

> diff --git a/scripts/selinux/genheaders/genheaders.c b/scripts/selinux/genheaders/genheaders.c
> index f355b3e0e968..5f7c0b7d9260 100644
> --- a/scripts/selinux/genheaders/genheaders.c
> +++ b/scripts/selinux/genheaders/genheaders.c
> @@ -15,6 +15,8 @@ struct security_class_mapping {
>         const char *perms[sizeof(unsigned) * 8 + 1];
>  };
>
> +/* Allow to convert entries in mappings to uppercase */
> +#define __SELINUX_GENHEADERS__
>  #include "classmap.h"
>  #include "initial_sid_to_string.h"

...

> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 35aac62a662e..07ade4af85ff 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -2,6 +2,12 @@
>  #include <linux/capability.h>
>  #include <linux/socket.h>
>
> +#ifdef __SELINUX_GENHEADERS__
> +# define const_qual
> +#else
> +# define const_qual const
> +#endif
> +
>  #define COMMON_FILE_SOCK_PERMS "ioctl", "read", "write", "create", \
>      "getattr", "setattr", "lock", "relabelfrom", "relabelto", "append", "map"
>
> @@ -38,7 +44,7 @@
>   * Note: The name for any socket class should be suffixed by "socket",
>   *      and doesn't contain more than one substr of "socket".
>   */
> -struct security_class_mapping secclass_map[] = {
> +const_qual struct security_class_mapping secclass_map[] = {
>         { "security",
>           { "compute_av", "compute_create", "compute_member",
>             "check_context", "load_policy", "compute_relabel",

...

> diff --git a/security/selinux/include/initial_sid_to_string.h b/security/selinux/include/initial_sid_to_string.h
> index 5d332aeb8b6c..915283cd89bd 100644
> --- a/security/selinux/include/initial_sid_to_string.h
> +++ b/security/selinux/include/initial_sid_to_string.h
> @@ -1,5 +1,12 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> -static const char *initial_sid_to_string[] =
> +
> +#ifdef __SELINUX_GENHEADERS__
> +# define const_qual
> +#else
> +# define const_qual const
> +#endif
> +
> +static const char *const_qual initial_sid_to_string[] =
>  {
>         NULL,
>         "kernel",

Thanks for this Christian.  I generally like when we can const'ify
things like this, but I'm not excited about the const_qual hack on
core SELinux kernel code to satisfy genheaders.c.  I understand why it
is needed, but I would rather clutter the genheaders.c code than the
core SELinux kernel code.  If we can't cast away the const'ification
in genheaders.c could we simply allocate duplicate arrays in
genheaders.c and store the transformed strings into the new arrays?

-- 
paul-moore.com