This is a note to let you know that I've just added the patch titled
KEYS: trusted: sanitize all key material
to the 3.18-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
keys-trusted-sanitize-all-key-material.patch
and it can be found in the queue-3.18 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From ee618b4619b72527aaed765f0f0b74072b281159 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers(a)google.com>
Date: Thu, 8 Jun 2017 14:49:18 +0100
Subject: KEYS: trusted: sanitize all key material
From: Eric Biggers <ebiggers(a)google.com>
commit ee618b4619b72527aaed765f0f0b74072b281159 upstream.
As the previous patch did for encrypted-keys, zero sensitive any
potentially sensitive data related to the "trusted" key type before it
is freed. Notably, we were not zeroing the tpm_buf structures in which
the actual key is stored for TPM seal and unseal, nor were we zeroing
the trusted_key_payload in certain error paths.
Cc: Mimi Zohar <zohar(a)linux.vnet.ibm.com>
Cc: David Safford <safford(a)us.ibm.com>
Signed-off-by: Eric Biggers <ebiggers(a)google.com>
Signed-off-by: David Howells <dhowells(a)redhat.com>
Signed-off-by: James Morris <james.l.morris(a)oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
security/keys/trusted.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -69,7 +69,7 @@ static int TSS_sha1(const unsigned char
}
ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest);
- kfree(sdesc);
+ kzfree(sdesc);
return ret;
}
@@ -113,7 +113,7 @@ static int TSS_rawhmac(unsigned char *di
if (!ret)
ret = crypto_shash_final(&sdesc->shash, digest);
out:
- kfree(sdesc);
+ kzfree(sdesc);
return ret;
}
@@ -164,7 +164,7 @@ static int TSS_authhmac(unsigned char *d
paramdigest, TPM_NONCE_SIZE, h1,
TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
out:
- kfree(sdesc);
+ kzfree(sdesc);
return ret;
}
@@ -245,7 +245,7 @@ static int TSS_checkhmac1(unsigned char
if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE))
ret = -EINVAL;
out:
- kfree(sdesc);
+ kzfree(sdesc);
return ret;
}
@@ -346,7 +346,7 @@ static int TSS_checkhmac2(unsigned char
if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE))
ret = -EINVAL;
out:
- kfree(sdesc);
+ kzfree(sdesc);
return ret;
}
@@ -563,7 +563,7 @@ static int tpm_seal(struct tpm_buf *tb,
*bloblen = storedsize;
}
out:
- kfree(td);
+ kzfree(td);
return ret;
}
@@ -677,7 +677,7 @@ static int key_seal(struct trusted_key_p
if (ret < 0)
pr_info("trusted_key: srkseal failed (%d)\n", ret);
- kfree(tb);
+ kzfree(tb);
return ret;
}
@@ -702,7 +702,7 @@ static int key_unseal(struct trusted_key
/* pull migratable flag out of sealed key */
p->migratable = p->key[--p->key_len];
- kfree(tb);
+ kzfree(tb);
return ret;
}
@@ -961,12 +961,12 @@ static int trusted_instantiate(struct ke
if (!ret && options->pcrlock)
ret = pcrlock(options->pcrlock);
out:
- kfree(datablob);
- kfree(options);
+ kzfree(datablob);
+ kzfree(options);
if (!ret)
rcu_assign_keypointer(key, payload);
else
- kfree(payload);
+ kzfree(payload);
return ret;
}
@@ -975,8 +975,7 @@ static void trusted_rcu_free(struct rcu_
struct trusted_key_payload *p;
p = container_of(rcu, struct trusted_key_payload, rcu);
- memset(p->key, 0, p->key_len);
- kfree(p);
+ kzfree(p);
}
/*
@@ -1018,7 +1017,7 @@ static int trusted_update(struct key *ke
ret = datablob_parse(datablob, new_p, new_o);
if (ret != Opt_update) {
ret = -EINVAL;
- kfree(new_p);
+ kzfree(new_p);
goto out;
}
/* copy old key values, and reseal with new pcrs */
@@ -1031,22 +1030,22 @@ static int trusted_update(struct key *ke
ret = key_seal(new_p, new_o);
if (ret < 0) {
pr_info("trusted_key: key_seal failed (%d)\n", ret);
- kfree(new_p);
+ kzfree(new_p);
goto out;
}
if (new_o->pcrlock) {
ret = pcrlock(new_o->pcrlock);
if (ret < 0) {
pr_info("trusted_key: pcrlock failed (%d)\n", ret);
- kfree(new_p);
+ kzfree(new_p);
goto out;
}
}
rcu_assign_keypointer(key, new_p);
call_rcu(&p->rcu, trusted_rcu_free);
out:
- kfree(datablob);
- kfree(new_o);
+ kzfree(datablob);
+ kzfree(new_o);
return ret;
}
Patches currently in stable-queue which might be from ebiggers(a)google.com are
queue-3.18/keys-trusted-sanitize-all-key-material.patch
The patch below does not apply to the 3.18-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From a3c812f7cfd80cf51e8f5b7034f7418f6beb56c1 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers(a)google.com>
Date: Thu, 2 Nov 2017 00:47:12 +0000
Subject: [PATCH] KEYS: trusted: fix writing past end of buffer in
trusted_read()
When calling keyctl_read() on a key of type "trusted", if the
user-supplied buffer was too small, the kernel ignored the buffer length
and just wrote past the end of the buffer, potentially corrupting
userspace memory. Fix it by instead returning the size required, as per
the documentation for keyctl_read().
We also don't even fill the buffer at all in this case, as this is
slightly easier to implement than doing a short read, and either
behavior appears to be permitted. It also makes it match the behavior
of the "encrypted" key type.
Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
Reported-by: Ben Hutchings <ben(a)decadent.org.uk>
Cc: <stable(a)vger.kernel.org> # v2.6.38+
Signed-off-by: Eric Biggers <ebiggers(a)google.com>
Signed-off-by: David Howells <dhowells(a)redhat.com>
Reviewed-by: Mimi Zohar <zohar(a)linux.vnet.ibm.com>
Reviewed-by: James Morris <james.l.morris(a)oracle.com>
Signed-off-by: James Morris <james.l.morris(a)oracle.com>
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index bd85315cbfeb..98aa89ff7bfd 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1147,20 +1147,21 @@ static long trusted_read(const struct key *key, char __user *buffer,
p = dereference_key_locked(key);
if (!p)
return -EINVAL;
- if (!buffer || buflen <= 0)
- return 2 * p->blob_len;
- ascii_buf = kmalloc(2 * p->blob_len, GFP_KERNEL);
- if (!ascii_buf)
- return -ENOMEM;
- bufp = ascii_buf;
- for (i = 0; i < p->blob_len; i++)
- bufp = hex_byte_pack(bufp, p->blob[i]);
- if ((copy_to_user(buffer, ascii_buf, 2 * p->blob_len)) != 0) {
+ if (buffer && buflen >= 2 * p->blob_len) {
+ ascii_buf = kmalloc(2 * p->blob_len, GFP_KERNEL);
+ if (!ascii_buf)
+ return -ENOMEM;
+
+ bufp = ascii_buf;
+ for (i = 0; i < p->blob_len; i++)
+ bufp = hex_byte_pack(bufp, p->blob[i]);
+ if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) {
+ kzfree(ascii_buf);
+ return -EFAULT;
+ }
kzfree(ascii_buf);
- return -EFAULT;
}
- kzfree(ascii_buf);
return 2 * p->blob_len;
}
Hello,
please integrate/backport the following commit into stable kernel release 4.4:
Commit-ID: 2b02c20ce0c28974b44e69a2e2f5ddc6a470ad6f (see https://github.com/torvalds/linux/commit/2b02c20ce0c28974b44e69a2e2f5ddc6a4…)
Subject: "cdc_ncm: Set NTB format again after altsetting switch for Huawei devices"
(The patch has been submitted and merged into the mainline.)
We regard this commit as a fix/workaround for a change in firmware behavior observed in some recent huawei devices (e.g.: Huawei MS2131 LTE stick).
As these devices are unusable with the 4.4.x LTS kernel we suggest to include this workarround as there will be more and more LTE sticks to expect until EOS in 2022...
See also the mail below of the originator of the commit.
(It may also make sense to integrate it into the other stable kernel release)
Thank you very much!
Kind regards,
Matthias
----
Sender: Enrico Mioso <mrkiko.rs(a)gmail.com>
Date: 08.11.2017 23:21
Subject: Re: Question regarding commit: cdc_ncm: Set NTB format again after altsetting switch for Huawei devices
Hello Mathias.
First of all, thank you for your interest in huawei_cdc_ncm driver and contacting me.
This commit could be backported to 4.4 kernel, I didn't send that to stable(a)vger.kernel.org .
Let me know if there are any difficulties doing so. Or I might do the submission in some days.
I think that commit could be related to a bug: even if it's arguable. That bug infact, is not related to the Linux kernel itself, but to the huawei firmware.
So I don't think there will be a bug tracker entry.
Thanks for all,
Enrico
On Mon, Nov 06, 2017 at 04:57:21PM -0800, Dan Williams wrote:
> Until there is a solution to the dma-to-dax vs truncate problem it is
> not safe to allow RDMA to create long standing memory registrations
> against filesytem-dax vmas.
Looks good:
Reviewed-by: Christoph Hellwig <hch(a)lst.de>
> +long get_user_pages_longterm(unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages,
> + struct vm_area_struct **vmas)
> +{
> + struct vm_area_struct **__vmas = vmas;
How about calling the vma argument vma_arg, and the one used vma to
make thigns a little more readable?
> + struct vm_area_struct *vma_prev = NULL;
> + long rc, i;
> +
> + if (!pages)
> + return -EINVAL;
> +
> + if (!vmas && IS_ENABLED(CONFIG_FS_DAX)) {
> + __vmas = kzalloc(sizeof(struct vm_area_struct *) * nr_pages,
> + GFP_KERNEL);
> + if (!__vmas)
> + return -ENOMEM;
> + }
> +
> + rc = get_user_pages(start, nr_pages, gup_flags, pages, __vmas);
> +
> + /* skip scan for fs-dax vmas if they are compile time disabled */
> + if (!IS_ENABLED(CONFIG_FS_DAX))
> + goto out;
Instead of all this IS_ENABLED magic I'd recomment to just conditionally
compile this function and define it to get_user_pages in the header
if FS_DAX is disabled.
Else this looks fine to me.
Currently only get_user_pages_fast() can safely handle the writable gup
case due to its use of pud_access_permitted() to check whether the pud
entry is writable. In the gup slow path pud_write() is used instead of
pud_access_permitted() and to date it has been unimplemented, just calls
BUG_ON().
kernel BUG at ./include/linux/hugetlb.h:244!
[..]
RIP: 0010:follow_devmap_pud+0x482/0x490
[..]
Call Trace:
follow_page_mask+0x28c/0x6e0
__get_user_pages+0xe4/0x6c0
get_user_pages_unlocked+0x130/0x1b0
get_user_pages_fast+0x89/0xb0
iov_iter_get_pages_alloc+0x114/0x4a0
nfs_direct_read_schedule_iovec+0xd2/0x350
? nfs_start_io_direct+0x63/0x70
nfs_file_direct_read+0x1e0/0x250
nfs_file_read+0x90/0xc0
Use pud_access_permitted() to implement pud_write(), a later cleanup can
remove {pte,pmd,pud}_write and replace them with
{pte,pmd,pud}_access_permitted() drectly so that we only have one set of
helpers these kinds of checks. For now, implementing pud_write()
simplifies -stable backports.
Cc: <stable(a)vger.kernel.org>
Cc: Dave Hansen <dave.hansen(a)intel.com>
Fixes: a00cc7d9dd93 ("mm, x86: add support for PUD-sized transparent hugepages")
Signed-off-by: Dan Williams <dan.j.williams(a)intel.com>
---
Sending this as RFC for opinion on whether this should just be a
pud_flags() & _PAGE_RW check, like pmd_write, or pud_access_permitted()
that also takes protection keys into account.
include/linux/hugetlb.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index fbf5b31d47ee..6a142b240ef7 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -242,8 +242,7 @@ static inline int pgd_write(pgd_t pgd)
#ifndef pud_write
static inline int pud_write(pud_t pud)
{
- BUG();
- return 0;
+ return pud_access_permitted(pud, WRITE);
}
#endif
Aleksa Sarai <asarai(a)suse.de> writes:
> On 11/05/2017 01:56 PM, Aleksa Sarai wrote:
>> Previously, the only capability effectively required to operate on the
>> /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files,
>> having an fsuid of GLOBAL_ROOT_UID was enough). This means that
>> semi-privileged processes could interfere with core components of a
>> system (such as causing a DoS by removing the underlying SCSI device of
>> the host's / mount).
>
> An alternative to this patch would be to make the open(2) call fail, if you try
> to open it write-only or read-write. Not sure which would be preferred (should
> it be possible to pass /proc/scsi/scsi to a semi-privileged process to write
> to?).
Making open fail is very much the preferred solution.
Testing for permission on write can be avoided by finding a suid root
application whose error output acts like a suid cat.
The best current practice for adding this kind of permission check is to
add the check in open. For some older use cases where we made this
mistake we had to maintian a check during write to avoid breaking
userspace. But as this check is new there is no reason to add a check
anywhere except in open.
Eric
On Thu, Nov 09, 2017 at 04:59:58PM +0000, Ben Hutchings wrote:
> On Thu, 2017-11-09 at 08:03 -0800, Guenter Roeck wrote:
> > On Thu, Nov 09, 2017 at 12:40:36PM +0000, Ben Hutchings wrote:
> > > On Thu, 2017-11-09 at 13:21 +0100, Arnd Bergmann wrote:
> > > > On Thu, Nov 9, 2017 at 1:08 PM, Greg KH <greg(a)kroah.com> wrote:
> > > > > On Thu, Nov 09, 2017 at 12:55:30PM +0100, Arnd Bergmann wrote:
> > >
> > > [...]
> > > > > > I think if you upload the branch to the stable-rc git, that should produce
> > > > > > the automated build and boot results via email or via the
> > > > > > https://kernelci.org/job/ interface. Once there are some results
> > > > > > there, I'll go through the list once more to see what warnings
> > > > > > and failures remain.
> > > > >
> > > > > I don't know of a way to have others push to that tree/branch at the
> > > > > moment :(
> > > > >
> > > > > I'll go update that branch now...
> > > >
> > > > Thanks!
> > > >
> > > > With the arm-soc tree, we simply have a shared group-id on
> > > > gitolite.kernel.org and everyone in that group can push to it.
> > > >
> > > > If that is the only thing you need, it should be trivial to let Ben
> > > > and Sasha push to /pub/scm/linux/kernel/git/stable/*.git as well,
> > > > I'm sure helpdesk(a)kernel.org can arrange that. Of course if you are
> > > > worried about having multiple accounts with write access to all the
> > > > branches, then that wouldn't be enough.
> > >
> > > I think I'd rather send a pull request to Greg at the start of the
> > > review period.
> > >
> >
> > If you change the trees I am supposed to pull from for my builders,
> > please let me know.
>
> If you're happy to keep supporting quilt-in-git then there's no change.
> I check your builders page and try to fix up build failures before even
> making a release candidate.
>
Ah yes, kernelci won't pick that up. No problem to keep kerneltests going
as long as it adds value.
Guenter