Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH 2/2 v2] efi: print appropriate status message when loading certificates
From: Lee, Chun-Yi @ 2019-03-24  0:26 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi
In-Reply-To: <20190324002621.3551-1-jlee@suse.com>

When loading certificates list from UEFI variable, the original error
message direct shows the efi status code from UEFI firmware. It looks
ugly:

[    2.335031] Couldn't get size: 0x800000000000000e
[    2.335032] Couldn't get UEFI MokListRT
[    2.339985] Couldn't get size: 0x800000000000000e
[    2.339987] Couldn't get UEFI dbx list

So, this patch shows the status string instead of status code.

On the other hand, the "Couldn't get UEFI" message doesn't need
to be exposed when db/dbx/mok variable do not exist. So, this
patch set the message level to debug.

v2.
Setting the MODSIGN messagse level to debug.

Link: https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516
Cc: James Morris <jmorris@namei.org>
Cc: Serge E. Hallyn" <serge@hallyn.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 security/integrity/platform_certs/load_uefi.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 81b19c52832b..e65244b31f04 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -48,7 +48,9 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 
 	status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
 	if (status != EFI_BUFFER_TOO_SMALL) {
-		pr_err("Couldn't get size: 0x%lx\n", status);
+		if (status != EFI_NOT_FOUND)
+			pr_err("Couldn't get size: %s\n",
+				efi_status_to_str(status));
 		return NULL;
 	}
 
@@ -59,7 +61,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 	status = efi.get_variable(name, guid, NULL, &lsize, db);
 	if (status != EFI_SUCCESS) {
 		kfree(db);
-		pr_err("Error reading db var: 0x%lx\n", status);
+		pr_err("Error reading db var: %s\n",
+			efi_status_to_str(status));
 		return NULL;
 	}
 
@@ -155,7 +158,7 @@ static int __init load_uefi_certs(void)
 	if (!uefi_check_ignore_db()) {
 		db = get_cert_list(L"db", &secure_var, &dbsize);
 		if (!db) {
-			pr_err("MODSIGN: Couldn't get UEFI db list\n");
+			pr_debug("MODSIGN: Couldn't get UEFI db list\n");
 		} else {
 			rc = parse_efi_signature_list("UEFI:db",
 					db, dbsize, get_handler_for_db);
@@ -168,7 +171,7 @@ static int __init load_uefi_certs(void)
 
 	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
 	if (!mok) {
-		pr_info("Couldn't get UEFI MokListRT\n");
+		pr_debug("Couldn't get UEFI MokListRT\n");
 	} else {
 		rc = parse_efi_signature_list("UEFI:MokListRT",
 					      mok, moksize, get_handler_for_db);
@@ -179,7 +182,7 @@ static int __init load_uefi_certs(void)
 
 	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
 	if (!dbx) {
-		pr_info("Couldn't get UEFI dbx list\n");
+		pr_debug("Couldn't get UEFI dbx list\n");
 	} else {
 		rc = parse_efi_signature_list("UEFI:dbx",
 					      dbx, dbxsize,
-- 
2.16.4


^ permalink raw reply related

* [PATCH 1/2] efi: add a function for transferring status to string
From: Lee, Chun-Yi @ 2019-03-24  0:26 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck

This function can be used to transfer EFI status code to string
for printing out debug message. Using this function can improve
the readability of log.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 54357a258b35..a43cb0dc37af 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	default:
+		pr_warn("Unknown efi status: 0x%lx", status);
+	}
+
+	return "Unknown efi status";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.16.4


^ permalink raw reply related

* [PATCH 1/2] efi: add a function for transferring status to string
From: Lee, Chun-Yi @ 2019-03-23  8:37 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck

This function can be used to transfer EFI status code to string
for printing out debug message. Using this function can improve
the readability of log.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 54357a258b35..a43cb0dc37af 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	default:
+		pr_warn("Unknown efi status: 0x%lx", status);
+	}
+
+	return "Unknown efi status";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.16.4


^ permalink raw reply related

* Re: INFO: task hung in process_measurement
From: syzbot @ 2019-03-23  6:58 UTC (permalink / raw)
  To: airlied, dmitry.kasatkin, dri-devel, james.l.morris, jmorris,
	jsarha, linux-ima-devel, linux-ima-user, linux-integrity,
	linux-kernel, linux-security-module, serge, syzkaller-bugs,
	tomi.valkeinen, zohar
In-Reply-To: <00000000000033ebee0577262a98@google.com>

syzbot has bisected this bug to:

commit 8fe5616b20e5742bb5fee0e77dffe2fc76ac92a0
Author: Jyri Sarha <jsarha@ti.com>
Date:   Tue Jun 14 08:43:30 2016 +0000

     drm/tilcdc: Restore old dpms state in pm_resume()

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=10daff1b200000
start commit:   291d0e5d Merge tag 'for-linus-20180929' of git://git.kerne..
git tree:       upstream
final crash:    https://syzkaller.appspot.com/x/report.txt?x=12daff1b200000
console output: https://syzkaller.appspot.com/x/log.txt?x=14daff1b200000
kernel config:  https://syzkaller.appspot.com/x/.config?x=a8212f992609a887
dashboard link: https://syzkaller.appspot.com/bug?extid=cdc562bc26a2b2b0a94f
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=140e285e400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1237fe81400000

Reported-by: syzbot+cdc562bc26a2b2b0a94f@syzkaller.appspotmail.com
Fixes: 8fe5616b20e5 ("drm/tilcdc: Restore old dpms state in pm_resume()")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

^ permalink raw reply

* Re: [PATCH 2/2] efi: print appropriate status message when loading certificates
From: jlee @ 2019-03-23  2:56 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Lee, Chun-Yi, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar, linux-efi,
	linux-security-module, Linux Kernel Mailing List
In-Reply-To: <CAKv+Gu8Xg3SBTpAFDSe5zyT8yjU71+HUT9LO18D+HiT+Hhp+Zg@mail.gmail.com>

Hi Ard,

On Fri, Mar 22, 2019 at 03:27:46PM +0100, Ard Biesheuvel wrote:
> On Fri, 22 Mar 2019 at 11:34, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> >
> > When loading certificates list from UEFI variable, the original error
> > message direct shows the efi status code from UEFI firmware. It looks
> > ugly:
> >
> > [    2.335031] Couldn't get size: 0x800000000000000e
> > [    2.335032] Couldn't get UEFI MokListRT
> > [    2.339985] Couldn't get size: 0x800000000000000e
> > [    2.339987] Couldn't get UEFI dbx list
> >
> 
> Why is it an error in the first place if these EFI variables do not exist?
> 

As you said, the error message should only be exposed when
these EFI variables exist. I will change it in next version of
my patch.

Thanks a lot!
Joey Lee

> 
> > So, this patch shows the status string instead of status code.
> >
> > On the other hand, the error message of EFI_NOT_FOUND
> > (0x800000000000000e) doesn't need to be exposed because kernel
> > already prints "Couldn't get UEFI..." message. This patch also
> > filtered out it.
> >
> > Link: https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516
> > Cc: James Morris <jmorris@namei.org>
> > Cc: Serge E. Hallyn" <serge@hallyn.com>
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Nayna Jain <nayna@linux.ibm.com>
> > Cc: Josh Boyer <jwboyer@fedoraproject.org>
> > Cc: Mimi Zohar <zohar@linux.ibm.com>
> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> > ---
> >  security/integrity/platform_certs/load_uefi.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> > index 81b19c52832b..fe261166621f 100644
> > --- a/security/integrity/platform_certs/load_uefi.c
> > +++ b/security/integrity/platform_certs/load_uefi.c
> > @@ -48,7 +48,9 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
> >
> >         status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
> >         if (status != EFI_BUFFER_TOO_SMALL) {
> > -               pr_err("Couldn't get size: 0x%lx\n", status);
> > +               if (status != EFI_NOT_FOUND)
> > +                       pr_err("Couldn't get size: %s\n",
> > +                               efi_status_to_str(status));
> >                 return NULL;
> >         }
> >
> > @@ -59,7 +61,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
> >         status = efi.get_variable(name, guid, NULL, &lsize, db);
> >         if (status != EFI_SUCCESS) {
> >                 kfree(db);
> > -               pr_err("Error reading db var: 0x%lx\n", status);
> > +               pr_err("Error reading db var: %s\n",
> > +                       efi_status_to_str(status));
> >                 return NULL;
> >         }
> >
> > --
> > 2.16.4
> >

^ permalink raw reply

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Tetsuo Handa @ 2019-03-23  2:44 UTC (permalink / raw)
  To: Kees Cook
  Cc: Casey Schaufler, Jakub Kicinski, linux-security-module,
	Trond Myklebust, open list:NFS, SUNRPC, AND..., Anna Schumaker,
	LKML
In-Reply-To: <CAGXu5jKVQ=-mdMWsefuNDagwF3pQEiosb4niRx-gBKCmRK_h-g@mail.gmail.com>

On 2019/03/23 7:45, Kees Cook wrote:
> It breaks the backward-compat for the "security=" line. If a system is
> booted with CONFIG_LSM="minors...,apparmor" and "security=selinux",
> neither apparmor nor selinux will be initialized. The logic on
> "security=..." depends on the other LSMs being present in the list.

Really? The logic on "security=..." does not depend on LSM_FLAG_LEGACY_MAJOR
LSMs being present in the CONFIG_LSM= list, for ordered_lsm_parse() does

  (Step 1) Enable LSM_ORDER_FIRST module (i.e. capability).

  (Step 2) Disable LSM_FLAG_LEGACY_MAJOR modules which was not specified
           by "security=" parameter when "security=" parameter was specified.

  (Step 3) Enable modules specified by "lsm=" parameter (or CONFIG_LSM= settings
           if "lsm=" parameter was not specified).

  (Step 4) Enable up to one LSM_FLAG_LEGACY_MAJOR module which was specified
           by "security=" parameter when "security=" parameter was specified.

  (Step 5) Disable all unused modules.

and (Step 4) will compensate for lack of that module in (Step 3).

^ permalink raw reply

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Kees Cook @ 2019-03-22 22:45 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Casey Schaufler, Jakub Kicinski, linux-security-module,
	Trond Myklebust, open list:NFS, SUNRPC, AND..., Anna Schumaker,
	LKML
In-Reply-To: <e20bf21e-77b0-6403-51b6-3f174d45529e@i-love.sakura.ne.jp>

On Thu, Mar 21, 2019 at 2:10 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2019/03/22 1:38, Kees Cook wrote:
> > This is mostly good. I'd like to keep the other LSMs listed though
> > (similar to what I had originally) so that if a legacy-major doesn't
> > initialize, later ones will be. I want to remove the concept of
> > "major" LSMs. The only thing that should matter is init order...
>
> Excuse me? Are you saying that
>
>   if a legacy-major (which is defined as the "Default security module")
>   doesn't initialize, later ones (any of selinux,smack,tomoyo,apparmor
>   except the one which is defined as "Default security module") will be
>   initialized
>
> ? That sounds strange to me. Any of selinux,smack,tomoyo,apparmor can be
> initialized when specified by lsm= kernel command line option (or security=
> kernel command line option if lsm= kernel command line option is not
> specified), won't it?

It breaks the backward-compat for the "security=" line. If a system is
booted with CONFIG_LSM="minors...,apparmor" and "security=selinux",
neither apparmor nor selinux will be initialized. The logic on
"security=..." depends on the other LSMs being present in the list.

-Kees

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH] KEYS: trusted: defer execution of TPM-specific code until key instantiate
From: Dan Williams @ 2019-03-22 18:49 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Jarkko Sakkinen, David Howells, Mimi Zohar, linux-integrity,
	keyrings, linux-security-module, Linux Kernel Mailing List,
	linux-nvdimm, david.safford, James Bottomley, Silviu Vlasceanu,
	stable
In-Reply-To: <20190322180139.18856-1-roberto.sassu@huawei.com>

On Fri, Mar 22, 2019 at 11:04 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> Commit 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure from
> tpm_default_chip()") changed the tpm_chip argument of every TPM function
> from NULL to a pointer that is retrieved at module initialization time.
>
> Unlike before this patch, the trusted module cannot be loaded if no TPM is
> available. Unfortunately, this causes a dependency problem because the
> encrypted key type requires the 'key_type_trusted' symbol when
> CONFIG_TRUSTED_KEYS is defined.
>
> This patch fixes the issue by deferring the execution of TPM-specific code
> until a new trusted key is instantiated: init_tpm(), to obtain a tpm_chip
> pointer; init_digests(), introduced by commit 0b6cf6b97b7e ("tpm: pass an
> array of tpm_extend_digest structures to tpm_pcr_extend()"), to get random
> bytes from the TPM to lock a PCR.
>
> Cc: stable@vger.kernel.org
> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure from tpm_default_chip()")
> Reported-by: Dan Williams <dan.j.williams@intel.com>

Tested-by: Dan Williams <dan.j.williams@intel.com>

Thanks Robert!

^ permalink raw reply

* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Andy Lutomirski @ 2019-03-22 18:20 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Andy Lutomirski, X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
	Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
	Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
	Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
	David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <20190322114325.GA10165@linux.intel.com>

On Fri, Mar 22, 2019 at 4:43 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, Mar 22, 2019 at 01:29:38PM +0200, Jarkko Sakkinen wrote:
> > On Thu, Mar 21, 2019 at 09:50:41AM -0700, Andy Lutomirski wrote:
> > > On Sun, Mar 17, 2019 at 2:18 PM Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > >
> > > > In order to provide a mechanism for devilering provisoning rights:
> > > >
> > > > 1. Add a new file to the securityfs file called sgx/provision that works
> > > >    as a token for allowing an enclave to have the provisioning privileges.
> > > > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
> > > >    following data structure:
> > > >
> > > >    struct sgx_enclave_set_attribute {
> > > >            __u64 addr;
> > > >            __u64 token_fd;
> > > >    };
> > >
> > > Here's a potential issue:
> > >
> > > For container use, is it reasonable for a container manager to
> > > bind-mount a file into securityfs?  Or would something in /dev make
> > > this easier?
> >
> > I guess that is a valid point given that the securityfs contains the LSM
> > (e.g. SELinux or AppArmor) policy. So yeah, I think your are right what
> > you say.
> >
> > I propose that we create /dev/sgx/enclave to act as the enclave manager
> > and /dev/sgx/provision for provisioning. Is this sustainable for you?
>
> Hmm.. on 2nd thought the LSM policy or even DAC policy  would restrict
> that the container manager can only access specific files inside
> securityfs. With this conclusion I still think it is probably the best
> place for seurity policy like things even for SGX. It is meant for that
> anyway.
>

LSM or DAC policy can certainly *restrict* it, but I suspect that most
container runtimes don't mount securityfs at all.  OTOH, the runtime
definitely needs to have a way to pass /dev/sgx/enclave (or whatever
it's called) through, so using another device node will definitely
work.

^ permalink raw reply

* [PATCH] KEYS: trusted: defer execution of TPM-specific code until key instantiate
From: Roberto Sassu @ 2019-03-22 18:01 UTC (permalink / raw)
  To: jarkko.sakkinen, dhowells, zohar, dan.j.williams
  Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
	linux-nvdimm, david.safford, jejb, silviu.vlasceanu,
	Roberto Sassu, stable

Commit 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure from
tpm_default_chip()") changed the tpm_chip argument of every TPM function
from NULL to a pointer that is retrieved at module initialization time.

Unlike before this patch, the trusted module cannot be loaded if no TPM is
available. Unfortunately, this causes a dependency problem because the
encrypted key type requires the 'key_type_trusted' symbol when
CONFIG_TRUSTED_KEYS is defined.

This patch fixes the issue by deferring the execution of TPM-specific code
until a new trusted key is instantiated: init_tpm(), to obtain a tpm_chip
pointer; init_digests(), introduced by commit 0b6cf6b97b7e ("tpm: pass an
array of tpm_extend_digest structures to tpm_pcr_extend()"), to get random
bytes from the TPM to lock a PCR.

Cc: stable@vger.kernel.org
Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure from tpm_default_chip()")
Reported-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/keys/trusted.c | 89 +++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ecec672d3a77..c5162ca9c944 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -946,6 +946,44 @@ static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
 	return p;
 }
 
+static int init_tpm(void)
+{
+	if (chip)
+		return 0;
+
+	chip = tpm_default_chip();
+	if (!chip)
+		return -ENODEV;
+
+	return 0;
+}
+
+static int init_digests(void)
+{
+	u8 digest[TPM_MAX_DIGEST_SIZE];
+	int ret;
+	int i;
+
+	if (digests)
+		return 0;
+
+	ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
+	if (ret < 0)
+		return ret;
+	if (ret < TPM_MAX_DIGEST_SIZE)
+		return -EFAULT;
+
+	digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
+			  GFP_KERNEL);
+	if (!digests)
+		return -ENOMEM;
+
+	for (i = 0; i < chip->nr_allocated_banks; i++)
+		memcpy(digests[i].digest, digest, TPM_MAX_DIGEST_SIZE);
+
+	return 0;
+}
+
 /*
  * trusted_instantiate - create a new trusted key
  *
@@ -967,6 +1005,14 @@ static int trusted_instantiate(struct key *key,
 	size_t key_len;
 	int tpm2;
 
+	ret = init_tpm();
+	if (ret < 0)
+		return ret;
+
+	ret = init_digests();
+	if (ret < 0)
+		return ret;
+
 	tpm2 = tpm_is_tpm2(chip);
 	if (tpm2 < 0)
 		return tpm2;
@@ -1218,58 +1264,23 @@ static int __init trusted_shash_alloc(void)
 	return ret;
 }
 
-static int __init init_digests(void)
-{
-	u8 digest[TPM_MAX_DIGEST_SIZE];
-	int ret;
-	int i;
-
-	ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
-	if (ret < 0)
-		return ret;
-	if (ret < TPM_MAX_DIGEST_SIZE)
-		return -EFAULT;
-
-	digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
-			  GFP_KERNEL);
-	if (!digests)
-		return -ENOMEM;
-
-	for (i = 0; i < chip->nr_allocated_banks; i++)
-		memcpy(digests[i].digest, digest, TPM_MAX_DIGEST_SIZE);
-
-	return 0;
-}
-
 static int __init init_trusted(void)
 {
 	int ret;
 
-	chip = tpm_default_chip();
-	if (!chip)
-		return -ENOENT;
-	ret = init_digests();
-	if (ret < 0)
-		goto err_put;
 	ret = trusted_shash_alloc();
 	if (ret < 0)
-		goto err_free;
+		return ret;
 	ret = register_key_type(&key_type_trusted);
 	if (ret < 0)
-		goto err_release;
-	return 0;
-err_release:
-	trusted_shash_release();
-err_free:
-	kfree(digests);
-err_put:
-	put_device(&chip->dev);
+		trusted_shash_release();
 	return ret;
 }
 
 static void __exit cleanup_trusted(void)
 {
-	put_device(&chip->dev);
+	if (chip)
+		put_device(&chip->dev);
 	kfree(digests);
 	trusted_shash_release();
 	unregister_key_type(&key_type_trusted);
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH 2/2] efi: print appropriate status message when loading certificates
From: Ard Biesheuvel @ 2019-03-22 14:27 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: James Morris, Serge E . Hallyn, David Howells, Josh Boyer,
	Nayna Jain, Mimi Zohar, linux-efi, linux-security-module,
	Linux Kernel Mailing List, Lee, Chun-Yi
In-Reply-To: <20190322103350.27764-2-jlee@suse.com>

On Fri, 22 Mar 2019 at 11:34, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
>
> When loading certificates list from UEFI variable, the original error
> message direct shows the efi status code from UEFI firmware. It looks
> ugly:
>
> [    2.335031] Couldn't get size: 0x800000000000000e
> [    2.335032] Couldn't get UEFI MokListRT
> [    2.339985] Couldn't get size: 0x800000000000000e
> [    2.339987] Couldn't get UEFI dbx list
>

Why is it an error in the first place if these EFI variables do not exist?


> So, this patch shows the status string instead of status code.
>
> On the other hand, the error message of EFI_NOT_FOUND
> (0x800000000000000e) doesn't need to be exposed because kernel
> already prints "Couldn't get UEFI..." message. This patch also
> filtered out it.
>
> Link: https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516
> Cc: James Morris <jmorris@namei.org>
> Cc: Serge E. Hallyn" <serge@hallyn.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Nayna Jain <nayna@linux.ibm.com>
> Cc: Josh Boyer <jwboyer@fedoraproject.org>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> ---
>  security/integrity/platform_certs/load_uefi.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index 81b19c52832b..fe261166621f 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -48,7 +48,9 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>
>         status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
>         if (status != EFI_BUFFER_TOO_SMALL) {
> -               pr_err("Couldn't get size: 0x%lx\n", status);
> +               if (status != EFI_NOT_FOUND)
> +                       pr_err("Couldn't get size: %s\n",
> +                               efi_status_to_str(status));
>                 return NULL;
>         }
>
> @@ -59,7 +61,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>         status = efi.get_variable(name, guid, NULL, &lsize, db);
>         if (status != EFI_SUCCESS) {
>                 kfree(db);
> -               pr_err("Error reading db var: 0x%lx\n", status);
> +               pr_err("Error reading db var: %s\n",
> +                       efi_status_to_str(status));
>                 return NULL;
>         }
>
> --
> 2.16.4
>

^ permalink raw reply

* Re: [PATCH 11/43] convert do_remount_sb() to fs_context
From: Andreas Schwab @ 2019-03-22 14:00 UTC (permalink / raw)
  To: David Howells
  Cc: viro, linux-fsdevel, torvalds, ebiederm, linux-security-module
In-Reply-To: <8736nfyr60.fsf@igel.home>

On Mär 22 2019, Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Mär 22 2019, David Howells <dhowells@redhat.com> wrote:
>
>> Andreas Schwab <schwab@linux-m68k.org> wrote:
>>
>>> That breaks booting for me.  The problem appears to be that udev fails
>>> to create the block devices.
>>
>> Can you give some more information?  Is this on m68k?  Are there any messages
>> logged?
>
> It is on ppc and ppc64, and I now see that devtmpfs has failed to mount.

# mount -t devtmpfs -o mode=0755,nr_inodes=0 devtmpfs /dev
mount: wrong fs type, bad option, bad superblock on devtmpfs,
       missing codepage or helper program, or other error

       In some cases useful info is found in syslog - try
       dmesg | tail or so.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH 11/43] convert do_remount_sb() to fs_context
From: Andreas Schwab @ 2019-03-22 13:28 UTC (permalink / raw)
  To: David Howells
  Cc: viro, linux-fsdevel, torvalds, ebiederm, linux-security-module
In-Reply-To: <21016.1553253953@warthog.procyon.org.uk>

On Mär 22 2019, David Howells <dhowells@redhat.com> wrote:

> Andreas Schwab <schwab@linux-m68k.org> wrote:
>
>> That breaks booting for me.  The problem appears to be that udev fails
>> to create the block devices.
>
> Can you give some more information?  Is this on m68k?  Are there any messages
> logged?

It is on ppc and ppc64, and I now see that devtmpfs has failed to mount.
But I can mount it manually with mount, whereas the error occurs when
systemd tries to mount it, I think.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH 11/43] convert do_remount_sb() to fs_context
From: David Howells @ 2019-03-22 11:25 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: dhowells, viro, linux-fsdevel, torvalds, ebiederm,
	linux-security-module
In-Reply-To: <87a7hnyx4x.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> wrote:

> That breaks booting for me.  The problem appears to be that udev fails
> to create the block devices.

Can you give some more information?  Is this on m68k?  Are there any messages
logged?

David

^ permalink raw reply

* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-03-22 11:29 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
	Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
	Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
	Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
	David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <CALCETrXQ0x2TD5GnaHhmF6SExk0C8+L0AwfnH-Krqwttfvwz2A@mail.gmail.com>

On Thu, Mar 21, 2019 at 09:50:41AM -0700, Andy Lutomirski wrote:
> On Sun, Mar 17, 2019 at 2:18 PM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > In order to provide a mechanism for devilering provisoning rights:
> >
> > 1. Add a new file to the securityfs file called sgx/provision that works
> >    as a token for allowing an enclave to have the provisioning privileges.
> > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
> >    following data structure:
> >
> >    struct sgx_enclave_set_attribute {
> >            __u64 addr;
> >            __u64 token_fd;
> >    };
> 
> Here's a potential issue:
> 
> For container use, is it reasonable for a container manager to
> bind-mount a file into securityfs?  Or would something in /dev make
> this easier?

I guess that is a valid point given that the securityfs contains the LSM
(e.g. SELinux or AppArmor) policy. So yeah, I think your are right what
you say.

I propose that we create /dev/sgx/enclave to act as the enclave manager
and /dev/sgx/provision for provisioning. Is this sustainable for you?

/Jarkko

^ permalink raw reply

* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-03-22 11:31 UTC (permalink / raw)
  To: Huang, Kai
  Cc: Christopherson, Sean J, Svahn, Kai, nhorman@redhat.com,
	jmorris@namei.org, rientjes@google.com, josh@joshtriplett.org,
	tglx@linutronix.de, Ayoun, Serge, Huang, Haitao,
	linux-security-module@vger.kernel.org, x86@kernel.org,
	akpm@linux-foundation.org, npmccallum@redhat.com,
	linux-sgx@vger.kernel.org, luto@kernel.org, Katz-zamir, Shay,
	Hansen, Dave, bp@alien8.de, serge@hallyn.com,
	andriy.shevchenko@linux.intel.com
In-Reply-To: <105F7BF4D0229846AF094488D65A0989356E6613@PGSMSX112.gar.corp.intel.com>

On Thu, Mar 21, 2019 at 09:41:37PM +0000, Huang, Kai wrote:
> > On Thu, Mar 21, 2019 at 02:08:36AM +0000, Huang, Kai wrote:
> > > On Tue, 2019-03-19 at 13:09 -0700, Sean Christopherson wrote:
> > > > On Sun, Mar 17, 2019 at 11:14:46PM +0200, Jarkko Sakkinen wrote:
> > > > > In order to provide a mechanism for devilering provisoning rights:
> > > > >
> > > > > 1. Add a new file to the securityfs file called sgx/provision that works
> > > > >    as a token for allowing an enclave to have the provisioning privileges.
> > > > > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that
> > accepts the
> > > > >    following data structure:
> > > > >
> > > > >    struct sgx_enclave_set_attribute {
> > > > >            __u64 addr;
> > > > >            __u64 token_fd;
> > > > >    };
> > >
> > > Would you elaborate why the name is "token_fd"? I think *token* in SGX
> > > has more specific meaning?
> > 
> > I'm open for other names.
> 
> How about attribute_fd, which is consistent with your IOCTL?

I'm cool with that name! Thanks.

/Jarkko

^ permalink raw reply

* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-03-22 11:43 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
	Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
	Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
	Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
	David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <20190322112938.GJ3122@linux.intel.com>

On Fri, Mar 22, 2019 at 01:29:38PM +0200, Jarkko Sakkinen wrote:
> On Thu, Mar 21, 2019 at 09:50:41AM -0700, Andy Lutomirski wrote:
> > On Sun, Mar 17, 2019 at 2:18 PM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > In order to provide a mechanism for devilering provisoning rights:
> > >
> > > 1. Add a new file to the securityfs file called sgx/provision that works
> > >    as a token for allowing an enclave to have the provisioning privileges.
> > > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
> > >    following data structure:
> > >
> > >    struct sgx_enclave_set_attribute {
> > >            __u64 addr;
> > >            __u64 token_fd;
> > >    };
> > 
> > Here's a potential issue:
> > 
> > For container use, is it reasonable for a container manager to
> > bind-mount a file into securityfs?  Or would something in /dev make
> > this easier?
> 
> I guess that is a valid point given that the securityfs contains the LSM
> (e.g. SELinux or AppArmor) policy. So yeah, I think your are right what
> you say.
> 
> I propose that we create /dev/sgx/enclave to act as the enclave manager
> and /dev/sgx/provision for provisioning. Is this sustainable for you?

Hmm.. on 2nd thought the LSM policy or even DAC policy  would restrict
that the container manager can only access specific files inside
securityfs. With this conclusion I still think it is probably the best
place for seurity policy like things even for SGX. It is meant for that
anyway.

/Jarkko

^ permalink raw reply

* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-03-22 11:22 UTC (permalink / raw)
  To: Nathaniel McCallum
  Cc: x86, linux-sgx, akpm, dave.hansen, sean.j.christopherson,
	Neil Horman, Ayoun, Serge, shay.katz-zamir, Huang, Haitao,
	andriy.shevchenko, tglx, Svahn, Kai, bp, josh, luto, kai.huang,
	rientjes, James Morris, Serge E . Hallyn, linux-security-module
In-Reply-To: <CAOASepNN=05=UTcxU13+1NJEJchDE8NjncJL9ObBzzgJxJaD9w@mail.gmail.com>

On Thu, Mar 21, 2019 at 10:38:32AM -0400, Nathaniel McCallum wrote:
> On Sun, Mar 17, 2019 at 5:18 PM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > In order to provide a mechanism for devilering provisoning rights:
>
> I'm not sure what nefarious spirits have to do with this patch.
> Perhaps you meant "delivering provisioning"? ;)

Oops, yeah, maybe summoning won't this time :-)

/Jarkko

^ permalink raw reply

* Re: [PATCH 11/43] convert do_remount_sb() to fs_context
From: Andreas Schwab @ 2019-03-22 11:19 UTC (permalink / raw)
  To: David Howells
  Cc: viro, linux-fsdevel, torvalds, ebiederm, linux-security-module
In-Reply-To: <155059379404.12449.7151387243186063769.stgit@warthog.procyon.org.uk>

On Feb 19 2019, David Howells <dhowells@redhat.com> wrote:

> Replace do_remount_sb() with a function, reconfigure_super(), that's
> fs_context aware.  The fs_context is expected to be parameterised already
> and have ->root pointing to the superblock to be reconfigured.
>
> A legacy wrapper is provided that is intended to be called from the
> fs_context ops when those appear, but for now is called directly from
> reconfigure_super().  This wrapper invokes the ->remount_fs() superblock op
> for the moment.  It is intended that the remount_fs() op will be phased
> out.
>
> The fs_context->purpose is set to FS_CONTEXT_FOR_RECONFIGURE to indicate
> that the context is being used for reconfiguration.
>
> do_umount_root() is provided to consolidate remount-to-R/O for umount and
> emergency remount by creating a context and invoking reconfiguration.
>
> do_remount(), do_umount() and do_emergency_remount_callback() are switched
> to use the new process.

That breaks booting for me.  The problem appears to be that udev fails
to create the block devices.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply

* [PATCH] tpm: turn on TPM on suspend for TPM 1.x
From: Jarkko Sakkinen @ 2019-03-22 10:51 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, pauldzim, Jarkko Sakkinen, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

tpm_chip_start/stop() should be also called for TPM 1.x devices on
suspend. Add that functionality back. Do not lock the chip because
it is unnecessary as there are no multiple threads using it when
doing the suspend.

Fixes: a3fbfae82b4c ("tpm: take TPM chip power gating out of tpm_transmit()")
Reported-by: Paul Zimmerman <pauldzim@gmail.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 83ece5639f86..ae1030c9b086 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -402,15 +402,13 @@ int tpm_pm_suspend(struct device *dev)
 	if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
 		return 0;
 
-	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		mutex_lock(&chip->tpm_mutex);
-		if (!tpm_chip_start(chip)) {
+	if (!tpm_chip_start(chip)) {
+		if (chip->flags & TPM_CHIP_FLAG_TPM2)
 			tpm2_shutdown(chip, TPM2_SU_STATE);
-			tpm_chip_stop(chip);
-		}
-		mutex_unlock(&chip->tpm_mutex);
-	} else {
-		rc = tpm1_pm_suspend(chip, tpm_suspend_pcr);
+		else
+			rc = tpm1_pm_suspend(chip, tpm_suspend_pcr);
+
+		tpm_chip_stop(chip);
 	}
 
 	return rc;
-- 
2.19.1


^ permalink raw reply related

* [PATCH 2/2] efi: print appropriate status message when loading certificates
From: Lee, Chun-Yi @ 2019-03-22 10:33 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi
In-Reply-To: <20190322103350.27764-1-jlee@suse.com>

When loading certificates list from UEFI variable, the original error
message direct shows the efi status code from UEFI firmware. It looks
ugly:

[    2.335031] Couldn't get size: 0x800000000000000e
[    2.335032] Couldn't get UEFI MokListRT
[    2.339985] Couldn't get size: 0x800000000000000e
[    2.339987] Couldn't get UEFI dbx list

So, this patch shows the status string instead of status code.

On the other hand, the error message of EFI_NOT_FOUND
(0x800000000000000e) doesn't need to be exposed because kernel
already prints "Couldn't get UEFI..." message. This patch also
filtered out it.

Link: https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516
Cc: James Morris <jmorris@namei.org>
Cc: Serge E. Hallyn" <serge@hallyn.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 security/integrity/platform_certs/load_uefi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 81b19c52832b..fe261166621f 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -48,7 +48,9 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 
 	status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
 	if (status != EFI_BUFFER_TOO_SMALL) {
-		pr_err("Couldn't get size: 0x%lx\n", status);
+		if (status != EFI_NOT_FOUND)
+			pr_err("Couldn't get size: %s\n",
+				efi_status_to_str(status));
 		return NULL;
 	}
 
@@ -59,7 +61,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 	status = efi.get_variable(name, guid, NULL, &lsize, db);
 	if (status != EFI_SUCCESS) {
 		kfree(db);
-		pr_err("Error reading db var: 0x%lx\n", status);
+		pr_err("Error reading db var: %s\n",
+			efi_status_to_str(status));
 		return NULL;
 	}
 
-- 
2.16.4


^ permalink raw reply related

* [PATCH 1/2] efi: add a function for transferring status to string
From: Lee, Chun-Yi @ 2019-03-22 10:33 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck

This function can be used to transfer EFI status code to string
for printing out debug message. Using this function can improve
the readability of log.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 54357a258b35..a43cb0dc37af 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	default:
+		pr_warn("Unknown efi status: 0x%lx", status);
+	}
+
+	return "Unknown efi status";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.16.4


^ permalink raw reply related

* RE: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Huang, Kai @ 2019-03-21 21:41 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Christopherson, Sean J, Svahn, Kai, nhorman@redhat.com,
	jmorris@namei.org, rientjes@google.com, josh@joshtriplett.org,
	tglx@linutronix.de, Ayoun, Serge, Huang, Haitao,
	linux-security-module@vger.kernel.org, x86@kernel.org,
	akpm@linux-foundation.org, npmccallum@redhat.com,
	linux-sgx@vger.kernel.org, luto@kernel.org, Katz-zamir, Shay,
	Hansen, Dave, bp@alien8.de, serge@hallyn.com,
	andriy.shevchenko@linux.intel.com
In-Reply-To: <20190321143208.GL4603@linux.intel.com>

> On Thu, Mar 21, 2019 at 02:08:36AM +0000, Huang, Kai wrote:
> > On Tue, 2019-03-19 at 13:09 -0700, Sean Christopherson wrote:
> > > On Sun, Mar 17, 2019 at 11:14:46PM +0200, Jarkko Sakkinen wrote:
> > > > In order to provide a mechanism for devilering provisoning rights:
> > > >
> > > > 1. Add a new file to the securityfs file called sgx/provision that works
> > > >    as a token for allowing an enclave to have the provisioning privileges.
> > > > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that
> accepts the
> > > >    following data structure:
> > > >
> > > >    struct sgx_enclave_set_attribute {
> > > >            __u64 addr;
> > > >            __u64 token_fd;
> > > >    };
> >
> > Would you elaborate why the name is "token_fd"? I think *token* in SGX
> > has more specific meaning?
> 
> I'm open for other names.

How about attribute_fd, which is consistent with your IOCTL?

Thanks,
-Kai

^ permalink raw reply

* Re: [PATCH v7 0/7] Allow initializing the kernfs node's secctx based on its parent
From: Paul Moore @ 2019-03-21 21:21 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: selinux, Stephen Smalley, Linux Security Module list, Tejun Heo,
	Casey Schaufler, Serge E . Hallyn, Greg Kroah-Hartman,
	James Morris, linux-fsdevel, cgroups
In-Reply-To: <CAFqZXNtjbYUcfWODC0Ygbbxa87VhVoHaMbpvFtKaNdp3HV-JvA@mail.gmail.com>

On Thu, Mar 21, 2019 at 4:56 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Thu, Mar 21, 2019 at 3:14 AM Paul Moore <paul@paul-moore.com> wrote:
> > On Fri, Feb 22, 2019 at 9:57 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > TL;DR:
> > > This series adds a new security hook that allows to initialize the security
> > > context of kernfs properly, taking into account the parent context (and
> > > possibly other attributes). Kernfs nodes require special handling here, since
> > > they are not bound to specific inodes/superblocks, but instead represent the
> > > backing tree structure that is used to build the VFS tree when the kernfs
> > > tree is mounted.
> > >
> > > Changes in v7:
> > > - simplify the new security hook's interface
> > >   - rather than trying to extract kernfs data into common structures, just
> > >     pass the kernfs nodes themselves and add helper functions to
> > >     <linux/kernfs.h> for accessing their security xattrs
> > >   - in case other LSMs need more kernfs node attributes than the file mode
> > >     (uid/gid/...), they can simply add new helpers to <linux/kernfs.h> as
> > >     needed
> > > - refactor "kernfs: use simple_xattrs for security attributes" to keep using
> > >   a single common simple_xattrs structure
> > >   - turns out having two separate simple_xattrs wouldn't work right (see
> > >     the definition of simple_xattr_list() in fs/xattr.c)
> > > - drop unnecessary initializations from inode_doinit_use_xattr()
> > > - move the IOP_XATTR check out of inode_doinit_use_xattr()
> > > - add two kernfs cleanup patches
> > >   - these could be applied independently, but the rest of the patches depend on
> > >     them, so I'd rather they stay bundled with the rest to avoid cross-tree
> > >     conflicts
> > >
> > > v6: https://lore.kernel.org/selinux/20190214095015.16032-1-omosnace@redhat.com/T/
> > > Changes in v6:
> > > - remove copy-pasted duplicate macro definition
> > >
> > > v5: https://lore.kernel.org/selinux/20190205110638.30782-1-omosnace@redhat.com/T/
> > > Changes in v5:
> > > - fix misplaced semicolon detected by 0day robot
> > >
> > > v4: https://lore.kernel.org/selinux/20190205085915.5183-1-omosnace@redhat.com/T/
> > > Changes in v4:
> > > - reorder and rename hook arguments
> > > - avoid allocating kernfs_iattrs unless needed
> > >
> > > v3: https://lore.kernel.org/selinux/20190130114150.27807-1-omosnace@redhat.com/T/
> > > Changes in v3:
> > > - rename the hook to "kernfs_init_security"
> > > - change the hook interface to simply pass pointers to struct iattr and
> > >   struct simple_xattrs of both the new node and its parent
> > > - add full security xattr support to kernfs (and fixup SELinux behavior
> > >   to handle it properly)
> > >
> > > v2: https://lore.kernel.org/selinux/20190109162830.8309-1-omosnace@redhat.com/T/
> > > Changes in v2:
> > > - add docstring for the new hook in union security_list_options
> > > - initialize *ctx to NULL and *ctxlen to 0 in case the hook is not
> > >   implemented
> > >
> > > v1: https://lore.kernel.org/selinux/20190109091028.24485-1-omosnace@redhat.com/T/
> > >
> > > The kernfs nodes initially do not store any security context and rely on
> > > the LSM to assign some default context to inodes created over them. Kernfs
> > > inodes, however, allow setting an explicit context via the *setxattr(2)
> > > syscalls, in which case the context is stored inside the kernfs node's
> > > internal structure.
> > >
> > > SELinux (and possibly other LSMs) initialize the context of newly created
> > > FS objects based on the parent object's context (usually the child inherits
> > > the parent's context, unless the policy dictates otherwise). This is done
> > > by hooking the creation of the new inode corresponding to the newly created
> > > file/directory via security_inode_init_security() (most filesystems always
> > > create a fresh inode when a new FS object is created). However, kernfs nodes
> > > can be created "behind the scenes" while the filesystem is not mounted
> > > anywhere and thus no inodes can exist for them yet.
> > >
> > > Therefore, to allow maintaining similar behavior for kernfs nodes, a new
> > > LSM hook is needed, which will allow initializing the kernfs node's
> > > security context based on its own attributes and those of the parent's
> > > node.
> > >
> > > The main motivation for this change is that the userspace users of cgroupfs
> > > (which is built on kernfs) expect the usual security context inheritance
> > > to work under SELinux (see [1] and [2]). This functionality is required for
> > > better confinement of containers under SELinux.
> > >
> > > Patch 1/7 simplifies the kernfs_iattrs structure and patch 2/7 optimizes
> > > kernfs to not allocate kernfs_iattrs when getting the value of an xattr.
> > >
> > > Patch 3/7 changes SELinux to fetch security context from extended
> > > attributes on kernfs filesystems, falling back to genfs-defined context
> > > if that fails. Without this patch the 4/7 would be a regression for
> > > SELinux (due to the removal of ...notifysecctx() call.
> > >
> > > Patch 4/7 implements full security xattr support in kernfs using
> > > simple_xattrs; patch 5/7 adds the new LSM hook; patch 6/7 implements the
> > > new hook in SELinux; and patch 7/7 modifies kernfs to call the new hook
> > > on new node creation.
> > >
> > > Testing:
> > > - passed the reproducer from the commit message of the last patch
> > > - passed SELinux testsuite on Fedora Rawhide (x86_64) when applied on top
> > >   of current Rawhide kernel (5.0.0-0.rc7.git2.1) [3]
> > >   - including the new proposed selinux-testsuite subtest [4] (adapted
> > >     from the reproducer)
> > >
> > > [1] https://github.com/SELinuxProject/selinux-kernel/issues/39
> > > [2] https://bugzilla.redhat.com/show_bug.cgi?id=1553803
> > > [3] https://koji.fedoraproject.org/koji/taskinfo?taskID=32963825
> > > [4] https://github.com/SELinuxProject/selinux-testsuite/pull/48
> > >
> > > Ondrej Mosnacek (7):
> > >   kernfs: clean up struct kernfs_iattrs
> > >   kernfs: do not alloc iattrs in kernfs_xattr_get
> > >   selinux: try security xattr after genfs for kernfs filesystems
> > >   kernfs: use simple_xattrs for security attributes
> > >   LSM: add new hook for kernfs node initialization
> > >   selinux: implement the kernfs_init_security hook
> > >   kernfs: initialize security of newly created nodes
> > >
> > >  fs/kernfs/dir.c                     |  28 ++--
> > >  fs/kernfs/inode.c                   | 166 +++++++++------------
> > >  fs/kernfs/kernfs-internal.h         |   8 +-
> > >  fs/kernfs/symlink.c                 |   4 +-
> > >  include/linux/kernfs.h              |  15 ++
> > >  include/linux/lsm_hooks.h           |  13 ++
> > >  include/linux/security.h            |   9 ++
> > >  security/security.c                 |   6 +
> > >  security/selinux/hooks.c            | 223 +++++++++++++++++++---------
> > >  security/selinux/include/security.h |   1 +
> > >  10 files changed, 290 insertions(+), 183 deletions(-)
> >
> > I just merged this patchset into selinux/next, thanks everyone.
> >
> > Ondrej, every patch in the patchset except for one required some
> > amount of merge fixups, please take a look and make sure everything in
> > the selinux/next branch still looks right to you.
>
> Looks good to me, thanks!  I checked by rebasing the original branch
> on v5.1-rc1 myself and then comparing my result with selinux/next.
> The only difference was one extra empty line on my side, but that
> doesn't bother me :)

Thanks for double-checking.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Tetsuo Handa @ 2019-03-21 21:10 UTC (permalink / raw)
  To: Kees Cook
  Cc: Casey Schaufler, Jakub Kicinski, linux-security-module,
	Trond Myklebust, open list:NFS, SUNRPC, AND..., Anna Schumaker,
	LKML
In-Reply-To: <CAGXu5jKj-Pq4krHmKd__4MoXBe1FK399cDZnSxdPigZo78tPfA@mail.gmail.com>

On 2019/03/22 1:38, Kees Cook wrote:
> This is mostly good. I'd like to keep the other LSMs listed though
> (similar to what I had originally) so that if a legacy-major doesn't
> initialize, later ones will be. I want to remove the concept of
> "major" LSMs. The only thing that should matter is init order...

Excuse me? Are you saying that

  if a legacy-major (which is defined as the "Default security module")
  doesn't initialize, later ones (any of selinux,smack,tomoyo,apparmor
  except the one which is defined as "Default security module") will be
  initialized

? That sounds strange to me. Any of selinux,smack,tomoyo,apparmor can be
initialized when specified by lsm= kernel command line option (or security=
kernel command line option if lsm= kernel command line option is not
specified), won't it?


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox