* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Christoph Hellwig @ 2020-06-24 14:43 UTC (permalink / raw)
To: Christian Borntraeger
Cc: mcgrof, ast, axboe, bfields, bridge, chainsaw, christian.brauner,
chuck.lever, davem, dhowells, gregkh, jarkko.sakkinen, jmorris,
josh, keescook, keyrings, kuba, lars.ellenberg, linux-fsdevel,
linux-kernel, linux-nfs, linux-security-module, nikolay,
philipp.reisner, ravenexp, roopa, serge, slyfox, viro, yangtiezhu,
netdev, markward, linux-s390
In-Reply-To: <3118dc0d-a3af-9337-c897-2380062a8644@de.ibm.com>
On Wed, Jun 24, 2020 at 01:11:54PM +0200, Christian Borntraeger wrote:
> Does anyone have an idea why "umh: fix processed error when UMH_WAIT_PROC is used" breaks the
> linux-bridge on s390?
Are we even sure this is s390 specific and doesn't happen on other
architectures with the same bridge setup?
^ permalink raw reply
* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Luis Chamberlain @ 2020-06-24 13:17 UTC (permalink / raw)
To: Christian Borntraeger, Andrew Morton, Martin Doucha
Cc: ast, axboe, bfields, bridge, chainsaw, christian.brauner,
chuck.lever, davem, dhowells, gregkh, jarkko.sakkinen, jmorris,
josh, keescook, keyrings, kuba, lars.ellenberg, linux-fsdevel,
linux-kernel, linux-nfs, linux-security-module, nikolay,
philipp.reisner, ravenexp, roopa, serge, slyfox, viro, yangtiezhu,
netdev, markward, linux-s390
In-Reply-To: <20200624120546.GC4332@42.do-not-panic.com>
Martin, your eyeballs would be appreciated for a bit on this.
On Wed, Jun 24, 2020 at 12:05:46PM +0000, Luis Chamberlain wrote:
> On Wed, Jun 24, 2020 at 01:11:54PM +0200, Christian Borntraeger wrote:
> >
> >
> > On 23.06.20 16:23, Christian Borntraeger wrote:
> > >
> > >
> > > On 23.06.20 16:11, Christian Borntraeger wrote:
> > >> Jens Markwardt reported a regression in the linux-next runs. with "umh: fix
> > >> processed error when UMH_WAIT_PROC is used" (from linux-next) a linux bridge
> > >> with an KVM guests no longer activates :
> > >>
> > >> without patch
> > >> # ip addr show dev virbr1
> > >> 6: virbr1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
> > >> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
> > >> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
> > >> valid_lft forever preferred_lft forever
> > >>
> > >> with this patch the bridge stays DOWN with NO-CARRIER
> > >>
> > >> # ip addr show dev virbr1
> > >> 6: virbr1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN group default qlen 1000
> > >> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
> > >> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
> > >> valid_lft forever preferred_lft forever
> > >>
> > >> This was bisected in linux-next. Reverting from linux-next also fixes the issue.
> > >>
> > >> Any idea?
> > >
> > > FWIW, s390 is big endian. Maybe some of the shifts inn the __KW* macros are wrong.
> >
> > Does anyone have an idea why "umh: fix processed error when UMH_WAIT_PROC is used" breaks the
> > linux-bridge on s390?
>
> glibc for instance defines __WEXITSTATUS in only one location: bits/waitstatus.h
> and it does not special case it per architecture, so at this point I'd
> have to say we have to look somewhere else for why this is happening.
I found however an LTP bug indicating the need to test for
s390 wait macros [0] in light of a recent bug in glibc for s390.
I am asking for references to that issue given I cannot find
any mention of this on glibc yet.
I'm in hopes Martin might be aware of that mentioned s390 glic bug.
[0] https://github.com/linux-test-project/ltp/issues/605
Luis
^ permalink raw reply
* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Luis Chamberlain @ 2020-06-24 12:05 UTC (permalink / raw)
To: Christian Borntraeger
Cc: ast, axboe, bfields, bridge, chainsaw, christian.brauner,
chuck.lever, davem, dhowells, gregkh, jarkko.sakkinen, jmorris,
josh, keescook, keyrings, kuba, lars.ellenberg, linux-fsdevel,
linux-kernel, linux-nfs, linux-security-module, nikolay,
philipp.reisner, ravenexp, roopa, serge, slyfox, viro, yangtiezhu,
netdev, markward, linux-s390
In-Reply-To: <3118dc0d-a3af-9337-c897-2380062a8644@de.ibm.com>
On Wed, Jun 24, 2020 at 01:11:54PM +0200, Christian Borntraeger wrote:
>
>
> On 23.06.20 16:23, Christian Borntraeger wrote:
> >
> >
> > On 23.06.20 16:11, Christian Borntraeger wrote:
> >> Jens Markwardt reported a regression in the linux-next runs. with "umh: fix
> >> processed error when UMH_WAIT_PROC is used" (from linux-next) a linux bridge
> >> with an KVM guests no longer activates :
> >>
> >> without patch
> >> # ip addr show dev virbr1
> >> 6: virbr1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
> >> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
> >> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
> >> valid_lft forever preferred_lft forever
> >>
> >> with this patch the bridge stays DOWN with NO-CARRIER
> >>
> >> # ip addr show dev virbr1
> >> 6: virbr1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN group default qlen 1000
> >> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
> >> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
> >> valid_lft forever preferred_lft forever
> >>
> >> This was bisected in linux-next. Reverting from linux-next also fixes the issue.
> >>
> >> Any idea?
> >
> > FWIW, s390 is big endian. Maybe some of the shifts inn the __KW* macros are wrong.
>
> Does anyone have an idea why "umh: fix processed error when UMH_WAIT_PROC is used" breaks the
> linux-bridge on s390?
glibc for instance defines __WEXITSTATUS in only one location: bits/waitstatus.h
and it does not special case it per architecture, so at this point I'd
have to say we have to look somewhere else for why this is happening.
The commmit which caused this is issuing a correct error code down the
pipeline, nothing more. I'll make taking a look at this a priority right
now. Let us see what I come up with today.
Luis
^ permalink raw reply
* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Christian Borntraeger @ 2020-06-24 11:11 UTC (permalink / raw)
To: mcgrof
Cc: ast, axboe, bfields, bridge, chainsaw, christian.brauner,
chuck.lever, davem, dhowells, gregkh, jarkko.sakkinen, jmorris,
josh, keescook, keyrings, kuba, lars.ellenberg, linux-fsdevel,
linux-kernel, linux-nfs, linux-security-module, nikolay,
philipp.reisner, ravenexp, roopa, serge, slyfox, viro, yangtiezhu,
netdev, markward, linux-s390
In-Reply-To: <b7d658b9-606a-feb1-61f9-b58e3420d711@de.ibm.com>
On 23.06.20 16:23, Christian Borntraeger wrote:
>
>
> On 23.06.20 16:11, Christian Borntraeger wrote:
>> Jens Markwardt reported a regression in the linux-next runs. with "umh: fix
>> processed error when UMH_WAIT_PROC is used" (from linux-next) a linux bridge
>> with an KVM guests no longer activates :
>>
>> without patch
>> # ip addr show dev virbr1
>> 6: virbr1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
>> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
>> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
>> valid_lft forever preferred_lft forever
>>
>> with this patch the bridge stays DOWN with NO-CARRIER
>>
>> # ip addr show dev virbr1
>> 6: virbr1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN group default qlen 1000
>> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
>> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
>> valid_lft forever preferred_lft forever
>>
>> This was bisected in linux-next. Reverting from linux-next also fixes the issue.
>>
>> Any idea?
>
> FWIW, s390 is big endian. Maybe some of the shifts inn the __KW* macros are wrong.
Does anyone have an idea why "umh: fix processed error when UMH_WAIT_PROC is used" breaks the
linux-bridge on s390?
^ permalink raw reply
* Re: [PATCH v3 1/1] fs: move kernel_read_file* to its own include file
From: Christoph Hellwig @ 2020-06-24 7:55 UTC (permalink / raw)
To: Scott Branden
Cc: Christoph Hellwig, Luis Chamberlain, Alexander Viro,
Eric Biederman, Jessica Yu, BCM Kernel Feedback,
Greg Kroah-Hartman, Rafael J . Wysocki, James Morris,
Serge E . Hallyn, Mimi Zohar, Dmitry Kasatkin, Kees Cook,
Paul Moore, Stephen Smalley, Eric Paris, linux-kernel,
linux-fsdevel, kexec, linux-security-module, linux-integrity,
selinux
In-Reply-To: <20200617161218.18487-1-scott.branden@broadcom.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [RFC][PATCH] net/bpfilter: Remove this broken and apparently unmantained
From: Tetsuo Handa @ 2020-06-24 7:05 UTC (permalink / raw)
To: linux-security-module
Cc: Alexei Starovoitov, Eric W. Biederman, Linus Torvalds, Kees Cook,
Andrew Morton, Alexei Starovoitov, David Miller, Al Viro, bpf,
linux-fsdevel, Daniel Borkmann, Jakub Kicinski, Masahiro Yamada,
Gary Lin, Bruno Meneguele, Casey Schaufler
In-Reply-To: <20200624063940.ctzhf4nnh3cjyxqi@ast-mbp.dhcp.thefacebook.com>
Forwarding to LSM-ML again. Any comments?
On 2020/06/24 15:39, Alexei Starovoitov wrote:
> On Wed, Jun 24, 2020 at 01:58:33PM +0900, Tetsuo Handa wrote:
>> On 2020/06/24 13:00, Alexei Starovoitov wrote:
>>>> However, regarding usermode_blob, although the byte array (which contains code / data)
>>>> might be initially loaded from the kernel space (which is protected), that byte array
>>>> is no longer protected (e.g. SIGKILL, strace()) when executed because they are placed
>>>> in the user address space. Thus, LSM modules (including pathname based security) want
>>>> to control how that byte array can behave.
>>>
>>> It's privileged memory regardless. root can poke into kernel or any process memory.
>>
>> LSM is there to restrict processes running as "root".
>
> hmm. do you really mean that it's possible for an LSM to restrict CAP_SYS_ADMIN effectively?
> LSM can certainly provide extra level of foolproof-ness against accidental
> mistakes, but it's not a security boundary.
>
>> Your "root can poke into kernel or any process memory." response is out of step with the times.
>>
>> Initial byte array used for usermode blob might be protected because of "part of .rodata or
>> .init.rodata of kernel module", but that byte array after started in userspace is no longer
>> protected.
>>
>> I don't trust such byte array as "part of kernel module", and I'm asking you how
>> such byte array does not interfere (or be interfered by) the rest of the system.
>
> Could you please explain the attack vector that you see in such scenario?
> How elf binaries embedded in the kernel modules different from pid 1?
> If anything can peek into their memory the system is compromised.
> Say, there are no user blobs in kernel modules. How pid 1 memory is different
> from all the JITed images? How is it different for all memory regions shared
> between kernel and user processes?
> I see an opportunity for an LSM to provide a protection against non-security
> bugs when system is running trusted apps, but not when arbitrary code can
> execute under root.
>
^ permalink raw reply
* Re: LTP: crypto: af_alg02 regression on linux-next 20200621 tag
From: Herbert Xu @ 2020-06-24 0:23 UTC (permalink / raw)
To: Eric Biggers
Cc: Naresh Kamboju, Luis Chamberlain, LTP List, open list,
linux-security-module, keyrings, lkft-triage, linux-crypto,
Jan Stancek, chrubis, Serge E. Hallyn, James Morris,
Jarkko Sakkinen, David Howells, David S. Miller
In-Reply-To: <20200623170217.GB150582@gmail.com>
On Tue, Jun 23, 2020 at 10:02:17AM -0700, Eric Biggers wrote:
>
> The source code for the two failing AF_ALG tests is here:
>
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg05.c
>
> They use read() and write(), not send() and recv().
>
> af_alg02 uses read() to read from a "salsa20" request socket without writing
> anything to it. It is expected that this returns 0, i.e. that behaves like
> encrypting an empty message.
>
> af_alg05 uses write() to write 15 bytes to a "cbc(aes-generic)" request socket,
> then read() to read 15 bytes. It is expected that this fails with EINVAL, since
> the length is not aligned to the AES block size (16 bytes).
Thanks. Sounds like it's my introduction of the init variable that
broke this. Let me investigate.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] security: Fix hook iteration and default value for inode_copy_up_xattr
From: James Morris @ 2020-06-23 23:40 UTC (permalink / raw)
To: KP Singh
Cc: bpf, linux-security-module, Alexei Starovoitov, Daniel Borkmann,
Jann Horn, Casey Schaufler, Kees Cook
In-Reply-To: <20200621222135.9136-1-kpsingh@chromium.org>
On Mon, 22 Jun 2020, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> inode_copy_up_xattr returns 0 to indicate the acceptance of the xattr
> and 1 to reject it. If the LSM does not know about the xattr, it's
> expected to return -EOPNOTSUPP, which is the correct default value for
> this hook. BPF LSM, currently, uses 0 as the default value and thereby
> falsely allows all overlay fs xattributes to be copied up.
>
> The iteration logic is also updated from the "bail-on-fail"
> call_int_hook to continue on the non-decisive -EOPNOTSUPP and bail out
> on other values.
>
> Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> Signed-off-by: KP Singh <kpsingh@google.com>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git fixes-v5.8
> ---
> include/linux/lsm_hook_defs.h | 2 +-
> security/security.c | 17 ++++++++++++++++-
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 6791813cd439..f4b2e54162ae 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -150,7 +150,7 @@ LSM_HOOK(int, 0, inode_listsecurity, struct inode *inode, char *buffer,
> size_t buffer_size)
> LSM_HOOK(void, LSM_RET_VOID, inode_getsecid, struct inode *inode, u32 *secid)
> LSM_HOOK(int, 0, inode_copy_up, struct dentry *src, struct cred **new)
> -LSM_HOOK(int, 0, inode_copy_up_xattr, const char *name)
> +LSM_HOOK(int, -EOPNOTSUPP, inode_copy_up_xattr, const char *name)
> LSM_HOOK(int, 0, kernfs_init_security, struct kernfs_node *kn_dir,
> struct kernfs_node *kn)
> LSM_HOOK(int, 0, file_permission, struct file *file, int mask)
> diff --git a/security/security.c b/security/security.c
> index 0ce3e73edd42..70a7ad357bc6 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1414,7 +1414,22 @@ EXPORT_SYMBOL(security_inode_copy_up);
>
> int security_inode_copy_up_xattr(const char *name)
> {
> - return call_int_hook(inode_copy_up_xattr, -EOPNOTSUPP, name);
> + struct security_hook_list *hp;
> + int rc;
> +
> + /*
> + * The implementation can return 0 (accept the xattr), 1 (discard the
> + * xattr), -EOPNOTSUPP if it does not know anything about the xattr or
> + * any other error code incase of an error.
> + */
> + hlist_for_each_entry(hp,
> + &security_hook_heads.inode_copy_up_xattr, list) {
> + rc = hp->hook.inode_copy_up_xattr(name);
> + if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> + return rc;
> + }
> +
> + return LSM_RET_DEFAULT(inode_copy_up_xattr);
> }
> EXPORT_SYMBOL(security_inode_copy_up_xattr);
>
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PATCH] ima: AppArmor satisfies the audit rule requirements
From: Tyler Hicks @ 2020-06-23 23:38 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Matthew Garrett, linux-kernel,
linux-integrity, linux-security-module
AppArmor meets all the requirements for IMA in terms of audit rules
since commit e79c26d04043 ("apparmor: Add support for audit rule
filtering"). Update IMA's Kconfig section for CONFIG_IMA_LSM_RULES to
reflect this.
Fixes: e79c26d04043 ("apparmor: Add support for audit rule filtering")
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---
security/integrity/ima/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index edde88dbe576..794ebb5cbf74 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -54,7 +54,7 @@ config IMA_MEASURE_PCR_IDX
config IMA_LSM_RULES
bool
- depends on IMA && AUDIT && (SECURITY_SELINUX || SECURITY_SMACK)
+ depends on IMA && AUDIT && (SECURITY_SELINUX || SECURITY_SMACK || SECURITY_APPARMOR)
default y
help
Disabling this option will disregard LSM based policy rules.
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 01/12] ima: Have the LSM free its audit rule
From: Tyler Hicks @ 2020-06-23 23:04 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module, Janne Karhunen
In-Reply-To: <20200623003236.830149-2-tyhicks@linux.microsoft.com>
On 2020-06-22 19:32:25, Tyler Hicks wrote:
> Ask the LSM to free its audit rule rather than directly calling kfree().
> Both AppArmor and SELinux do additional work in their audit_rule_free()
> hooks. Fix memory leaks by allowing the LSMs to perform necessary work.
>
> Fixes: b16942455193 ("ima: use the lsm policy update notifier")
> Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> Cc: Janne Karhunen <janne.karhunen@gmail.com>
> ---
> security/integrity/ima/ima.h | 6 ++++++
> security/integrity/ima/ima_policy.c | 2 +-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index df93ac258e01..de05d7f1d3ec 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -404,6 +404,7 @@ static inline void ima_free_modsig(struct modsig *modsig)
> #ifdef CONFIG_IMA_LSM_RULES
>
> #define security_filter_rule_init security_audit_rule_init
> +#define security_filter_rule_free security_audit_rule_free
> #define security_filter_rule_match security_audit_rule_match
>
> #else
> @@ -414,6 +415,11 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr,
> return -EINVAL;
> }
>
> +static inline void security_filter_rule_free(void *lsmrule)
> +{
> + return -EINVAL;
Bah, I introduced a build warning here when CONFIG_IMA_LSM_RULES is
disabled. This function should return nothing. I'll wait for additional
feedback before spinning a v2.
Tyler
> +}
> +
> static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
> void *lsmrule)
> {
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index e493063a3c34..236a731492d1 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -258,7 +258,7 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry)
> int i;
>
> for (i = 0; i < MAX_LSM_RULES; i++) {
> - kfree(entry->lsm[i].rule);
> + security_filter_rule_free(entry->lsm[i].rule);
> kfree(entry->lsm[i].args_p);
> }
> kfree(entry);
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v4] ima: extend boot_aggregate with kernel measurements
From: Bruno Meneguele @ 2020-06-23 18:53 UTC (permalink / raw)
To: Maurizio Drocco
Cc: zohar, Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
linux-integrity, linux-kernel, linux-security-module, mdrocco,
roberto.sassu, serge
In-Reply-To: <20200623155732.105-1-maurizio.drocco@ibm.com>
[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]
On Tue, Jun 23, 2020 at 11:57:32AM -0400, Maurizio Drocco wrote:
> Registers 8-9 are used to store measurements of the kernel and its
> command line (e.g., grub2 bootloader with tpm module enabled). IMA
> should include them in the boot aggregate. Registers 8-9 should be
> only included in non-SHA1 digests to avoid ambiguity.
>
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---
> Changelog:
> v4:
> - Reworded comments: PCRs 8 & 9 are always included in non-sha1 digests
> v3:
> - Limit including PCRs 8 & 9 to non-sha1 hashes
> v2:
> - Minor comment improvements
> v1:
> - Include non zero PCRs 8 & 9 in the boot_aggregate
>
> security/integrity/ima/ima.h | 2 +-
> security/integrity/ima/ima_crypto.c | 15 ++++++++++++++-
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index df93ac258e01..9d94080bdad8 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -30,7 +30,7 @@
>
> enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
> IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
> -enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
> +enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>
> /* digest size for IMA, fits SHA1 or MD5 */
> #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 220b14920c37..011c3c76af86 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -823,13 +823,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> if (rc != 0)
> return rc;
>
> - /* cumulative sha1 over tpm registers 0-7 */
> + /* cumulative digest over TPM registers 0-7 */
> for (i = TPM_PCR0; i < TPM_PCR8; i++) {
> ima_pcrread(i, &d);
> /* now accumulate with current aggregate */
> rc = crypto_shash_update(shash, d.digest,
> crypto_shash_digestsize(tfm));
> }
> + /*
> + * Extend cumulative digest over TPM registers 8-9, which contain
> + * measurement for the kernel command line (reg. 8) and image (reg. 9)
> + * in a typical PCR allocation. Registers 8-9 are only included in
> + * non-SHA1 boot_aggregate digests to avoid ambiguity.
> + */
> + if (alg_id != TPM_ALG_SHA1) {
> + for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> + ima_pcrread(i, &d);
> + rc = crypto_shash_update(shash, d.digest,
> + crypto_shash_digestsize(tfm));
> + }
> + }
> if (!rc)
> crypto_shash_final(shash, digest);
> return rc;
> --
> 2.17.1
>
Reviewed-by: Bruno Meneguele <bmeneg@redhat.com>
I've tested this patch with both TPM 1.2 and TPM 2.0 + ima-evm-utils
support patch. Everything seems fine.
Thanks.
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
From: Bruno Meneguele @ 2020-06-23 18:13 UTC (permalink / raw)
To: Maurizio Drocco
Cc: zohar, Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
linux-integrity, linux-kernel, linux-security-module, mdrocco,
roberto.sassu, serge
In-Reply-To: <20200623180122.209-1-maurizio.drocco@ibm.com>
[-- Attachment #1: Type: text/plain, Size: 1952 bytes --]
On Tue, Jun 23, 2020 at 02:01:22PM -0400, Maurizio Drocco wrote:
> From: Maurizio <maurizio.drocco@ibm.com>
>
> If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
> them into the digest.
>
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---
> Changelog:
> v2:
> - Always include PCRs 8 & 9 to non-sha1 hashes
> v1:
> - Include non-zero PCRs 8 & 9 to boot aggregates
>
> src/evmctl.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 1d065ce..46b7092 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
> }
> }
>
> + if (strcmp(bank->algo_name, "sha1") != 0) {
> + for (i = 8; i < 10; i++) {
> + err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
> + if (!err) {
> + log_err("EVP_DigestUpdate() failed\n");
> + return;
> + }
> + }
> + }
> +
> err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
> if (!err) {
> log_err("EVP_DigestFinal() failed\n");
> @@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
> /*
> * The IMA measurement list boot_aggregate is the link between the preboot
> * event log and the IMA measurement list. Read and calculate all the
> - * possible per TPM bank boot_aggregate digests based on the existing
> - * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
> + * possible per TPM bank boot_aggregate digests based on the existing PCRs
> + * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
> + * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
> */
> static int cmd_ima_bootaggr(struct command *cmd)
> {
> --
> 2.17.1
>
Reviewed-by: Bruno Meneguele <bmeneg@redhat.com>
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
From: Maurizio Drocco @ 2020-06-23 18:01 UTC (permalink / raw)
To: zohar
Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris, linux-integrity,
linux-kernel, linux-security-module, maurizio.drocco, mdrocco,
roberto.sassu, serge
In-Reply-To: <1592856871.4987.21.camel@linux.ibm.com>
From: Maurizio <maurizio.drocco@ibm.com>
If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
them into the digest.
Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
Changelog:
v2:
- Always include PCRs 8 & 9 to non-sha1 hashes
v1:
- Include non-zero PCRs 8 & 9 to boot aggregates
src/evmctl.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index 1d065ce..46b7092 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
}
}
+ if (strcmp(bank->algo_name, "sha1") != 0) {
+ for (i = 8; i < 10; i++) {
+ err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
+ if (!err) {
+ log_err("EVP_DigestUpdate() failed\n");
+ return;
+ }
+ }
+ }
+
err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
if (!err) {
log_err("EVP_DigestFinal() failed\n");
@@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
/*
* The IMA measurement list boot_aggregate is the link between the preboot
* event log and the IMA measurement list. Read and calculate all the
- * possible per TPM bank boot_aggregate digests based on the existing
- * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
+ * possible per TPM bank boot_aggregate digests based on the existing PCRs
+ * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
+ * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
*/
static int cmd_ima_bootaggr(struct command *cmd)
{
--
2.17.1
^ permalink raw reply related
* Re: LTP: crypto: af_alg02 regression on linux-next 20200621 tag
From: Eric Biggers @ 2020-06-23 17:02 UTC (permalink / raw)
To: Herbert Xu
Cc: Naresh Kamboju, Luis Chamberlain, LTP List, open list,
linux-security-module, keyrings, lkft-triage, linux-crypto,
Jan Stancek, chrubis, Serge E. Hallyn, James Morris,
Jarkko Sakkinen, David Howells, David S. Miller
In-Reply-To: <20200623064056.GA8121@gondor.apana.org.au>
On Tue, Jun 23, 2020 at 04:40:56PM +1000, Herbert Xu wrote:
> On Tue, Jun 23, 2020 at 11:53:43AM +0530, Naresh Kamboju wrote:
> >
> > Thanks for the investigation.
> > After reverting, two test cases got PASS out of four reported failure cases.
> > ltp-crypto-tests:
> > * af_alg02 - still failing - Hung and time out
> > * af_alg05 - still failing - Hung and time out
> > ltp-syscalls-tests:
> > * keyctl07 - PASS
> > * request_key03 - PASS
> >
> > Please suggest the way to debug / fix the af_alg02 and af_alg05 failures.
>
> Did you clear the MSG_MORE flag in the final send(2) call before
> you call recv(2)?
>
The source code for the two failing AF_ALG tests is here:
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg05.c
They use read() and write(), not send() and recv().
af_alg02 uses read() to read from a "salsa20" request socket without writing
anything to it. It is expected that this returns 0, i.e. that behaves like
encrypting an empty message.
af_alg05 uses write() to write 15 bytes to a "cbc(aes-generic)" request socket,
then read() to read 15 bytes. It is expected that this fails with EINVAL, since
the length is not aligned to the AES block size (16 bytes).
- Eric
^ permalink raw reply
* [PATCH v4] ima: extend boot_aggregate with kernel measurements
From: Maurizio Drocco @ 2020-06-23 15:57 UTC (permalink / raw)
To: zohar
Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris, linux-integrity,
linux-kernel, linux-security-module, maurizio.drocco, mdrocco,
roberto.sassu, serge
In-Reply-To: <1592920990.5437.15.camel@linux.ibm.com>
Registers 8-9 are used to store measurements of the kernel and its
command line (e.g., grub2 bootloader with tpm module enabled). IMA
should include them in the boot aggregate. Registers 8-9 should be
only included in non-SHA1 digests to avoid ambiguity.
Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
Changelog:
v4:
- Reworded comments: PCRs 8 & 9 are always included in non-sha1 digests
v3:
- Limit including PCRs 8 & 9 to non-sha1 hashes
v2:
- Minor comment improvements
v1:
- Include non zero PCRs 8 & 9 in the boot_aggregate
security/integrity/ima/ima.h | 2 +-
security/integrity/ima/ima_crypto.c | 15 ++++++++++++++-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index df93ac258e01..9d94080bdad8 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -30,7 +30,7 @@
enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
-enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
+enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
/* digest size for IMA, fits SHA1 or MD5 */
#define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 220b14920c37..011c3c76af86 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -823,13 +823,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
if (rc != 0)
return rc;
- /* cumulative sha1 over tpm registers 0-7 */
+ /* cumulative digest over TPM registers 0-7 */
for (i = TPM_PCR0; i < TPM_PCR8; i++) {
ima_pcrread(i, &d);
/* now accumulate with current aggregate */
rc = crypto_shash_update(shash, d.digest,
crypto_shash_digestsize(tfm));
}
+ /*
+ * Extend cumulative digest over TPM registers 8-9, which contain
+ * measurement for the kernel command line (reg. 8) and image (reg. 9)
+ * in a typical PCR allocation. Registers 8-9 are only included in
+ * non-SHA1 boot_aggregate digests to avoid ambiguity.
+ */
+ if (alg_id != TPM_ALG_SHA1) {
+ for (i = TPM_PCR8; i < TPM_PCR10; i++) {
+ ima_pcrread(i, &d);
+ rc = crypto_shash_update(shash, d.digest,
+ crypto_shash_digestsize(tfm));
+ }
+ }
if (!rc)
crypto_shash_final(shash, digest);
return rc;
--
2.17.1
^ permalink raw reply related
* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Christian Borntraeger @ 2020-06-23 14:23 UTC (permalink / raw)
To: mcgrof
Cc: ast, axboe, bfields, bridge, chainsaw, christian.brauner,
chuck.lever, davem, dhowells, gregkh, jarkko.sakkinen, jmorris,
josh, keescook, keyrings, kuba, lars.ellenberg, linux-fsdevel,
linux-kernel, linux-nfs, linux-security-module, nikolay,
philipp.reisner, ravenexp, roopa, serge, slyfox, viro, yangtiezhu,
netdev, markward, linux-s390
In-Reply-To: <20200623141157.5409-1-borntraeger@de.ibm.com>
On 23.06.20 16:11, Christian Borntraeger wrote:
> Jens Markwardt reported a regression in the linux-next runs. with "umh: fix
> processed error when UMH_WAIT_PROC is used" (from linux-next) a linux bridge
> with an KVM guests no longer activates :
>
> without patch
> # ip addr show dev virbr1
> 6: virbr1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
> valid_lft forever preferred_lft forever
>
> with this patch the bridge stays DOWN with NO-CARRIER
>
> # ip addr show dev virbr1
> 6: virbr1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN group default qlen 1000
> link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
> inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
> valid_lft forever preferred_lft forever
>
> This was bisected in linux-next. Reverting from linux-next also fixes the issue.
>
> Any idea?
FWIW, s390 is big endian. Maybe some of the shifts inn the __KW* macros are wrong.
^ permalink raw reply
* linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Christian Borntraeger @ 2020-06-23 14:11 UTC (permalink / raw)
To: mcgrof
Cc: ast, axboe, bfields, bridge, chainsaw, christian.brauner,
chuck.lever, davem, dhowells, gregkh, jarkko.sakkinen, jmorris,
josh, keescook, keyrings, kuba, lars.ellenberg, linux-fsdevel,
linux-kernel, linux-nfs, linux-security-module, nikolay,
philipp.reisner, ravenexp, roopa, serge, slyfox, viro, yangtiezhu,
netdev, markward, linux-s390
In-Reply-To: <20200610154923.27510-5-mcgrof@kernel.org>
Jens Markwardt reported a regression in the linux-next runs. with "umh: fix
processed error when UMH_WAIT_PROC is used" (from linux-next) a linux bridge
with an KVM guests no longer activates :
without patch
# ip addr show dev virbr1
6: virbr1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
valid_lft forever preferred_lft forever
with this patch the bridge stays DOWN with NO-CARRIER
# ip addr show dev virbr1
6: virbr1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN group default qlen 1000
link/ether 52:54:00:1e:3f:c0 brd ff:ff:ff:ff:ff:ff
inet 192.168.254.254/24 brd 192.168.254.255 scope global virbr1
valid_lft forever preferred_lft forever
This was bisected in linux-next. Reverting from linux-next also fixes the issue.
Any idea?
Christian
^ permalink raw reply
* Re: [PATCH] ima: extend boot_aggregate with kernel measurements
From: Mimi Zohar @ 2020-06-23 14:03 UTC (permalink / raw)
To: Maurizio Drocco
Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris, linux-integrity,
linux-kernel, linux-security-module, mdrocco, roberto.sassu,
serge
In-Reply-To: <20200622045019.1636-1-maurizio.drocco@ibm.com>
Hi Maurizio,
When re-posting patches, please include the version number (e.g.
[PATCH v4] ima: ... ).
On Mon, 2020-06-22 at 00:50 -0400, Maurizio Drocco wrote:
> IMA is not considering TPM registers 8-9 when calculating the boot
> aggregate.
This line is unnecessary with the following change.
> When registers 8-9 are used to store measurements of the
> kernel and its command line (e.g., grub2 bootloader with tpm module
> enabled), IMA should include them in the boot aggregate.
The "When" clause makes this sound like PCRs 8 & 9 are not always
included. I would split this into two sentences.
> Registers
> 8-9 are only included in non-SHA1 boot_aggregate digests to avoid
> ambiguity.
>
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---
Missing "Changelog:".
Changelog:
v2:
- Limit including PCRs 8 & 9 to non-sha1 hashes
v1:
- Include non zero PCRs 8 & 9 in the boot_aggregate
> security/integrity/ima/ima.h | 2 +-
> security/integrity/ima/ima_crypto.c | 15 ++++++++++++++-
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index df93ac258e01..9d94080bdad8 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -30,7 +30,7 @@
>
> enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
> IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
> -enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
> +enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>
> /* digest size for IMA, fits SHA1 or MD5 */
> #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 220b14920c37..d02917d85033 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -823,13 +823,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> if (rc != 0)
> return rc;
>
> - /* cumulative sha1 over tpm registers 0-7 */
> + /* cumulative digest over tpm registers 0-7 */
Please uppercase "tpm" here and below.
> for (i = TPM_PCR0; i < TPM_PCR8; i++) {
> ima_pcrread(i, &d);
> /* now accumulate with current aggregate */
> rc = crypto_shash_update(shash, d.digest,
> crypto_shash_digestsize(tfm));
> }
> + /*
> + * extend cumulative digest over tpm registers 8-9, which contain
> + * measurement for the kernel command line (reg. 8) and image (reg. 9)
> + * in a typical PCR allocation. Registers 8-9 are only included in
> + * non-SHA1 boot_aggregate digests to avoid ambiguity.
> + */
Comments that are full sentences should start with an uppercase letter
and end with a period (e.g. Extend).
thanks,
Mimi
> + if (alg_id != TPM_ALG_SHA1) {
> + for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> + ima_pcrread(i, &d);
> + rc = crypto_shash_update(shash, d.digest,
> + crypto_shash_digestsize(tfm));
> + }
> + }
> if (!rc)
> crypto_shash_final(shash, digest);
> return rc;
^ permalink raw reply
* [PATCH v5 0/2] tpm2: Make TPM2 logs accessible for non-UEFI firmware
From: Stefan Berger @ 2020-06-23 12:06 UTC (permalink / raw)
To: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module
Cc: Stefan Berger
From: Stefan Berger <stfeanb@linux.ibm.com>
This series of patches extends the existing TPM2 ACPI table with additional
fields found in the TPM2 TCG ACPI specification (reference is in the patch)
that allow access to the log's address and its size. We then modify the
code that so far only enables access to a TPM 1.2's log for a TPM2 as well.
This then enables access to the TPM2's log on non-UEFI system that for example
run SeaBIOS.
Stefan
v4->v5:
- Added R-bs and A-bs.
v3->v4:
- Repost as one series
v2->v3:
- Split the series into two separate patches
- Added comments to ACPI table fields
- Added check for null pointer to log area and zero log size
v1->v2:
- Repost of the series
Stefan Berger (2):
acpi: Extend TPM2 ACPI table with missing log fields
tpm: Add support for event log pointer found in TPM2 ACPI table
drivers/char/tpm/eventlog/acpi.c | 56 ++++++++++++++++++++------------
drivers/char/tpm/tpm_crb.c | 13 ++++++--
drivers/char/tpm/tpm_tis.c | 4 ++-
include/acpi/actbl3.h | 5 +--
4 files changed, 51 insertions(+), 27 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH v5 1/2] acpi: Extend TPM2 ACPI table with missing log fields
From: Stefan Berger @ 2020-06-23 12:06 UTC (permalink / raw)
To: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module
Cc: Stefan Berger, Rafael J . Wysocki
In-Reply-To: <20200623120636.1453470-1-stefanb@linux.vnet.ibm.com>
From: Stefan Berger <stefanb@linux.ibm.com>
Recent extensions of the TPM2 ACPI table added 3 more fields
including 12 bytes of start method specific parameters and Log Area
Minimum Length (u32) and Log Area Start Address (u64). So, we extend
the existing structure with these fields to allow non-UEFI systems
to access the TPM2's log.
The specification that has the new fields is the following:
TCG ACPI Specification
Family "1.2" and "2.0"
Version 1.2, Revision 8
Adapt all existing table size calculations to use
offsetof(struct acpi_table_tpm2, start_method_specific)
[where start_method_specific is a newly added field]
rather than sizeof(struct acpi_table_tpm2) so that the addition
of the new fields does not affect current systems that may not
have them.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Cc: linux-acpi@vger.kernel.org
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_crb.c | 13 ++++++++++---
drivers/char/tpm/tpm_tis.c | 4 +++-
include/acpi/actbl3.h | 5 +++--
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index a9dcf31eadd2..0565aa5482f9 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -669,7 +669,9 @@ static int crb_acpi_add(struct acpi_device *device)
status = acpi_get_table(ACPI_SIG_TPM2, 1,
(struct acpi_table_header **) &buf);
- if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
+ if (ACPI_FAILURE(status) || buf->header.length <
+ offsetof(struct acpi_table_tpm2,
+ start_method_specific)) {
dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
return -EINVAL;
}
@@ -684,14 +686,19 @@ static int crb_acpi_add(struct acpi_device *device)
return -ENOMEM;
if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
- if (buf->header.length < (sizeof(*buf) + sizeof(*crb_smc))) {
+ if (buf->header.length <
+ (offsetof(struct acpi_table_tpm2,
+ start_method_specific) +
+ sizeof(*crb_smc))) {
dev_err(dev,
FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",
buf->header.length,
ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC);
return -EINVAL;
}
- crb_smc = ACPI_ADD_PTR(struct tpm2_crb_smc, buf, sizeof(*buf));
+ crb_smc = ACPI_ADD_PTR(struct tpm2_crb_smc, buf,
+ offsetof(struct acpi_table_tpm2,
+ start_method_specific));
priv->smc_func_id = crb_smc->smc_func_id;
}
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index e7df342a317d..a80f36860bac 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -111,7 +111,9 @@ static int check_acpi_tpm2(struct device *dev)
*/
st =
acpi_get_table(ACPI_SIG_TPM2, 1, (struct acpi_table_header **)&tbl);
- if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
+ if (ACPI_FAILURE(st) || tbl->header.length <
+ offsetof(struct acpi_table_tpm2,
+ start_method_specific)) {
dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
return -EINVAL;
}
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index b0b163b9efc6..8b55426bbcf6 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -411,8 +411,9 @@ struct acpi_table_tpm2 {
u16 reserved;
u64 control_address;
u32 start_method;
-
- /* Platform-specific data follows */
+ u8 start_method_specific[12];
+ u32 log_area_minimum_length; /* optional */
+ u64 log_area_start_address; /* optional */
};
/* Values for start_method above */
--
2.26.2
^ permalink raw reply related
* [PATCH v5 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Stefan Berger @ 2020-06-23 12:06 UTC (permalink / raw)
To: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module
Cc: Stefan Berger
In-Reply-To: <20200623120636.1453470-1-stefanb@linux.vnet.ibm.com>
From: Stefan Berger <stefanb@linux.ibm.com>
In case a TPM2 is attached, search for a TPM2 ACPI table when trying
to get the event log from ACPI. If one is found, use it to get the
start and length of the log area. This allows non-UEFI systems, such
as SeaBIOS, to pass an event log when using a TPM2.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/eventlog/acpi.c | 56 ++++++++++++++++++++------------
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index 63ada5e53f13..e714a2bd0423 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -49,9 +49,8 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
void __iomem *virt;
u64 len, start;
struct tpm_bios_log *log;
-
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- return -ENODEV;
+ struct acpi_table_tpm2 *tbl;
+ int format;
log = &chip->log;
@@ -61,23 +60,38 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
if (!chip->acpi_dev_handle)
return -ENODEV;
- /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
- status = acpi_get_table(ACPI_SIG_TCPA, 1,
- (struct acpi_table_header **)&buff);
-
- if (ACPI_FAILURE(status))
- return -ENODEV;
-
- switch(buff->platform_class) {
- case BIOS_SERVER:
- len = buff->server.log_max_len;
- start = buff->server.log_start_addr;
- break;
- case BIOS_CLIENT:
- default:
- len = buff->client.log_max_len;
- start = buff->client.log_start_addr;
- break;
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ status = acpi_get_table("TPM2", 1,
+ (struct acpi_table_header **)&tbl);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+ if (tbl->header.length < sizeof(*tbl))
+ return -ENODEV;
+ len = tbl->log_area_minimum_length;
+ start = tbl->log_area_start_address;
+ if (!start || !len)
+ return -ENODEV;
+ format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
+ } else {
+ /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
+ status = acpi_get_table(ACPI_SIG_TCPA, 1,
+ (struct acpi_table_header **)&buff);
+
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ switch (buff->platform_class) {
+ case BIOS_SERVER:
+ len = buff->server.log_max_len;
+ start = buff->server.log_start_addr;
+ break;
+ case BIOS_CLIENT:
+ default:
+ len = buff->client.log_max_len;
+ start = buff->client.log_start_addr;
+ break;
+ }
+ format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
}
if (!len) {
dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
@@ -98,7 +112,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
memcpy_fromio(log->bios_event_log, virt, len);
acpi_os_unmap_iomem(virt, len);
- return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+ return format;
err:
kfree(log->bios_event_log);
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3] acpi: Extend TPM2 ACPI table with missing log fields
From: Stefan Berger @ 2020-06-23 11:27 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Rafael J. Wysocki, Stefan Berger, Rafael J. Wysocki,
linux-integrity, ACPI Devel Maling List,
Linux Kernel Mailing List, linux-security-module
In-Reply-To: <20200623005647.GD28795@linux.intel.com>
On 6/22/20 8:56 PM, Jarkko Sakkinen wrote:
> On Fri, Jun 19, 2020 at 11:14:20AM -0400, Stefan Berger wrote:
>> On 4/2/20 3:21 PM, Jarkko Sakkinen wrote:
>>> On Wed, Apr 01, 2020 at 11:05:36AM +0200, Rafael J. Wysocki wrote:
>>>> On Wed, Apr 1, 2020 at 10:37 AM Jarkko Sakkinen
>>>> <jarkko.sakkinen@linux.intel.com> wrote:
>>>>> On Tue, Mar 31, 2020 at 05:49:49PM -0400, Stefan Berger wrote:
>>>>>> From: Stefan Berger <stefanb@linux.ibm.com>
>>>>>>
>>>>>> Recent extensions of the TPM2 ACPI table added 3 more fields
>>>>>> including 12 bytes of start method specific parameters and Log Area
>>>>>> Minimum Length (u32) and Log Area Start Address (u64). So, we extend
>>>>>> the existing structure with these fields to allow non-UEFI systems
>>>>>> to access the TPM2's log.
>>>>>>
>>>>>> The specification that has the new fields is the following:
>>>>>> TCG ACPI Specification
>>>>>> Family "1.2" and "2.0"
>>>>>> Version 1.2, Revision 8
>>>>>>
>>>>>> Adapt all existing table size calculations to use
>>>>>> offsetof(struct acpi_table_tpm2, start_method_specific)
>>>>>> [where start_method_specific is a newly added field]
>>>>>> rather than sizeof(struct acpi_table_tpm2) so that the addition
>>>>>> of the new fields does not affect current systems that may not
>>>>>> have them.
>>>>>>
>>>>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>>>>> Cc: linux-acpi@vger.kernel.org
>>>>> I think I'm cool with this but needs an ack from ACPI maintainer.
>>>>>
>>>>> Rafael, given that this not an intrusive change in any possible means,
>>>>> can I pick this patch and put it to my next pull request?
>>>> Yes, please.
>>>>
>>>> Thanks!
>>> Great, thanks Rafael.
>>>
>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>
>>> Do you mind if I add your ack to the commit?
>>
>> Any chance to get v4 applied?
> You should split the actbl3.h change to a separate patch and add 'Cc:'
> tag to Rafael to the commit message.
I did this in one patch because it seems like a mistake to extend the
structure and not modify the size checks.
Stefan
>
> /Jarkko
^ permalink raw reply
* Re: LTP: crypto: af_alg02 regression on linux-next 20200621 tag
From: Herbert Xu @ 2020-06-23 6:40 UTC (permalink / raw)
To: Naresh Kamboju
Cc: Luis Chamberlain, LTP List, open list, linux-security-module,
keyrings, lkft-triage, linux-crypto, Jan Stancek, chrubis,
Serge E. Hallyn, James Morris, Jarkko Sakkinen, David Howells,
David S. Miller, Eric Biggers
In-Reply-To: <CA+G9fYsXDZUspc5OyfqrGZn=k=2uRiGzWY_aPePK2C_kZ+dYGQ@mail.gmail.com>
On Tue, Jun 23, 2020 at 11:53:43AM +0530, Naresh Kamboju wrote:
>
> Thanks for the investigation.
> After reverting, two test cases got PASS out of four reported failure cases.
> ltp-crypto-tests:
> * af_alg02 - still failing - Hung and time out
> * af_alg05 - still failing - Hung and time out
> ltp-syscalls-tests:
> * keyctl07 - PASS
> * request_key03 - PASS
>
> Please suggest the way to debug / fix the af_alg02 and af_alg05 failures.
Did you clear the MSG_MORE flag in the final send(2) call before
you call recv(2)?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: LTP: crypto: af_alg02 regression on linux-next 20200621 tag
From: Naresh Kamboju @ 2020-06-23 6:23 UTC (permalink / raw)
To: Luis Chamberlain
Cc: LTP List, open list, linux-security-module, keyrings, lkft-triage,
linux-crypto, Herbert Xu, Jan Stancek, chrubis, Serge E. Hallyn,
James Morris, Jarkko Sakkinen, David Howells, David S. Miller,
Eric Biggers
In-Reply-To: <20200622224920.GA4332@42.do-not-panic.com>
On Tue, 23 Jun 2020 at 04:19, Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Tue, Jun 23, 2020 at 12:04:06AM +0530, Naresh Kamboju wrote:
> > LTP crypto regressions noticed on linux next 20200621.
> >
> > The common case for all tests is timeout after 15 minutes which
> > means tests got hung and LTP timers killed those test runs after
> > timeout.
> > The root cause of the failure is under investigation.
> >
> > ltp-crypto-tests:
> > * af_alg02 - failed
> > * af_alg05 - failed
> > ltp-syscalls-tests:
> > * keyctl07 - failed
> > * request_key03 - failed
<trim>
>
> Can you try reverting:
>
> d13ef8e10756873b0a8b7cc8f230a2d1026710ea
>
> The patch is titled "umh: fix processed error when UMH_WAIT_PROC is used"
Thanks for the investigation.
After reverting, two test cases got PASS out of four reported failure cases.
ltp-crypto-tests:
* af_alg02 - still failing - Hung and time out
* af_alg05 - still failing - Hung and time out
ltp-syscalls-tests:
* keyctl07 - PASS
* request_key03 - PASS
Please suggest the way to debug / fix the af_alg02 and af_alg05 failures.
- Naresh
^ permalink raw reply
* Re: [PATCH 01/12] ima: Have the LSM free its audit rule
From: Tyler Hicks @ 2020-06-23 3:04 UTC (permalink / raw)
To: Casey Schaufler
Cc: Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E . Hallyn,
Lakshmi Ramasubramanian, Prakhar Srivastava, linux-kernel,
linux-integrity, linux-security-module, Janne Karhunen
In-Reply-To: <277dd210-c443-c067-e731-44ac53418fa5@schaufler-ca.com>
On 2020-06-22 17:55:59, Casey Schaufler wrote:
> On 6/22/2020 5:32 PM, Tyler Hicks wrote:
> > Ask the LSM to free its audit rule rather than directly calling kfree().
> > Both AppArmor and SELinux do additional work in their audit_rule_free()
> > hooks. Fix memory leaks by allowing the LSMs to perform necessary work.
> >
> > Fixes: b16942455193 ("ima: use the lsm policy update notifier")
> > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> > Cc: Janne Karhunen <janne.karhunen@gmail.com>
> > ---
> > security/integrity/ima/ima.h | 6 ++++++
> > security/integrity/ima/ima_policy.c | 2 +-
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index df93ac258e01..de05d7f1d3ec 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -404,6 +404,7 @@ static inline void ima_free_modsig(struct modsig *modsig)
> > #ifdef CONFIG_IMA_LSM_RULES
> >
> > #define security_filter_rule_init security_audit_rule_init
> > +#define security_filter_rule_free security_audit_rule_free
> > #define security_filter_rule_match security_audit_rule_match
>
> In context this seems perfectly reasonable. If, however, you're
> working with the LSM infrastructure this set of #defines is maddening.
> The existing ones have been driving my nuts for the past few years,
> so I'd like to discourage adding another. Since the security_filter_rule
> functions are IMA specific they shouldn't be prefixed security_. I know
> that it seems to be code churn/bikesheading, but we please change these:
>
> static inline int ima_filter_rule_init(.....)
> {
> return security_audit_rule_init(.....);
> }
>
> and so forth. I understand if you don't want to make the change.
> I have plenty of other things driving me crazy just now, so this
> doesn't seem likely to push me over the edge.
I'd be happy to take a stab at that as a follow-up or a 13/12 patch. I'd
like to leave this one as-is for stable kernel reasons since it is
straightforward and simple.
Tyler
>
> >
> > #else
> > @@ -414,6 +415,11 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr,
> > return -EINVAL;
> > }
> >
> > +static inline void security_filter_rule_free(void *lsmrule)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
> > void *lsmrule)
> > {
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index e493063a3c34..236a731492d1 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -258,7 +258,7 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry)
> > int i;
> >
> > for (i = 0; i < MAX_LSM_RULES; i++) {
> > - kfree(entry->lsm[i].rule);
> > + security_filter_rule_free(entry->lsm[i].rule);
> > kfree(entry->lsm[i].args_p);
> > }
> > kfree(entry);
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox