Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: viro@zeniv.linux.org.uk, gregkh@linuxfoundation.org,
	rafael@kernel.org, jeyu@kernel.org, jmorris@namei.org,
	keescook@chromium.org, paul@paul-moore.com,
	stephen.smalley.work@gmail.com, eparis@parisplace.org,
	nayna@linux.ibm.com, zohar@linux.ibm.com,
	scott.branden@broadcom.com, dan.carpenter@oracle.com,
	skhan@linuxfoundation.org, geert@linux-m68k.org,
	tglx@linutronix.de, bauerman@linux.ibm.com, dhowells@redhat.com,
	linux-integrity@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	kexec@lists.infradead.org, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] security: add symbol namespace for reading file data
Date: Wed, 13 May 2020 10:40:31 -0500	[thread overview]
Message-ID: <87k11fonbk.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20200513152108.25669-3-mcgrof@kernel.org> (Luis Chamberlain's message of "Wed, 13 May 2020 15:21:07 +0000")

Luis Chamberlain <mcgrof@kernel.org> writes:

> Certain symbols are not meant to be used by everybody, the security
> helpers for reading files directly is one such case. Use a symbol
> namespace for them.
>
> This will prevent abuse of use of these symbols in places they were
> not inteded to be used, and provides an easy way to audit where these
> types of operations happen as a whole.

Why not just remove the ability for the firmware loader to be a module?

Is there some important use case that requires the firmware loader
to be a module?

We already compile the code in by default.  So it is probably just
easier to remove the modular support all together.  Which would allow
the export of the security hooks to be removed as well.

Eric


> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  drivers/base/firmware_loader/fallback.c | 1 +
>  fs/exec.c                               | 2 ++
>  kernel/kexec.c                          | 2 ++
>  kernel/module.c                         | 2 ++
>  security/security.c                     | 6 +++---
>  5 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index d9ac7296205e..b088886dafda 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -19,6 +19,7 @@
>   */
>  
>  MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);
> +MODULE_IMPORT_NS(SECURITY_READ);
>  
>  extern struct firmware_fallback_config fw_fallback_config;
>  
> diff --git a/fs/exec.c b/fs/exec.c
> index 9791b9eef9ce..30bd800ab1d6 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -72,6 +72,8 @@
>  
>  #include <trace/events/sched.h>
>  
> +MODULE_IMPORT_NS(SECURITY_READ);
> +
>  int suid_dumpable = 0;
>  
>  static LIST_HEAD(formats);
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index f977786fe498..8d572b41a157 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -19,6 +19,8 @@
>  
>  #include "kexec_internal.h"
>  
> +MODULE_IMPORT_NS(SECURITY_READ);
> +
>  static int copy_user_segment_list(struct kimage *image,
>  				  unsigned long nr_segments,
>  				  struct kexec_segment __user *segments)
> diff --git a/kernel/module.c b/kernel/module.c
> index 80faaf2116dd..8973a463712e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -59,6 +59,8 @@
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>  
> +MODULE_IMPORT_NS(SECURITY_READ);
> +
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/module.h>
>  
> diff --git a/security/security.c b/security/security.c
> index 8ae66e4c370f..bdbd1fc5105a 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1654,7 +1654,7 @@ int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
>  		return ret;
>  	return ima_read_file(file, id);
>  }
> -EXPORT_SYMBOL_GPL(security_kernel_read_file);
> +EXPORT_SYMBOL_NS_GPL(security_kernel_read_file, SECURITY_READ);
>  
>  int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
>  				   enum kernel_read_file_id id)
> @@ -1666,7 +1666,7 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
>  		return ret;
>  	return ima_post_read_file(file, buf, size, id);
>  }
> -EXPORT_SYMBOL_GPL(security_kernel_post_read_file);
> +EXPORT_SYMBOL_NS_GPL(security_kernel_post_read_file, SECURITY_READ);
>  
>  int security_kernel_load_data(enum kernel_load_data_id id)
>  {
> @@ -1677,7 +1677,7 @@ int security_kernel_load_data(enum kernel_load_data_id id)
>  		return ret;
>  	return ima_load_data(id);
>  }
> -EXPORT_SYMBOL_GPL(security_kernel_load_data);
> +EXPORT_SYMBOL_NS_GPL(security_kernel_load_data, SECURITY_READ);
>  
>  int security_task_fix_setuid(struct cred *new, const struct cred *old,
>  			     int flags)

  reply	other threads:[~2020-05-13 15:44 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 15:21 [PATCH 0/3] fs: reduce export usage of kerne_read*() calls Luis Chamberlain
2020-05-13 15:21 ` [PATCH 1/3] fs: unexport kernel_read_file() Luis Chamberlain
2020-05-13 15:21 ` [PATCH 2/3] security: add symbol namespace for reading file data Luis Chamberlain
2020-05-13 15:40   ` Eric W. Biederman [this message]
2020-05-13 16:09     ` Greg KH
2020-05-13 16:16     ` Luis Chamberlain
2020-05-13 16:26       ` Greg KH
2020-05-13 18:07       ` Josh Triplett
2020-05-13 15:21 ` [PATCH 3/3] fs: move kernel_read*() calls to its own symbol namespace Luis Chamberlain
2020-05-13 16:08   ` Greg KH
2020-05-13 18:17 ` [PATCH 0/3] fs: reduce export usage of kerne_read*() calls Christoph Hellwig
2020-05-15 21:29   ` Luis Chamberlain
2020-05-18  6:22     ` Christoph Hellwig
2020-05-18 12:37       ` Mimi Zohar
2020-05-18 15:21         ` Kees Cook
2020-07-29  1:20           ` Luis Chamberlain
2020-05-22 22:24         ` Scott Branden
2020-05-22 23:04           ` Kees Cook
2020-05-22 23:25             ` Scott Branden
2020-05-24  2:52               ` Mimi Zohar
2020-06-05 18:15                 ` Scott Branden
2020-06-05 18:37                   ` Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k11fonbk.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=bauerman@linux.ibm.com \
    --cc=dan.carpenter@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=eparis@parisplace.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jeyu@kernel.org \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=kexec@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=nayna@linux.ibm.com \
    --cc=paul@paul-moore.com \
    --cc=rafael@kernel.org \
    --cc=scott.branden@broadcom.com \
    --cc=selinux@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zohar@linux.ibm.com \
    --subject='Re: [PATCH 2/3] security: add symbol namespace for reading file data' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).