Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
Hi Marcin,
On Wed, Sep 27, 2017 at 02:18:36PM +0200, Marcin Nowakowski wrote:
> This module registers crc32 and crc32c algorithms that use the
> optional CRC32[bhwd] and CRC32C[bhwd] instructions in MIPSr6 cores.
>
> Signed-off-by: Marcin Nowakowski
> Cc: linux-crypto@vger.kernel.org
> Cc: Herbert Xu
> Cc: "David S. Miller"
>
> ---
> arch/mips/Kconfig | 4 +
> arch/mips/Makefile| 3 +
> arch/mips/crypto/Makefile | 5 +
> arch/mips/crypto/crc32-mips.c | 361
> ++
> crypto/Kconfig| 9 ++
> 5 files changed, 382 insertions(+)
> create mode 100644 arch/mips/crypto/Makefile
> create mode 100644 arch/mips/crypto/crc32-mips.c
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index cb7fcc4..0f96812 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2036,6 +2036,7 @@ config CPU_MIPSR6
> select CPU_HAS_RIXI
> select HAVE_ARCH_BITREVERSE
> select MIPS_ASID_BITS_VARIABLE
> + select MIPS_CRC_SUPPORT
> select MIPS_SPRAM
>
> config EVA
> @@ -2503,6 +2504,9 @@ config MIPS_ASID_BITS
> config MIPS_ASID_BITS_VARIABLE
> bool
>
> +config MIPS_CRC_SUPPORT
> + bool
> +
> #
> # - Highmem only makes sense for the 32-bit kernel.
> # - The current highmem code will only work properly on physically indexed
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index a96d97a..aa77536 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -216,6 +216,8 @@ cflags-$(toolchain-msa) +=
> -DTOOLCHAIN_SUPPORTS_MSA
> endif
> toolchain-virt := $(call
> cc-option-yn,$(mips-cflags) -mvirt)
> cflags-$(toolchain-virt) += -DTOOLCHAIN_SUPPORTS_VIRT
> +toolchain-crc:= $(call
> cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
> +cflags-$(toolchain-crc) += -DTOOLCHAIN_SUPPORTS_CRC
>
> #
> # Firmware support
> @@ -324,6 +326,7 @@ libs-y+= arch/mips/math-emu/
> # See arch/mips/Kbuild for content of core part of the kernel
> core-y += arch/mips/
>
> +drivers-$(CONFIG_MIPS_CRC_SUPPORT) += arch/mips/crypto/
> drivers-$(CONFIG_OPROFILE) += arch/mips/oprofile/
>
> # suspend and hibernation support
> diff --git a/arch/mips/crypto/Makefile b/arch/mips/crypto/Makefile
> new file mode 100644
> index 000..665c725
> --- /dev/null
> +++ b/arch/mips/crypto/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for MIPS crypto files..
> +#
> +
> +obj-$(CONFIG_CRYPTO_CRC32_MIPS) += crc32-mips.o
> diff --git a/arch/mips/crypto/crc32-mips.c b/arch/mips/crypto/crc32-mips.c
> new file mode 100644
> index 000..dfa8bb1
> --- /dev/null
> +++ b/arch/mips/crypto/crc32-mips.c
> @@ -0,0 +1,361 @@
> +/*
> + * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
> + *
> + * Module based on arm64/crypto/crc32-arm.c
> + *
> + * Copyright (C) 2014 Linaro Ltd
> + * Copyright (C) 2017 Imagination Technologies, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +enum crc_op_size {
> + b, h, w, d,
> +};
> +
> +enum crc_type {
> + crc32,
> + crc32c,
> +};
> +
> +#ifdef TOOLCHAIN_SUPPORTS_CRC
> +
> +#define _CRC32(crc, value, size, type) \
> +do { \
> + __asm__ __volatile__( \
> + ".set push\n\t" \
> + ".set crc\n\t"\
> + #type #size " %0, %1, %0\n\t" \
> + ".set pop\n\t"\
Technically the \n\t on the last line is redundant.
> + : "+r" (crc)\
> + : "r" (value) \
> +); \
> +} while(0)
> +
> +#define CRC_REGISTER
> +
> +#else/* TOOLCHAIN_SUPPORTS_CRC */
> +/*
> + * Crc argument is currently ignored and the assembly below assumes
> + * the crc is stored in $2. As the register number is encoded in the
> + * instruction we can't let the compiler chose the register it wants.
> + * An alternative is to change the code to do
> + * move $2, %0
> + * crc32
> + * move %0, $2
> + * but that adds unnecessary operations that the crc32 operation is
> + * designed to avoid. This issue can go away once the assembler
> + * is extended to support this operation and the compiler can make
> + * the right register choice automatically
> + */
> +
> +#define _CRC32(crc, value, size, type)
> \
> +do {
> \
> + __asm__ __volatile__(
> \
> +
[Part2 PATCH v4.1 05/30] crypto: ccp: Add Platform Security Processor (PSP) device support
The Platform Security Processor (PSP) is part of AMD Secure Processor
(AMD-SP), PSP is a dedicated processor that provides the support for
key management commands in a Secure Encrypted Virtualization (SEV) mode,
along with software-based Trusted Execution Environment (TEE) to enable
the third-party trusted applications.
Cc: Paolo Bonzini
Cc: "Radim Krčmář"
Cc: Borislav Petkov
Cc: Herbert Xu
Cc: Gary Hook
Cc: Tom Lendacky
Cc: linux-crypto@vger.kernel.org
Cc: k...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Brijesh Singh
---
Hi Boris,
I have been going through feedbacks and issues reported by kbuild robot and
have been making the fixes at:
repo: https://github.com/codomania/kvm.git
branch: sev-v4-p2+fixes
After I am finished with all the fixes then will submit v5 part2.
In meantime, if you don't mind then can you look at updated patches
on sev-v4-p2+fixes branch or you can just wait for me to submit v5.
thank you very much.
-Brijesh
drivers/crypto/ccp/Kconfig | 11 +
drivers/crypto/ccp/Makefile | 1 +
drivers/crypto/ccp/psp-dev.c | 113 +++
drivers/crypto/ccp/psp-dev.h | 61 +++
drivers/crypto/ccp/sp-dev.c | 26 ++
drivers/crypto/ccp/sp-dev.h | 26 +-
drivers/crypto/ccp/sp-pci.c | 46 ++
7 files changed, 283 insertions(+), 1 deletion(-)
create mode 100644 drivers/crypto/ccp/psp-dev.c
create mode 100644 drivers/crypto/ccp/psp-dev.h
diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
index 6d626606b9c5..627f3e61dcac 100644
--- a/drivers/crypto/ccp/Kconfig
+++ b/drivers/crypto/ccp/Kconfig
@@ -32,3 +32,14 @@ config CRYPTO_DEV_CCP_CRYPTO
Support for using the cryptographic API with the AMD Cryptographic
Coprocessor. This module supports offload of SHA and AES algorithms.
If you choose 'M' here, this module will be called ccp_crypto.
+
+config CRYPTO_DEV_SP_PSP
+ bool "Platform Security Processor (PSP) device"
+ default y
+ depends on CRYPTO_DEV_CCP_DD
+ help
+Provide the support for AMD Platform Security Processor (PSP). PSP is
+a dedicated processor that provides the support for key management
+commands in a Secure Encrypted Virtualiztion (SEV) mode, along with
+software-based Trusted Execution Environment (TEE) to enable the
+third-party trusted applications.
diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
index 57f8debfcfb3..008bae7e26ec 100644
--- a/drivers/crypto/ccp/Makefile
+++ b/drivers/crypto/ccp/Makefile
@@ -7,6 +7,7 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_CCP) += ccp-dev.o \
ccp-dmaengine.o \
ccp-debugfs.o
ccp-$(CONFIG_PCI) += sp-pci.o
+ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o
obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
ccp-crypto-objs := ccp-crypto-main.o \
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
new file mode 100644
index ..b201c524c28f
--- /dev/null
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -0,0 +1,113 @@
+/*
+ * AMD Platform Security Processor (PSP) interface
+ *
+ * Copyright (C) 2016-2017 Advanced Micro Devices, Inc.
+ *
+ * Author: Brijesh Singh
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "sp-dev.h"
+#include "psp-dev.h"
+
+const struct psp_vdata psp_entry = {
+ .offset = 0x10500,
+};
+
+static struct psp_device *psp_alloc_struct(struct sp_device *sp)
+{
+ struct device *dev = sp->dev;
+ struct psp_device *psp;
+
+ psp = devm_kzalloc(dev, sizeof(*psp), GFP_KERNEL);
+ if (!psp)
+ return NULL;
+
+ psp->dev = dev;
+ psp->sp = sp;
+
+ snprintf(psp->name, sizeof(psp->name), "psp-%u", sp->ord);
+
+ return psp;
+}
+
+static irqreturn_t psp_irq_handler(int irq, void *data)
+{
+ return IRQ_HANDLED;
+}
+
+int psp_dev_init(struct sp_device *sp)
+{
+ struct device *dev = sp->dev;
+ struct psp_device *psp;
+ int ret;
+
+ ret = -ENOMEM;
+ psp = psp_alloc_struct(sp);
+ if (!psp)
+ goto e_err;
+
+ sp->psp_data = psp;
+
+ psp->vdata = (struct psp_vdata *)sp->dev_vdata->psp_vdata;
+ if (!psp->vdata) {
+ ret = -ENODEV;
+ dev_err(dev, "missing driver data\n");
+ goto e_err;
+ }
+
+ psp->io_regs = sp->io_map + psp->vdata->offset;
+
+ /* Disable and clear interrupts until ready */
+ iowrite32(0, psp->io_regs + PSP_P2CMSG_INTEN);
+ iowrite32(-1, psp->io_regs + PSP_P2CMSG_INTSTS);
+
+ dev_dbg(dev, "requesting an IRQ ...\n");
+ /* Request an irq */
+
Re: [PATCH] iommu/vt-d: Fix scatterlist offset handling
| From: Harsh Jain | Sent: Friday, September 29, 2017 1:14:45 AM | | Robin, | | I tried running patch on our test setup. | | With "intel_iommu=on" : I can see single occurrence of DMAR Write failure | on perf traffic with 10 thread. | | [ 749.616480] perf: interrupt took too long (3203 > 3202), lowering kernel.perf_event_max_sample_rate to 62000 | [ 852.500671] DMAR: DRHD: handling fault status reg 2 | [ 852.506039] DMAR: [DMA Write] Request device [02:00.4] fault addr ef919000 [fault reason 05] PTE Write access is not set | [root@heptagon linux_t4_build]# cat /proc/cmdline | BOOT_IMAGE=/vmlinuz-4.9.51+ root=UUID=ccbb7f18-b3f0-43df-89de-07521e9c02fe ro intel_iommu=on crashkernel=auto rhgb quiet rhgb quiet console=ttyS0,115200, console=tty0 LANG=en_US.UTF-8 Harsh. Can you provide the debugging information for that one DMA FAILURE trace? It May be yet another corner case in __domain_mapping() or a different path. | With intel_iommu=sp_off : It works fine for more than 30 minutes without | any issues. I think that even without Robin's patch using intel_iommu=sp_off worked without errors, right? Casey
Re: [Part2 PATCH v4 05/29] crypto: ccp: Add Platform Security Processor (PSP) device support
On Tue, Sep 19, 2017 at 03:46:03PM -0500, Brijesh Singh wrote:
> Platform Security Processor (PSP) is part of AMD Secure Processor (AMD-SP),
The Platform...
> PSP is a dedicated processor that provides the support for key management
> commands in a Secure Encrypted Virtualiztion (SEV) mode, along with
Virtualization
Is integrating that spellchecker hard? Because what I do, for example,
is press F7 in vim when I've written the commit message. And F7 is
mapped to:
map :set spell! spelllang=en_us
spellfile=~/.vim/spellfile.add:echo "spellcheck: " . strpart("offon",
3 * &spell, 3)
in my .vimrc
And I'm pretty sure you can do a similar thing with other editors.
> software-based Trusted Executation Environment (TEE) to enable the
> third-party trusted applications.
>
> Cc: Paolo Bonzini
> Cc: "Radim Krčmář"
> Cc: Borislav Petkov
> Cc: Herbert Xu
> Cc: Gary Hook
> Cc: Tom Lendacky
> Cc: linux-crypto@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Brijesh Singh
> ---
> drivers/crypto/ccp/Kconfig | 11 +
> drivers/crypto/ccp/Makefile | 1 +
> drivers/crypto/ccp/psp-dev.c | 111
> +++
> drivers/crypto/ccp/psp-dev.h | 61
> drivers/crypto/ccp/sp-dev.c | 32 +
> drivers/crypto/ccp/sp-dev.h | 27 ++-
> drivers/crypto/ccp/sp-pci.c | 46 ++
> 7 files changed, 288 insertions(+), 1 deletion(-)
> create mode 100644 drivers/crypto/ccp/psp-dev.c
> create mode 100644 drivers/crypto/ccp/psp-dev.h
>
> diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
> index 6d626606b9c5..1d927e13bf31 100644
> --- a/drivers/crypto/ccp/Kconfig
> +++ b/drivers/crypto/ccp/Kconfig
> @@ -32,3 +32,14 @@ config CRYPTO_DEV_CCP_CRYPTO
> Support for using the cryptographic API with the AMD Cryptographic
> Coprocessor. This module supports offload of SHA and AES algorithms.
> If you choose 'M' here, this module will be called ccp_crypto.
> +
> +config CRYPTO_DEV_SP_PSP
> + bool "Platform Security Processor (PSP) device"
> + default y
> + depends on CRYPTO_DEV_CCP_DD
So this last symbol CRYPTO_DEV_CCP_DD is default m and it doesn't depend
on anything. And I'm pretty sure it should depend on CPU_SUP_AMD as this
is AMD-specific hw. You can add that dependency in a prepatch.
And what happened to adding dependencies on CONFIG_KVM_AMD? Or can you
use the PSP without virtualization in any sensible way?
> + help
> + Provide the support for AMD Platform Security Processor (PSP). PSP is
... for the AMD ... The PSP
...
> + a dedicated processor that provides the support for key management
that provides support for
> + commands in in a Secure Encrypted Virtualiztion (SEV) mode, along with
... in Secure Encrypted Virtualization
> + software-based Trusted Executation Environment (TEE) to enable the
> + third-party trusted applications.
> diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
> index 57f8debfcfb3..008bae7e26ec 100644
> --- a/drivers/crypto/ccp/Makefile
> +++ b/drivers/crypto/ccp/Makefile
> @@ -7,6 +7,7 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_CCP) += ccp-dev.o \
> ccp-dmaengine.o \
> ccp-debugfs.o
> ccp-$(CONFIG_PCI) += sp-pci.o
> +ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o
>
> obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
> ccp-crypto-objs := ccp-crypto-main.o \
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> new file mode 100644
> index ..e60e53272e71
> --- /dev/null
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -0,0 +1,111 @@
> +/*
> + * AMD Platform Security Processor (PSP) interface
> + *
> + * Copyright (C) 2016-2017 Advanced Micro Devices, Inc.
> + *
> + * Author: Brijesh Singh
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "sp-dev.h"
> +#include "psp-dev.h"
> +
> +const struct psp_vdata psp_entry = {
> + .offset = 0x10500,
> +};
> +
> +static struct psp_device *psp_alloc_struct(struct sp_device *sp)
> +{
> + struct device *dev = sp->dev;
> + struct psp_device *psp;
> +
> + psp = devm_kzalloc(dev, sizeof(*psp), GFP_KERNEL);
> + if (!psp)
> + return NULL;
> +
> + psp->dev = dev;
> + psp->sp = sp;
> +
> + snprintf(psp->name, sizeof(psp->name), "psp-%u", sp->ord);
> +
> + return psp;
> +}
> +
> +irqreturn_t psp_irq_handler(int irq, void *data)
Not static because... ?
> +{
> + return IRQ_HANDLED;
> +}
> +
> +int psp_
Re: [v2 PATCH 5/5] Bluetooth: let the crypto subsystem generate the ecc privkey
Hi Tudor,
>>> That Bluetooth SMP knows about the private key is pointless, since the
>>> detection of debug key usage is actually via the public key portion.
>>> With this patch, the Bluetooth SMP will stop keeping a copy of the
>>> ecdh private key and will let the crypto subsystem to generate and
>>> handle the ecdh private key, potentially benefiting of hardware
>>> ecc private key generation and retention.
>>>
>>> The loop that tries to generate a correct private key is now removed and
>>> we trust the crypto subsystem to generate a correct private key. This
>>> backup logic should be done in crypto, if really needed.
>>>
>>> Signed-off-by: Tudor Ambarus
>>> ---
>>> net/bluetooth/ecdh_helper.c | 186
>>>
>>> net/bluetooth/ecdh_helper.h | 9 ++-
>>> net/bluetooth/selftest.c| 14 +++-
>>> net/bluetooth/smp.c | 66 +++-
>>> 4 files changed, 147 insertions(+), 128 deletions(-)
>>>
>>> diff --git a/net/bluetooth/ecdh_helper.c b/net/bluetooth/ecdh_helper.c
>>> index 16e022f..2155ce8 100644
>>> --- a/net/bluetooth/ecdh_helper.c
>>> +++ b/net/bluetooth/ecdh_helper.c
>>> @@ -49,15 +49,21 @@ static inline void swap_digits(u64 *in, u64 *out,
>>> unsigned int ndigits)
>>> out[i] = __swab64(in[ndigits - 1 - i]);
>>> }
>>>
>>> +/* compute_ecdh_secret() - function assumes that the private key was
>>> + * already set.
>>> + * @tfm: KPP tfm handle allocated with crypto_alloc_kpp().
>>> + * @public_key: pair's ecc public key.
>>> + * secret:memory where the ecdh computed shared secret will be
>>> saved.
>>> + *
>>> + * Return: zero on success; error code in case of error.
>>> + */
>>> int compute_ecdh_secret(struct crypto_kpp *tfm, const u8 public_key[64],
>>> - const u8 private_key[32], u8 secret[32])
>>> + u8 secret[32])
>>> {
>>> struct kpp_request *req;
>>> - struct ecdh p;
>>> + u8 *tmp;
>>> struct ecdh_completion result;
>>> struct scatterlist src, dst;
>>> - u8 *tmp, *buf;
>>> - unsigned int buf_len;
>>> int err;
>>>
>>> tmp = kmalloc(64, GFP_KERNEL);
>>> @@ -72,28 +78,6 @@ int compute_ecdh_secret(struct crypto_kpp *tfm, const u8
>>> public_key[64],
>>>
>>> init_completion(&result.completion);
>>>
>>> - /* Security Manager Protocol holds digits in litte-endian order
>>> -* while ECC API expect big-endian data
>>> -*/
>>> - swap_digits((u64 *)private_key, (u64 *)tmp, 4);
>>> - p.key = (char *)tmp;
>>> - p.key_size = 32;
>>> - /* Set curve_id */
>>> - p.curve_id = ECC_CURVE_NIST_P256;
>>> - buf_len = crypto_ecdh_key_len(&p);
>>> - buf = kmalloc(buf_len, GFP_KERNEL);
>>> - if (!buf) {
>>> - err = -ENOMEM;
>>> - goto free_req;
>>> - }
>>> -
>>> - crypto_ecdh_encode_key(buf, buf_len, &p);
>>> -
>>> - /* Set A private Key */
>>> - err = crypto_kpp_set_secret(tfm, (void *)buf, buf_len);
>>> - if (err)
>>> - goto free_all;
>>> -
>>> swap_digits((u64 *)public_key, (u64 *)tmp, 4); /* x */
>>> swap_digits((u64 *)&public_key[32], (u64 *)&tmp[32], 4); /* y */
>>>
>>> @@ -118,26 +102,76 @@ int compute_ecdh_secret(struct crypto_kpp *tfm, const
>>> u8 public_key[64],
>>> memcpy(secret, tmp, 32);
>>>
>>> free_all:
>>> - kzfree(buf);
>>> -free_req:
>>> kpp_request_free(req);
>>> free_tmp:
>>> kzfree(tmp);
>>> return err;
>>> }
>>>
>>> -int generate_ecdh_keys(struct crypto_kpp *tfm, u8 public_key[64],
>>> - u8 private_key[32])
>>> +/* set_ecdh_privkey() - set or generate ecc private key.
>>> + *
>>> + * Function generates an ecc private key in the crypto subsystem when
>>> receiving
>>> + * a NULL private key or sets the received key when not NULL.
>>> + *
>>> + * @tfm: KPP tfm handle allocated with crypto_alloc_kpp().
>>> + * @private_key: user's ecc private key. When not NULL, the key is
>>> expected
>>> + * in little endian format.
>>> + *
>>> + * Return: zero on success; error code in case of error.
>>> + */
>>> +int set_ecdh_privkey(struct crypto_kpp *tfm, const u8 private_key[32])
>>> +{
>>> + u8 *buf, *tmp = NULL;
>>> + unsigned int buf_len;
>>> + int err;
>>> + struct ecdh p = {0};
>>> +
>>> + p.curve_id = ECC_CURVE_NIST_P256;
>>> +
>>> + if (private_key) {
>>> + tmp = kmalloc(32, GFP_KERNEL);
>>> + if (!tmp)
>>> + return -ENOMEM;
>>> + swap_digits((u64 *)private_key, (u64 *)tmp, 4);
>>> + p.key = tmp;
>>> + p.key_size = 32;
>>> + }
>>> +
>>> + buf_len = crypto_ecdh_key_len(&p);
>>> + buf = kmalloc(buf_len, GFP_KERNEL);
>>> + if (!buf) {
>>> + err = -ENOMEM;
>>> + goto free_tmp;
>>> + }
>>> +
>>> + err = crypto_ecdh_encode_key(buf, buf_len, &p);
>>> + if (err)
>>> + goto free_all;
>>> +
>>> + err = crypto_kpp_set_secret(tfm, buf, buf_len);
>>> + /* fall thr
Re: [v2 PATCH 0/5] Bluetooth: let the crypto subsystem generate the ecc privkey
Hi Tudor, > That Bluetooth SMP knows about the private key is pointless, since the > detection of debug key usage is actually via the public key portion. > With this patch set, the Bluetooth SMP will stop keeping a copy of the > ecdh private key. We let the crypto subsystem to generate and handle > the ecdh private key, potentially benefiting of hardware ecc private key > generation and retention. > > Tested with selftest and with btmon and smp-tester on top of hci_vhci, > with ecdh done in both software and hardware (through atmel-ecc driver). > All tests passed. > > RFC version can be found at: > https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg28036.html > > Changes in v2: > - add patches 2, 3, 4. > - adress Marcel's suggestions: > - revive the check for accidentally generated debug keys > - bypass the handling of private key to the crypto subsytem, >even when using debug keys. > > > Tudor Ambarus (5): > Bluetooth: move ecdh allocation outside of ecdh_helper > Bluetooth: ecdh_helper - reveal error codes > Bluetooth: selftest - check for errors when computing ZZ > Bluetooth: ecdh_helper - fix leak of private key > Bluetooth: let the crypto subsystem generate the ecc privkey > > net/bluetooth/ecdh_helper.c | 228 ++-- > net/bluetooth/ecdh_helper.h | 9 +- > net/bluetooth/selftest.c| 46 +++-- > net/bluetooth/smp.c | 127 +++- > 4 files changed, 240 insertions(+), 170 deletions(-) all 5 patches have been applied to bluetooth-next tree. Regards Marcel
[PATCH 2/3] crypto: dh_helper - return unsigned value for crypto_dh_key_len()
DH_KPP_SECRET_MIN_SIZE and dh_data_size() are both returning
unsigned values.
Signed-off-by: Tudor Ambarus
---
crypto/dh_helper.c | 2 +-
include/crypto/dh.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 69869da..a413b31 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -33,7 +33,7 @@ static inline unsigned int dh_data_size(const struct dh *p)
return p->key_size + p->p_size + p->g_size;
}
-int crypto_dh_key_len(const struct dh *p)
+unsigned int crypto_dh_key_len(const struct dh *p)
{
return DH_KPP_SECRET_MIN_SIZE + dh_data_size(p);
}
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index f638998..71e1bb2 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -53,7 +53,7 @@ struct dh {
*
* Return: size of the key in bytes
*/
-int crypto_dh_key_len(const struct dh *params);
+unsigned int crypto_dh_key_len(const struct dh *params);
/**
* crypto_dh_encode_key() - encode the private key
--
2.9.4
[PATCH 3/3] KEYS: dh - make some length variables unsigned
Both crypto_kpp_maxsize() and crypto_dh_key_len() are returning
unsigned integers.
Signed-off-by: Tudor Ambarus
---
security/keys/dh.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/keys/dh.c b/security/keys/dh.c
index d1ea9f3..89e9255 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -242,8 +242,8 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user
*params,
{
long ret;
ssize_t dlen;
- int secretlen;
- int outlen;
+ unsigned int secretlen;
+ unsigned int outlen;
struct keyctl_dh_params pcopy;
struct dh dh_inputs;
struct scatterlist outsg;
--
2.9.4
[PATCH 1/3] crypto: dh_helper - return unsigned int for dh_data_size()
p->key_size, p->p_size, p->g_size are all of unsigned int type.
Signed-off-by: Tudor Ambarus
---
crypto/dh_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 8ba8a3f..69869da 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -28,7 +28,7 @@ static inline const u8 *dh_unpack_data(void *dst, const void
*src, size_t size)
return src + size;
}
-static inline int dh_data_size(const struct dh *p)
+static inline unsigned int dh_data_size(const struct dh *p)
{
return p->key_size + p->p_size + p->g_size;
}
--
2.9.4
[PATCH] crypto: ecdh_helper - return unsigned value for crypto_ecdh_key_len()
ECDH_KPP_SECRET_MIN_SIZE and params->key_size are both returning
unsigned values.
Signed-off-by: Tudor Ambarus
---
crypto/ecdh_helper.c | 2 +-
include/crypto/ecdh.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/ecdh_helper.c b/crypto/ecdh_helper.c
index f05bea5..d3af8e8 100644
--- a/crypto/ecdh_helper.c
+++ b/crypto/ecdh_helper.c
@@ -28,7 +28,7 @@ static inline const u8 *ecdh_unpack_data(void *dst, const
void *src, size_t sz)
return src + sz;
}
-int crypto_ecdh_key_len(const struct ecdh *params)
+unsigned int crypto_ecdh_key_len(const struct ecdh *params)
{
return ECDH_KPP_SECRET_MIN_SIZE + params->key_size;
}
diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
index 1aff2a8..d696317 100644
--- a/include/crypto/ecdh.h
+++ b/include/crypto/ecdh.h
@@ -54,7 +54,7 @@ struct ecdh {
*
* Return: size of the key in bytes
*/
-int crypto_ecdh_key_len(const struct ecdh *params);
+unsigned int crypto_ecdh_key_len(const struct ecdh *params);
/**
* crypto_ecdh_encode_key() - encode the private key
--
2.9.4
Re: [PATCH] iommu/vt-d: Fix scatterlist offset handling
Robin,
I tried running patch on our test setup.
With "intel_iommu=on" : I can see single occurrence of DMAR Write failure on
perf traffic with 10 thread.
[ 749.616480] perf: interrupt took too long (3203 > 3202), lowering
kernel.perf_event_max_sample_rate to 62000
[ 852.500671] DMAR: DRHD: handling fault status reg 2
[ 852.506039] DMAR: [DMA Write] Request device [02:00.4] fault addr ef919000
[fault reason 05] PTE Write access is not set
[root@heptagon linux_t4_build]# cat /proc/cmdline
BOOT_IMAGE=/vmlinuz-4.9.51+ root=UUID=ccbb7f18-b3f0-43df-89de-07521e9c02fe ro
intel_iommu=on crashkernel=auto rhgb quiet rhgb quiet console=ttyS0,115200,
console=tty0 LANG=en_US.UTF-8
With intel_iommu=sp_off : It works fine for more than 30 minutes without any
issues.
On 28-09-2017 19:44, Robin Murphy wrote:
> The intel-iommu DMA ops fail to correctly handle scatterlists where
> sg->offset is greater than PAGE_SIZE - the IOVA allocation is computed
> appropriately based on the page-aligned portion of the offset, but the
> mapping is set up relative to sg->page, which means it fails to actually
> cover the whole buffer (and in the worst case doesn't cover it at all):
>
> (sg->dma_address + sg->dma_len) +
> sg->dma_address -+ |
> iov_pfn--+ | |
> | | |
> v v v
> iova: abcdef
> ||||||
> <...calculated>
> [_mapped__]
> pfn:012345
> ||||||
> ^ ^ ^
> | | |
> sg->page + | |
> sg->offset --+ |
> (sg->offset + sg->length) --+
>
> As a result, the caller ends up overrunning the mapping into whatever
> lies beyond, which usually goes badly:
>
> [ 429.645492] DMAR: DRHD: handling fault status reg 2
> [ 429.650847] DMAR: [DMA Write] Request device [02:00.4] fault addr f2682000
> ...
>
> Whilst this is a fairly rare occurrence, it can happen from the result
> of intermediate scatterlist processing such as scatterwalk_ffwd() in the
> crypto layer. Whilst that particular site could be fixed up, it still
> seems worthwhile to bring intel-iommu in line with other DMA API
> implementations in handling this robustly.
>
> To that end, fix the intel_map_sg() path to line up the mapping
> correctly (in units of MM pages rather than VT-d pages to match the
> aligned_nrpages() calculation) regardless of the offset, and use
> sg_phys() consistently for clarity.
>
> Reported-by: Harsh Jain
> Signed-off-by: Robin Murphy
> ---
> drivers/iommu/intel-iommu.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 6784a05dd6b2..83f3d4831f94 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -2254,10 +2254,12 @@ static int __domain_mapping(struct dmar_domain
> *domain, unsigned long iov_pfn,
> uint64_t tmp;
>
> if (!sg_res) {
> + unsigned int pgoff = sg->offset & ~PAGE_MASK;
> +
> sg_res = aligned_nrpages(sg->offset, sg->length);
> - sg->dma_address = ((dma_addr_t)iov_pfn <<
> VTD_PAGE_SHIFT) + sg->offset;
> + sg->dma_address = ((dma_addr_t)iov_pfn <<
> VTD_PAGE_SHIFT) + pgoff;
> sg->dma_length = sg->length;
> - pteval = page_to_phys(sg_page(sg)) | prot;
> + pteval = (sg_phys(sg) - pgoff) | prot;
> phys_pfn = pteval >> VTD_PAGE_SHIFT;
> }
>
> @@ -3790,7 +3792,7 @@ static int intel_nontranslate_map_sg(struct device
> *hddev,
>
> for_each_sg(sglist, sg, nelems, i) {
> BUG_ON(!sg_page(sg));
> - sg->dma_address = page_to_phys(sg_page(sg)) + sg->offset;
> + sg->dma_address = sg_phys(sg);
> sg->dma_length = sg->length;
> }
> return nelems;
