linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>, Jessica Yu <jeyu@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Scott Branden <scott.branden@broadcom.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Casey Schaufler <casey@schaufler-ca.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Matthew Garrett <matthewgarrett@google.com>,
	David Howells <dhowells@redhat.com>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	"Joel Fernandes (Google)" <joel@joelfernandes.org>,
	KP Singh <kpsingh@google.com>, Dave Olsthoorn <dave@bewaar.me>,
	Hans de Goede <hdegoede@redhat.com>,
	Peter Jones <pjones@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Stephen Boyd <stephen.boyd@linaro.org>,
	Paul Moore <paul@paul-moore.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH 4/4] module: Add hook for security_kernel_post_read_file()
Date: Tue, 7 Jul 2020 20:10:35 -0700	[thread overview]
Message-ID: <202007071951.605F38D43@keescook> (raw)
In-Reply-To: <1594169240.23056.143.camel@linux.ibm.com>

On Tue, Jul 07, 2020 at 08:47:20PM -0400, Mimi Zohar wrote:
> On Tue, 2020-07-07 at 01:19 -0700, Kees Cook wrote:
> > Calls to security_kernel_load_data() should be paired with a call to
> > security_kernel_post_read_file() with a NULL file argument. Add the
> > missing call so the module contents are visible to the LSMs interested
> > in measuring the module content. (This also paves the way for moving
> > module signature checking out of the module core and into an LSM.)
> > 
> > Cc: Jessica Yu <jeyu@kernel.org>
> > Fixes: c77b8cdf745d ("module: replace the existing LSM hook in init_module")
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  kernel/module.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 0c6573b98c36..af9679f8e5c6 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2980,7 +2980,12 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
> >  		return -EFAULT;
> >  	}
> >  
> > -	return 0;
> > +	err = security_kernel_post_read_file(NULL, (char *)info->hdr,
> > +					     info->len, READING_MODULE);
> 
> There was a lot of push back on calling security_kernel_read_file()
> with a NULL file descriptor here.[1]  The result was defining a new
> security hook - security_kernel_load_data - and enumeration -
> LOADING_MODULE.  I would prefer calling the same pre and post security
> hook.
> 
> Mimi
> 
> [1] http://kernsec.org/pipermail/linux-security-module-archive/2018-May/007110.html

Ah yes, thanks for the pointer to the discussion.

I think we have four cases then, for differing LSM hooks:

- no "file", no contents
	e.g. init_module() before copying user buffer
	security_kernel_load_data()
- only a "file" available, no contents
	e.g. kernel_read_file() before actually reading anything
	security_kernel_read_file()
- "file" and contents
	e.g. kernel_read_file() after reading
	security_kernel_post_read_file()
- no "file" available, just the contents
	e.g. firmware platform fallback from EFI space (no "file")
	unimplemented!

If an LSM wants to be able to examine the contents of firmware, modules,
kexec, etc, it needs either a "file" or the full contents.

The "file" methods all pass through the kernel_read_file()-family. The
others happen via blobs coming from userspace or (more recently) the EFI
universe.

So, if a NULL file is unreasonable, we need, perhaps,
security_kernel_post_load_data()

?

-- 
Kees Cook

  reply	other threads:[~2020-07-08  3:10 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07  8:19 [PATCH 0/4] Fix misused kernel_read_file() enums Kees Cook
2020-07-07  8:19 ` [PATCH 1/4] firmware_loader: EFI firmware loader must handle pre-allocated buffer Kees Cook
2020-07-07  8:19 ` [PATCH 2/4] fs: Remove FIRMWARE_PREALLOC_BUFFER from kernel_read_file() enums Kees Cook
2020-07-07 16:42   ` Scott Branden
2020-07-07 21:55     ` Kees Cook
2020-07-08  3:06       ` Scott Branden
2020-07-08  3:14         ` Kees Cook
2020-07-08  1:37   ` [fs] 676800b78f: BUG:unable_to_handle_page_fault_for_address kernel test robot
2020-07-10 21:00   ` [PATCH 2/4] fs: Remove FIRMWARE_PREALLOC_BUFFER from kernel_read_file() enums Scott Branden
2020-07-10 22:04     ` Matthew Wilcox
2020-07-10 22:10       ` Scott Branden
2020-07-10 22:44         ` Kees Cook
2020-07-10 22:58           ` Scott Branden
2020-07-16 20:35           ` Scott Branden
2020-07-16 21:16             ` Kees Cook
2020-07-07  8:19 ` [PATCH 3/4] fs: Remove FIRMWARE_EFI_EMBEDDED " Kees Cook
2020-07-07  8:19 ` [PATCH 4/4] module: Add hook for security_kernel_post_read_file() Kees Cook
2020-07-08  0:47   ` Mimi Zohar
2020-07-08  3:10     ` Kees Cook [this message]
2020-07-08 13:47       ` Mimi Zohar
2020-07-07  9:31 ` [PATCH 0/4] Fix misused kernel_read_file() enums Greg Kroah-Hartman
2020-07-07 15:36 ` Mimi Zohar
2020-07-07 21:45   ` Kees Cook
2020-07-08 11:01 ` Hans de Goede
2020-07-08 11:37   ` Hans de Goede
2020-07-08 11:55     ` Luis Chamberlain
2020-07-08 11:58       ` Hans de Goede
2020-07-08 13:30         ` Luis Chamberlain
2020-07-09  2:00           ` Kees Cook

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=202007071951.605F38D43@keescook \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=casey@schaufler-ca.com \
    --cc=dave@bewaar.me \
    --cc=dhowells@redhat.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=jeyu@kernel.org \
    --cc=jmorris@namei.org \
    --cc=joel@joelfernandes.org \
    --cc=kpsingh@google.com \
    --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=matthewgarrett@google.com \
    --cc=mcgrof@kernel.org \
    --cc=mchehab+huawei@kernel.org \
    --cc=paul@paul-moore.com \
    --cc=peterz@infradead.org \
    --cc=pjones@redhat.com \
    --cc=rafael@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=scott.branden@broadcom.com \
    --cc=serge@hallyn.com \
    --cc=stephen.boyd@linaro.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zohar@linux.ibm.com \
    --subject='Re: [PATCH 4/4] module: Add hook for security_kernel_post_read_file()' \
    /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).