[PATCH] crypto: caam/qi - abort algorithm setup on DPAA2 parts
caam/qi frontend (i.e. caamalg_qi) mustn't be used in case it runs on a DPAA2 part (this could happen when using a multiplatform kernel). Fixes: 297b9cebd2fc ("crypto: caam/jr - add support for DPAA2 parts") Signed-off-by: Horia Geantă --- If this patch won't make it into v4.14 (likely the case), eventually it should be sent to -stable. drivers/crypto/caam/caamalg_qi.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c index 2eefc4a26bc2..f9f08fce4356 100644 --- a/drivers/crypto/caam/caamalg_qi.c +++ b/drivers/crypto/caam/caamalg_qi.c @@ -7,7 +7,7 @@ */ #include "compat.h" - +#include "ctrl.h" #include "regs.h" #include "intern.h" #include "desc_constr.h" @@ -2312,6 +2312,11 @@ static int __init caam_qi_algapi_init(void) if (!priv || !priv->qi_present) return -ENODEV; + if (caam_dpaa2) { + dev_info(ctrldev, "caam/qi frontend driver not suitable for DPAA 2.x, aborting...\n"); + return -ENODEV; + } + INIT_LIST_HEAD(&alg_list); /* -- 2.12.0.264.gd6db3f216544
[PATCH] crypto: caam - fix incorrect define
From: Radu Alexe Fixes: 3ebfa92f49a6 ("crypto: caam - Add new macros for building extended SEC descriptors (> 64 words)") Signed-off-by: Radu Alexe Signed-off-by: Horia Geantă --- drivers/crypto/caam/desc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/caam/desc.h b/drivers/crypto/caam/desc.h index 2e6766a1573f..2386d3e2d2c9 100644 --- a/drivers/crypto/caam/desc.h +++ b/drivers/crypto/caam/desc.h @@ -1439,7 +1439,7 @@ #define MATH_SRC1_REG2 (0x02 << MATH_SRC1_SHIFT) #define MATH_SRC1_REG3 (0x03 << MATH_SRC1_SHIFT) #define MATH_SRC1_IMM (0x04 << MATH_SRC1_SHIFT) -#define MATH_SRC1_DPOVRD (0x07 << MATH_SRC0_SHIFT) +#define MATH_SRC1_DPOVRD (0x07 << MATH_SRC1_SHIFT) #define MATH_SRC1_INFIFO (0x0a << MATH_SRC1_SHIFT) #define MATH_SRC1_OUTFIFO (0x0b << MATH_SRC1_SHIFT) #define MATH_SRC1_ONE (0x0c << MATH_SRC1_SHIFT) -- 2.12.0.264.gd6db3f216544
Re: Kernel panic when using ccm(aes) with the Atmel AES HW accelerator
On Mon, Oct 23, 2017 at 03:38:59PM +0300, Tudor Ambarus wrote: > > I will propose a fix, but I'm taking my time to better understand why > CTR requires to overwrite the iv with the last ciphertext block. That's an API requirement. So we should fix ccm. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] staging: ccree: Fix lines longer than 80 characters
On Mon, Oct 23, 2017 at 07:53:18AM -0700, Stephen Brennan wrote: > Simply break down some long lines and tab-indent them. Hi Stephen, Welcome to the Linux kernel. Great that you have put in a patch, you are, however, unlikely to see success fixing 'line over 80' warnings. There are a bunch of arguments for and against the line over 80 limit, a web search will surely show them to you. The TL;DR is that it these changes do not _really_ improve the readability of the code, they are just changes to quiet the static analysis tool. There are a bunch of other white space fixes that are more beneficial, perhaps you could wet your feet with these (incorrect placement of parenthesis for example). Good luck, Tobin.
[Part2 PATCH v6.1 20/38] crypto: ccp: Implement SEV_PDH_CERT_EXPORT ioctl command
The SEV_PDH_CERT_EXPORT command can be used to export the PDH and its certificate chain. The command is defined in SEV spec section 5.10. 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 --- Changes since v6: * when sev_do_cmd() and sev_platform_shutdown() fails then propogate the error status code from sev_do_cmd() because it can give us much better reason for the failure. drivers/crypto/ccp/psp-dev.c | 110 +++ 1 file changed, 110 insertions(+) diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index 108fc06bcdb3..b9f594cb10c1 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -390,6 +390,113 @@ static int sev_ioctl_do_pek_cert_import(struct sev_issue_cmd *argp) return ret; } +static int sev_ioctl_do_pdh_cert_export(struct sev_issue_cmd *argp) +{ + struct sev_user_data_pdh_cert_export input; + void *pdh_blob = NULL, *cert_blob = NULL; + struct sev_data_pdh_cert_export *data; + int ret, err; + + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) + return -EFAULT; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + /* Userspace wants to query the certificate length */ + if (!input.pdh_cert_address || !input.pdh_cert_len || + !input.cert_chain_address || !input.cert_chain_address) + goto cmd; + + /* allocate a physically contiguous buffer to store the PDH blob */ + if (!access_ok(VERIFY_WRITE, input.pdh_cert_address, input.pdh_cert_len) || + (input.pdh_cert_len > SEV_FW_BLOB_MAX_SIZE)) { + ret = -EFAULT; + goto e_free; + } + + pdh_blob = kmalloc(input.pdh_cert_len, GFP_KERNEL); + if (!pdh_blob) { + ret = -ENOMEM; + goto e_free; + } + + data->pdh_cert_address = __psp_pa(pdh_blob); + data->pdh_cert_len = input.pdh_cert_len; + + /* allocate a physically contiguous buffer to store the cert chain blob */ + if (!access_ok(VERIFY_WRITE, input.cert_chain_address, input.cert_chain_len) || + (input.cert_chain_len > SEV_FW_BLOB_MAX_SIZE)) { + ret = -EFAULT; + goto e_free_pdh; + } + + cert_blob = kmalloc(input.cert_chain_len, GFP_KERNEL); + if (!cert_blob) { + ret = -ENOMEM; + goto e_free_pdh; + } + + data->cert_chain_address = __psp_pa(cert_blob); + data->cert_chain_len = input.cert_chain_len; + +cmd: + ret = sev_platform_init(NULL, &argp->error); + if (ret) + goto e_free_cert; + + ret = sev_do_cmd(SEV_CMD_PDH_CERT_EXPORT, data, &argp->error); + + /* +* If we query the length, FW responded with expected data +*/ + input.cert_chain_len = data->cert_chain_len; + input.pdh_cert_len = data->pdh_cert_len; + + if (sev_platform_shutdown(&err)) { + /* +* If both sev_do_cmd() and sev_platform_shutdown() commands +* failed then propogate the error code from the sev_do_cmd() +* because it contains a useful status code for the command +* failure. +*/ + if (ret) + goto e_free_cert; + + ret = -EIO; + argp->error = err; + goto e_free_cert; + } + + if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) { + ret = -EFAULT; + goto e_free_cert; + } + + if (pdh_blob) { + if (copy_to_user((void __user *)input.pdh_cert_address, +pdh_blob, input.pdh_cert_len)) { + ret = -EFAULT; + goto e_free_cert; + } + } + + if (cert_blob) { + if (copy_to_user((void __user *)input.cert_chain_address, +cert_blob, input.cert_chain_len)) + ret = -EFAULT; + } + +e_free_cert: + kfree(cert_blob); +e_free_pdh: + kfree(pdh_blob); +e_free: + kfree(data); + return ret; +} + static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) { void __user *argp = (void __user *)arg; @@ -425,6 +532,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) case SEV_PEK_CERT_IMPORT: ret = sev_ioctl_do_pek_cert_import(&input); break; + case SEV_PDH_CERT_EXPORT: + ret = sev_ioctl_do_pdh_cert_export(&input); + break; default: ret = -EINVAL;
[Part2 PATCH v6.1 19/38] crypto: ccp: Implement SEV_PEK_CERT_IMPORT ioctl command
The SEV_PEK_CERT_IMPORT command can be used to import the signed PEK certificate. The command is defined in SEV spec section 5.8. 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 --- Changes since v6: * when sev_do_cmd() and sev_platform_shutdown() fails then propogate the error status code from sev_do_cmd() because it can give us much better reason for the failure. drivers/crypto/ccp/psp-dev.c | 92 include/linux/psp-sev.h | 4 ++ 2 files changed, 96 insertions(+) diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index aaf1c5cf821d..108fc06bcdb3 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -301,6 +301,95 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp) return ret; } +void *psp_copy_user_blob(u64 __user uaddr, u32 len) +{ + void *data; + + if (!uaddr || !len) + return ERR_PTR(-EINVAL); + + /* verify that blob length does not exceed our limit */ + if (len > SEV_FW_BLOB_MAX_SIZE) + return ERR_PTR(-EINVAL); + + data = kmalloc(len, GFP_KERNEL); + if (!data) + return ERR_PTR(-ENOMEM); + + if (copy_from_user(data, (void __user *)(uintptr_t)uaddr, len)) + goto e_free; + + return data; + +e_free: + kfree(data); + return ERR_PTR(-EFAULT); +} +EXPORT_SYMBOL_GPL(psp_copy_user_blob); + +static int sev_ioctl_do_pek_cert_import(struct sev_issue_cmd *argp) +{ + struct sev_user_data_pek_cert_import input; + struct sev_data_pek_cert_import *data; + void *pek_blob, *oca_blob; + int ret, err; + + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) + return -EFAULT; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + /* copy PEK certificate blobs from userspace */ + pek_blob = psp_copy_user_blob(input.pek_cert_address, input.pek_cert_len); + if (IS_ERR(pek_blob)) { + ret = PTR_ERR(pek_blob); + goto e_free; + } + + data->pek_cert_address = __psp_pa(pek_blob); + data->pek_cert_len = input.pek_cert_len; + + /* copy PEK certificate blobs from userspace */ + oca_blob = psp_copy_user_blob(input.oca_cert_address, input.oca_cert_len); + if (IS_ERR(oca_blob)) { + ret = PTR_ERR(oca_blob); + goto e_free_pek; + } + + data->oca_cert_address = __psp_pa(oca_blob); + data->oca_cert_len = input.oca_cert_len; + + ret = sev_platform_init(NULL, &argp->error); + if (ret) + goto e_free_oca; + + ret = sev_do_cmd(SEV_CMD_PEK_CERT_IMPORT, data, &argp->error); + + if (sev_platform_shutdown(&err)) { + /* +* If both sev_do_cmd() and sev_platform_shutdown() commands +* failed then propogate the error code from the sev_do_cmd() +* because it contains a useful status code for the command +* failure. +*/ + if (ret) + goto e_free_oca; + + ret = -EIO; + argp->error = err; + } + +e_free_oca: + kfree(oca_blob); +e_free_pek: + kfree(pek_blob); +e_free: + kfree(data); + return ret; +} + static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) { void __user *argp = (void __user *)arg; @@ -333,6 +422,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) case SEV_PEK_CSR: ret = sev_ioctl_do_pek_csr(&input); break; + case SEV_PEK_CERT_IMPORT: + ret = sev_ioctl_do_pek_cert_import(&input); + break; default: ret = -EINVAL; goto out; diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h index eac850a97610..d535153ca82d 100644 --- a/include/linux/psp-sev.h +++ b/include/linux/psp-sev.h @@ -620,6 +620,8 @@ int sev_guest_df_flush(int *error); */ int sev_guest_decommission(struct sev_data_decommission *data, int *error); +void *psp_copy_user_blob(u64 __user uaddr, u32 len); + #else /* !CONFIG_CRYPTO_DEV_SP_PSP */ static inline int @@ -648,6 +650,8 @@ sev_issue_cmd_external_user(struct file *filep, return -ENODEV; } +static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_PTR(-EINVAL); } + #endif /* CONFIG_CRYPTO_DEV_SP_PSP */ #endif /* __PSP_SEV_H__ */ -- 2.9.5
[Part2 PATCH v6.1 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command
The SEV_PEK_CSR command can be used to generate a PEK certificate signing request. The command is defined in SEV spec section 5.7. 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 --- Changes since v6: * when sev_do_cmd() and sev_platform_shutdown() fails then propogate the error status code from sev_do_cmd() because it can give us much better reason for the failure. drivers/crypto/ccp/psp-dev.c | 81 1 file changed, 81 insertions(+) diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index 3672435150cf..aaf1c5cf821d 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -223,6 +223,84 @@ static int sev_ioctl_do_pek_pdh_gen(int cmd, struct sev_issue_cmd *argp) return ret; } +static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp) +{ + struct sev_user_data_pek_csr input; + struct sev_data_pek_csr *data; + void *blob = NULL; + int ret, err; + + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) + return -EFAULT; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + /* userspace wants to query CSR length */ + if (!input.address || !input.length) + goto cmd; + + /* allocate a physically contiguous buffer to store the CSR blob */ + if (!access_ok(VERIFY_WRITE, input.address, input.length) || + input.length > SEV_FW_BLOB_MAX_SIZE) { + ret = -EFAULT; + goto e_free; + } + + blob = kmalloc(input.length, GFP_KERNEL); + if (!blob) { + ret = -ENOMEM; + goto e_free; + } + + data->address = __psp_pa(blob); + data->len = input.length; + +cmd: + ret = sev_platform_init(NULL, &argp->error); + if (ret) + goto e_free_blob; + + ret = sev_do_cmd(SEV_CMD_PEK_CSR, data, &argp->error); + + /* +* If we query the CSR length, FW responded with expected data +*/ + input.length = data->len; + + if (sev_platform_shutdown(&err)) { + /* +* If both sev_do_cmd() and sev_platform_shutdown() commands +* failed then propogate the error code from the sev_do_cmd() +* because it contains a useful status code for the command +* failure. +*/ + if (ret) + goto e_free_blob; + + ret = -EIO; + argp->error = err; + goto e_free_blob; + } + + if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) { + ret = -EFAULT; + goto e_free_blob; + } + + if (blob) { + if (copy_to_user((void __user *)input.address, blob, input.length)) + ret = -EFAULT; + } + +e_free_blob: + kfree(blob); +e_free: + kfree(data); + return ret; +} + static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) { void __user *argp = (void __user *)arg; @@ -252,6 +330,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) case SEV_PDH_GEN: ret = sev_ioctl_do_pek_pdh_gen(SEV_CMD_PDH_GEN, &input); break; + case SEV_PEK_CSR: + ret = sev_ioctl_do_pek_csr(&input); + break; default: ret = -EINVAL; goto out; -- 2.9.5
[Part2 PATCH v6.1 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command
The SEV_PEK_CSR command can be used to generate a PEK certificate signing request. The command is defined in SEV spec section 5.7. 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 --- Changes since v6: * when sev_do_cmd() and sev_platform_shutdown() fails then propogate the error status code from sev_do_cmd() because it can give us much better reason for the failure. drivers/crypto/ccp/psp-dev.c | 81 1 file changed, 81 insertions(+) diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index 3672435150cf..aaf1c5cf821d 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -223,6 +223,84 @@ static int sev_ioctl_do_pek_pdh_gen(int cmd, struct sev_issue_cmd *argp) return ret; } +static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp) +{ + struct sev_user_data_pek_csr input; + struct sev_data_pek_csr *data; + void *blob = NULL; + int ret, err; + + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) + return -EFAULT; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + /* userspace wants to query CSR length */ + if (!input.address || !input.length) + goto cmd; + + /* allocate a physically contiguous buffer to store the CSR blob */ + if (!access_ok(VERIFY_WRITE, input.address, input.length) || + input.length > SEV_FW_BLOB_MAX_SIZE) { + ret = -EFAULT; + goto e_free; + } + + blob = kmalloc(input.length, GFP_KERNEL); + if (!blob) { + ret = -ENOMEM; + goto e_free; + } + + data->address = __psp_pa(blob); + data->len = input.length; + +cmd: + ret = sev_platform_init(NULL, &argp->error); + if (ret) + goto e_free_blob; + + ret = sev_do_cmd(SEV_CMD_PEK_CSR, data, &argp->error); + + /* +* If we query the CSR length, FW responded with expected data +*/ + input.length = data->len; + + if (sev_platform_shutdown(&err)) { + /* +* If both sev_do_cmd() and sev_platform_shutdown() commands +* failed then propogate the error code from the sev_do_cmd() +* because it contains a useful status code for the command +* failure. +*/ + if (ret) + goto e_free_blob; + + ret = -EIO; + argp->error = err; + goto e_free_blob; + } + + if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) { + ret = -EFAULT; + goto e_free_blob; + } + + if (blob) { + if (copy_to_user((void __user *)input.address, blob, input.length)) + ret = -EFAULT; + } + +e_free_blob: + kfree(blob); +e_free: + kfree(data); + return ret; +} + static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) { void __user *argp = (void __user *)arg; @@ -252,6 +330,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) case SEV_PDH_GEN: ret = sev_ioctl_do_pek_pdh_gen(SEV_CMD_PDH_GEN, &input); break; + case SEV_PEK_CSR: + ret = sev_ioctl_do_pek_csr(&input); + break; default: ret = -EINVAL; goto out; -- 2.9.5
[Part2 PATCH v6.1 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command
The SEV_PEK_GEN command is used to generate a new Platform Endorsement Key (PEK). The command is defined in SEV spec section 5.6. 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 --- Changes since v6: * when sev_do_cmd() and sev_platform_shutdown() fails then propogate the error status code from sev_do_cmd() because it can give us much better reason for the failure. drivers/crypto/ccp/psp-dev.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index dd4bab143de9..18e2d8291997 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -195,6 +195,34 @@ static int sev_ioctl_do_platform_status(struct sev_issue_cmd *argp) return ret; } +static int sev_ioctl_do_pek_pdh_gen(int cmd, struct sev_issue_cmd *argp) +{ + int ret, err; + + ret = sev_platform_init(NULL, &argp->error); + if (ret) + return ret; + + ret = sev_do_cmd(cmd, 0, &argp->error); + + if (sev_platform_shutdown(&err)) { + /* +* If both sev_do_cmd() and sev_platform_shutdown() commands +* failed then propogate the error code from the sev_do_cmd() +* because it contains a useful status code for the command +* failure. +*/ + if (ret) + goto done; + + argp->error = err; + ret = -EIO; + } + +done: + return ret; +} + static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) { void __user *argp = (void __user *)arg; @@ -218,6 +246,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) case SEV_PLATFORM_STATUS: ret = sev_ioctl_do_platform_status(&input); break; + case SEV_PEK_GEN: + ret = sev_ioctl_do_pek_pdh_gen(SEV_CMD_PEK_GEN, &input); + break; default: ret = -EINVAL; goto out; -- 2.9.5
Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support
On 10/23/2017 02:34 AM, Borislav Petkov wrote: ... Just minor cleanups: Thanks Boris, I have applied your cleanups. -Brijesh --- diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index e9966d5fc6c4..f9a9a6e6ab99 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -31,7 +31,7 @@ static DEFINE_MUTEX(sev_cmd_mutex); static DEFINE_MUTEX(fw_init_mutex); -static struct sev_misc_dev *sev_misc_dev; +static struct sev_misc_dev *misc_dev; static int fw_init_count; static struct psp_device *psp_alloc_struct(struct sp_device *sp) @@ -299,14 +299,14 @@ static int sev_ops_init(struct psp_device *psp) * sev_do_cmd() finds the right master device to which to issue the * command to the firmware. */ - if (!sev_misc_dev) { + if (!misc_dev) { struct miscdevice *misc; - sev_misc_dev = devm_kzalloc(dev, sizeof(*sev_misc_dev), GFP_KERNEL); - if (!sev_misc_dev) + misc_dev = devm_kzalloc(dev, sizeof(*misc_dev), GFP_KERNEL); + if (!misc_dev) return -ENOMEM; - misc = &sev_misc_dev->misc; + misc = &misc_dev->misc; misc->minor = MISC_DYNAMIC_MINOR; misc->name = DEVICE_NAME; misc->fops = &sev_fops; @@ -315,13 +315,13 @@ static int sev_ops_init(struct psp_device *psp) if (ret) return ret; - kref_init(&sev_misc_dev->refcount); + kref_init(&misc_dev->refcount); } else { - kref_get(&sev_misc_dev->refcount); + kref_get(&misc_dev->refcount); } init_waitqueue_head(&psp->sev_int_queue); - psp->sev_misc = sev_misc_dev; + psp->sev_misc = misc_dev; dev_info(dev, "registered SEV device\n"); return 0; @@ -340,9 +340,9 @@ static int sev_init(struct psp_device *psp) static void sev_exit(struct kref *ref) { - struct sev_misc_dev *sev_dev = container_of(ref, struct sev_misc_dev, refcount); + struct sev_misc_dev *misc_dev = container_of(ref, struct sev_misc_dev, refcount); - misc_deregister(&sev_dev->misc); + misc_deregister(&misc_dev->misc); } int psp_dev_init(struct sp_device *sp) @@ -405,7 +405,7 @@ void psp_dev_destroy(struct sp_device *sp) struct psp_device *psp = sp->psp_data; if (psp->sev_misc) - kref_put(&sev_misc_dev->refcount, sev_exit); + kref_put(&misc_dev->refcount, sev_exit); sp_free_psp_irq(sp, psp); } diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h index 21511419bfe6..eac850a97610 100644 --- a/include/linux/psp-sev.h +++ b/include/linux/psp-sev.h @@ -525,7 +525,7 @@ int sev_platform_shutdown(int *error); /** * sev_platform_status - perform SEV PLATFORM_STATUS command * - * @init: sev_data_status structure to be processed + * @status: sev_user_data_status structure to be processed * @error: SEV command return code * * Returns:
Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command
On 10/23/2017 09:10 AM, Borislav Petkov wrote: On Mon, Oct 23, 2017 at 08:32:57AM -0500, Brijesh Singh wrote: If both the command fails then we return status from the last command. IIRC, in my previous patches I was returning status from sev_do_cmd() instead of sev_platform_shutdown() but based on our previous communication I thought you asked to return the status from the last failed command. Did I miss understood ? So my problem is that it looks strange that you save an error value from sev_do_cmd() but you don't look at it. And as I said in the other mail, you should either ignore it and say so in a comment why it is OK to ignore it or handle it but not overwrite it without looking at it. Does that make more sense? I see your point, if both commands failed then I am now inclined towards ignoring the error code from shutdown command and add some comments explaining why its OK. thanks -Brijesh
Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support
On 10/23/2017 04:20 AM, Borislav Petkov wrote: On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: +static int __sev_platform_init(struct sev_data_init *data, int *error) +{ + int rc = 0; + + mutex_lock(&fw_init_mutex); + + if (!fw_init_count) { I still don't like global semaphores. Can you get the status and check for PSTATE.INIT state and do the init only if the platform is in PSTATE.UNINIT? Calling PLATFORM_GET_STATUS is not required, we can manage the state through a simple ref count variable. Issuing PSP commands will always be much more expensive compare to accessing a protected global variable. I would prefer to avoid invoking PSP command if possible. Additionally, the global semaphore is still needed to serialize the sev_platform_init() and sev_platform_shutdown() from multiple processes. e.g If process "A" calls sev_platform_init() and if it gets preempted due to whatever reason then we don't want another process to issue the shutdown command while process "A" is in middle of sev_platform_init(). -Brijesh
Re: [tpmdd-devel] [PATCH] tpm: remove chip_num parameter from in-kernel API
On Mon, Oct 23, 2017 at 10:07:31AM -0400, Stefan Berger wrote: > >-int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) > >+int tpm_pcr_extend(int pcr_idx, const u8 *hash) > > { > > > I think every kernel internal TPM driver API should be called with the > tpm_chip as a parameter. This is in foresight of namespacing of IMA where we > want to provide the flexibility of passing a dedicated vTPM to each > namespace and IMA would use the chip as a parameter to all of these > functions to talk to the right tpm_vtpm_proxy instance. From that > perspective this patch goes into the wrong direction. Yes, we should ultimately try and get to there.. Someday the tpm_chip_find_get() will need to become namespace aware. But this patch is along the right path, eliminating the chip_num is the right thing to do.. > >-tpm2 = tpm_is_tpm2(TPM_ANY_NUM); > >+tpm2 = tpm_is_tpm2(); > > if (tpm2 < 0) > > return tpm2; > > > >@@ -1008,7 +1007,7 @@ static int trusted_instantiate(struct key *key, > > switch (key_cmd) { > > case Opt_load: > > if (tpm2) > >-ret = tpm_unseal_trusted(TPM_ANY_NUM, payload, options); > >+ret = tpm_unseal_trusted(payload, options); Sequences like this are sketchy. It should be struct tpm_chip *tpm; tpm = tpm_chip_find_get() tpm2 = tpm_is_tpm2(tpm); [..] if (tpm2) ret = tpm_unseal_trusted(tpm, payload, options); [..] tpm_put_chip(tpm); As hot plug could alter the 'tpm' between the two tpm calls. Jason
invalid opcode: 0000 [#1] SMP [aesni_intel]
Hi! Got the following kernel panic: invalid opcode: [#1] SMP CPU: 0 PID: 1449 Comm: openvpn Not tainted 4.8.13-1.el6.elrepo.x86_64 #1 cut Call Trace: [] ? enqueue_entity+0x45e/0x6f0 [] ? aesni_gcm_enc_avx+0x95/0xd0 [aesni_intel] [] helper_rfc4106_encrypt+0x167/0x2f0 [aesni_intel] [] rfc4106_encrypt+0x5b/0x90 [aesni_intel] cut The detailed bug report with full oops dump can be found here: https://bugzilla.kernel.org/show_bug.cgi?id=197363 Could not trigger this bug intentionally, but it happened three times already (all three dumps are available).
[PATCH 3/3] crypto: atmel-aes/tdes/sha - remove useless irq init
irq would be set to -1 and then unused, if we failed to get IORESOURCE_MEM. Signed-off-by: Tudor Ambarus --- drivers/crypto/atmel-aes.c | 2 -- drivers/crypto/atmel-sha.c | 2 -- drivers/crypto/atmel-tdes.c | 2 -- 3 files changed, 6 deletions(-) diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c index bc859ab..2048292 100644 --- a/drivers/crypto/atmel-aes.c +++ b/drivers/crypto/atmel-aes.c @@ -2644,8 +2644,6 @@ static int atmel_aes_probe(struct platform_device *pdev) crypto_init_queue(&aes_dd->queue, ATMEL_AES_QUEUE_LENGTH); - aes_dd->irq = -1; - /* Get the base address */ aes_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!aes_res) { diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c index 42c1f74..8874aa5 100644 --- a/drivers/crypto/atmel-sha.c +++ b/drivers/crypto/atmel-sha.c @@ -2777,8 +2777,6 @@ static int atmel_sha_probe(struct platform_device *pdev) crypto_init_queue(&sha_dd->queue, ATMEL_SHA_QUEUE_LENGTH); - sha_dd->irq = -1; - /* Get the base address */ sha_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!sha_res) { diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c index 48e8653..592124f 100644 --- a/drivers/crypto/atmel-tdes.c +++ b/drivers/crypto/atmel-tdes.c @@ -1363,8 +1363,6 @@ static int atmel_tdes_probe(struct platform_device *pdev) crypto_init_queue(&tdes_dd->queue, ATMEL_TDES_QUEUE_LENGTH); - tdes_dd->irq = -1; - /* Get the base address */ tdes_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!tdes_res) { -- 2.9.4
[PATCH 1/3] crypto: atmel-aes/tdes/sha - return appropriate error code
Return -ENODEV when dma_request_slave_channel_compat() fails. Signed-off-by: Tudor Ambarus --- drivers/crypto/atmel-aes.c | 3 +-- drivers/crypto/atmel-sha.c | 3 +-- drivers/crypto/atmel-tdes.c | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c index 903fd43..9659759 100644 --- a/drivers/crypto/atmel-aes.c +++ b/drivers/crypto/atmel-aes.c @@ -2383,7 +2383,6 @@ static int atmel_aes_dma_init(struct atmel_aes_dev *dd, struct crypto_platform_data *pdata) { struct at_dma_slave *slave; - int err = -ENOMEM; dma_cap_mask_t mask; dma_cap_zero(mask); @@ -2408,7 +2407,7 @@ static int atmel_aes_dma_init(struct atmel_aes_dev *dd, dma_release_channel(dd->src.chan); err_dma_in: dev_warn(dd->dev, "no DMA channel available\n"); - return err; + return -ENODEV; } static void atmel_aes_dma_cleanup(struct atmel_aes_dev *dd) diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c index 3e2f41b..42c1f74 100644 --- a/drivers/crypto/atmel-sha.c +++ b/drivers/crypto/atmel-sha.c @@ -2628,7 +2628,6 @@ static bool atmel_sha_filter(struct dma_chan *chan, void *slave) static int atmel_sha_dma_init(struct atmel_sha_dev *dd, struct crypto_platform_data *pdata) { - int err = -ENOMEM; dma_cap_mask_t mask_in; /* Try to grab DMA channel */ @@ -2639,7 +2638,7 @@ static int atmel_sha_dma_init(struct atmel_sha_dev *dd, atmel_sha_filter, &pdata->dma_slave->rxdata, dd->dev, "tx"); if (!dd->dma_lch_in.chan) { dev_warn(dd->dev, "no DMA channel available\n"); - return err; + return -ENODEV; } dd->dma_lch_in.dma_conf.direction = DMA_MEM_TO_DEV; diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c index f4b335d..0ece4b8 100644 --- a/drivers/crypto/atmel-tdes.c +++ b/drivers/crypto/atmel-tdes.c @@ -720,7 +720,6 @@ static bool atmel_tdes_filter(struct dma_chan *chan, void *slave) static int atmel_tdes_dma_init(struct atmel_tdes_dev *dd, struct crypto_platform_data *pdata) { - int err = -ENOMEM; dma_cap_mask_t mask; dma_cap_zero(mask); @@ -765,7 +764,7 @@ static int atmel_tdes_dma_init(struct atmel_tdes_dev *dd, dma_release_channel(dd->dma_lch_in.chan); err_dma_in: dev_warn(dd->dev, "no DMA channel available\n"); - return err; + return -ENODEV; } static void atmel_tdes_dma_cleanup(struct atmel_tdes_dev *dd) -- 2.9.4
[PATCH 2/3] crypto: atmel-aes/tdes - remove empty function
This empty function was used to initialize a member of a static structure. Pointer members of an object with static storage duration, if not explicitly initialized, will be initialized to a NULL pointer. The crypto API checks if this pointer is not NULL before using it, so we are safe to remove this empty function along with all the references to it. Signed-off-by: Tudor Ambarus --- drivers/crypto/atmel-aes.c | 14 -- drivers/crypto/atmel-tdes.c | 18 -- 2 files changed, 32 deletions(-) diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c index 9659759..bc859ab 100644 --- a/drivers/crypto/atmel-aes.c +++ b/drivers/crypto/atmel-aes.c @@ -1237,10 +1237,6 @@ static int atmel_aes_ctr_cra_init(struct crypto_tfm *tfm) return 0; } -static void atmel_aes_cra_exit(struct crypto_tfm *tfm) -{ -} - static struct crypto_alg aes_algs[] = { { .cra_name = "ecb(aes)", @@ -1253,7 +1249,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1273,7 +1268,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1294,7 +1288,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1315,7 +1308,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1336,7 +1328,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1357,7 +1348,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1378,7 +1368,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1399,7 +1388,6 @@ static struct crypto_alg aes_algs[] = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_ctr_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1422,7 +1410,6 @@ static struct crypto_alg aes_cfb64_alg = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= AES_MIN_KEY_SIZE, .max_keysize= AES_MAX_KEY_SIZE, @@ -1956,7 +1943,6 @@ static struct crypto_alg aes_xts_alg = { .cra_type = &crypto_ablkcipher_type, .cra_module = THIS_MODULE, .cra_init = atmel_aes_xts_cra_init, - .cra_exit = atmel_aes_cra_exit, .cra_u.ablkcipher = { .min_keysize= 2 * AES_M
Re: [PATCH] staging: ccree: Fix lines longer than 80 characters
Hi Gilad, Thanks for the quick reply, I really appreciate your taking time to help a newbie get started. I've made the appropriate changes and re-submitted. > TIP: if you run the scripts/get_maintainers.pl script on your patch it > will tell you exactly which > list and which people your patch needs to be addressed, so you don't > have to guess. When I ran this tool, it listed out quite a few mailing lists, including linux-ker...@vger.kernel.org. Is it correct to simply address your patch to the whole list output by the script? I omitted linux-kernel on my resubmission, simply to avoid contributing to the heavy volume of that list, given how trivial this patch is. Thanks again! Stephen
[PATCH] staging: ccree: Fix lines longer than 80 characters
Simply break down some long lines and tab-indent them. Signed-off-by: Stephen Brennan --- I'm learning the patch submission process, and this is my first patch. I know it's trivial but I'm just trying to get my feet wet. Thanks in advance for taking the time to review this! drivers/staging/ccree/ssi_pm.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ccree/ssi_pm.c b/drivers/staging/ccree/ssi_pm.c index 11bbdbeec22e..98ba9e918d2a 100644 --- a/drivers/staging/ccree/ssi_pm.c +++ b/drivers/staging/ccree/ssi_pm.c @@ -41,7 +41,9 @@ int ssi_power_mgr_runtime_suspend(struct device *dev) int rc; dev_dbg(dev, "set HOST_POWER_DOWN_EN\n"); - WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE); + WRITE_REGISTER( + drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), + POWER_DOWN_ENABLE); rc = ssi_request_mgr_runtime_suspend_queue(drvdata); if (rc != 0) { dev_err(dev, "ssi_request_mgr_runtime_suspend_queue (%x)\n", @@ -60,7 +62,9 @@ int ssi_power_mgr_runtime_resume(struct device *dev) (struct ssi_drvdata *)dev_get_drvdata(dev); dev_dbg(dev, "unset HOST_POWER_DOWN_EN\n"); - WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_DISABLE); + WRITE_REGISTER( + drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), + POWER_DOWN_DISABLE); rc = cc_clk_on(drvdata); if (rc) { -- 2.14.2
Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command
On Mon, Oct 23, 2017 at 08:32:57AM -0500, Brijesh Singh wrote: > If both the command fails then we return status from the last command. > IIRC, in my previous patches I was returning status from sev_do_cmd() > instead of sev_platform_shutdown() but based on our previous > communication I thought you asked to return the status from the last > failed command. Did I miss understood ? So my problem is that it looks strange that you save an error value from sev_do_cmd() but you don't look at it. And as I said in the other mail, you should either ignore it and say so in a comment why it is OK to ignore it or handle it but not overwrite it without looking at it. Does that make more sense? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [tpmdd-devel] [PATCH] tpm: remove chip_num parameter from in-kernel API
On 10/23/2017 08:38 AM, Jarkko Sakkinen wrote: The reasoning is simple and obvious. Since every call site passes the value TPM_ANY_NUM (0x) the parameter does not have right to exist. Refined the documentation of the corresponding functions. Signed-off-by: Jarkko Sakkinen --- drivers/char/hw_random/tpm-rng.c| 2 +- drivers/char/tpm/tpm-chip.c | 38 drivers/char/tpm/tpm-interface.c| 87 ++--- drivers/char/tpm/tpm.h | 2 +- include/linux/tpm.h | 43 -- security/integrity/ima/ima_crypto.c | 2 +- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_queue.c | 2 +- security/keys/trusted.c | 35 --- 9 files changed, 101 insertions(+), 112 deletions(-) diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c index d6d448266f07..8823efcddab8 100644 --- a/drivers/char/hw_random/tpm-rng.c +++ b/drivers/char/hw_random/tpm-rng.c @@ -25,7 +25,7 @@ static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) { - return tpm_get_random(TPM_ANY_NUM, data, max); + return tpm_get_random(data, max); } static struct hwrng tpm_rng = { diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index a114e8f7fb90..ec35643b 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -81,34 +81,32 @@ void tpm_put_ops(struct tpm_chip *chip) EXPORT_SYMBOL_GPL(tpm_put_ops); /** - * tpm_chip_find_get() - return tpm_chip for a given chip number - * @chip_num: id to find + * tpm_chip_find_get() - reserved the first available TPM chip * - * The return'd chip has been tpm_try_get_ops'd and must be released via - * tpm_put_ops + * Finds the first available TPM chip and reserves its class device and + * operations. + * + * Return: a reserved &struct tpm_chip instance */ -struct tpm_chip *tpm_chip_find_get(int chip_num) +struct tpm_chip *tpm_chip_find_get(void) { - struct tpm_chip *chip, *res = NULL; + struct tpm_chip *chip; + struct tpm_chip *res = NULL; int chip_prev; + int chip_num; mutex_lock(&idr_lock); - if (chip_num == TPM_ANY_NUM) { - chip_num = 0; - do { - chip_prev = chip_num; - chip = idr_get_next(&dev_nums_idr, &chip_num); - if (chip && !tpm_try_get_ops(chip)) { - res = chip; - break; - } - } while (chip_prev != chip_num); - } else { - chip = idr_find(&dev_nums_idr, chip_num); - if (chip && !tpm_try_get_ops(chip)) + chip_num = 0; + + do { + chip_prev = chip_num; + chip = idr_get_next(&dev_nums_idr, &chip_num); + if (chip && !tpm_try_get_ops(chip)) { res = chip; - } + break; + } + } while (chip_prev != chip_num); mutex_unlock(&idr_lock); Here you are keeping the loop, which I think is good and I would like you to keep it... diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index d8e2e5bca903..b3907d3556ce 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -802,18 +802,19 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf) } /** - * tpm_is_tpm2 - is the chip a TPM2 chip? - * @chip_num: tpm idx # or ANY + * tpm_is_tpm2 - do we a have a TPM2 chip? * - * Returns < 0 on error, and 1 or 0 on success depending whether the chip - * is a TPM2 chip. + * Return: + * 1 if we have a TPM2 chip. + * 0 if we don't have a TPM2 chip. + * A negative number for system errors (errno). */ -int tpm_is_tpm2(u32 chip_num) +int tpm_is_tpm2(void) { struct tpm_chip *chip; int rc; - chip = tpm_chip_find_get(chip_num); + chip = tpm_chip_find_get(); if (chip == NULL) return -ENODEV; @@ -826,22 +827,18 @@ int tpm_is_tpm2(u32 chip_num) EXPORT_SYMBOL_GPL(tpm_is_tpm2); /** - * tpm_pcr_read - read a pcr value - * @chip_num: tpm idx # or ANY - * @pcr_idx: pcr idx to retrieve - * @res_buf: TPM_PCR value - * size of res_buf is 20 bytes (or NULL if you don't care) + * tpm_pcr_read - read a PCR value from SHA1 bank + * @pcr_idx: the PCR to be retrieved + * @res_buf: the value of the PCR * - * The TPM driver should be built-in, but for whatever reason it - * isn't, protect against the chip disappearing, by incrementing - * the module usage count. + * Return: same as with tpm_transmit_cmd() */ -int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) +int tpm_pcr_read(int pcr_idx, u8 *res_buf) { struct tpm_chip *chip; int rc; - chip = tpm_chip_find_get(chip_num
Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command
On 10/23/17 7:32 AM, Borislav Petkov wrote: > On Mon, Oct 23, 2017 at 07:15:30AM -0500, Brijesh Singh wrote: >> I am not sure if I am able to understand your feedback. The >> sev_platform_shutdown() is called unconditionally. > How's that: > > If sev_do_cmd() fails and sev_do_cmd(SEV_CMD_SHUTDOWN, ...) in > sev_platform_shutdown() fails, then the first ret from sev_do_cmd() is > gone. If both the command fails then we return status from the last command. IIRC, in my previous patches I was returning status from sev_do_cmd() instead of sev_platform_shutdown() but based on our previous communication I thought you asked to return the status from the last failed command. Did I miss understood ? -Brijesh
Re: [Part2 PATCH v6 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command
On Thu, Oct 19, 2017 at 09:33:53PM -0500, Brijesh Singh wrote: > The SEV_PEK_CSR command can be used to generate a PEK certificate > signing request. The command is defined in SEV spec section 5.7. > > 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/psp-dev.c | 69 > > 1 file changed, 69 insertions(+) Improvements-by: Borislav Petkov > +static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp) > +{ > + struct sev_user_data_pek_csr input; > + struct sev_data_pek_csr *data; > + void *blob = NULL; > + int ret, err; > + > + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) > + return -EFAULT; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + /* userspace wants to query CSR length */ > + if (!input.address || !input.length) > + goto cmd; > + > + /* allocate a physically contiguous buffer to store the CSR blob */ > + if (!access_ok(VERIFY_WRITE, input.address, input.length) || > + input.length > SEV_FW_BLOB_MAX_SIZE) { > + ret = -EFAULT; > + goto e_free; > + } > + > + blob = kmalloc(input.length, GFP_KERNEL); > + if (!blob) { > + ret = -ENOMEM; > + goto e_free; > + } > + > + data->address = __psp_pa(blob); > + data->len = input.length; > + > +cmd: > + ret = sev_platform_init(NULL, &argp->error); > + if (ret) > + goto e_free_blob; > + > + ret = sev_do_cmd(SEV_CMD_PEK_CSR, data, &argp->error); This is the same issue: nothing is handling the case where sev_do_cmd() here fails. If it doesn't matter, then you should simply do: sev_do_cmd(SEV_CMD_PEK_CSR, data, &argp->error); along with explaining why we don't care about the command failing. But simply writing into ret to overwrite it later, looks strange. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: Kernel panic when using ccm(aes) with the Atmel AES HW accelerator
Hi, Romain, On 10/18/2017 04:32 PM, Romain Izard wrote: my fix also led to a systematic oops when running the ccm(aes) test case. The NULL deference appears because of a memory corruption issue. atmel-aes does not implement ccm(aes), so the algorithm will be in the following form: ccm_base(atmel-ctr-aes,cbcmac(aes-generic)). ccm auth uses the first byte of the IV as length and eventually will memset memory to zero based on that length (see set_msg_len()). CTR overwrites the iv with the last ciphertext block and the length will be wrong. I will propose a fix, but I'm taking my time to better understand why CTR requires to overwrite the iv with the last ciphertext block. Cheers, ta
[PATCH] tpm: remove chip_num parameter from in-kernel API
The reasoning is simple and obvious. Since every call site passes the value TPM_ANY_NUM (0x) the parameter does not have right to exist. Refined the documentation of the corresponding functions. Signed-off-by: Jarkko Sakkinen --- drivers/char/hw_random/tpm-rng.c| 2 +- drivers/char/tpm/tpm-chip.c | 38 drivers/char/tpm/tpm-interface.c| 87 ++--- drivers/char/tpm/tpm.h | 2 +- include/linux/tpm.h | 43 -- security/integrity/ima/ima_crypto.c | 2 +- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_queue.c | 2 +- security/keys/trusted.c | 35 --- 9 files changed, 101 insertions(+), 112 deletions(-) diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c index d6d448266f07..8823efcddab8 100644 --- a/drivers/char/hw_random/tpm-rng.c +++ b/drivers/char/hw_random/tpm-rng.c @@ -25,7 +25,7 @@ static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) { - return tpm_get_random(TPM_ANY_NUM, data, max); + return tpm_get_random(data, max); } static struct hwrng tpm_rng = { diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index a114e8f7fb90..ec35643b 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -81,34 +81,32 @@ void tpm_put_ops(struct tpm_chip *chip) EXPORT_SYMBOL_GPL(tpm_put_ops); /** - * tpm_chip_find_get() - return tpm_chip for a given chip number - * @chip_num: id to find + * tpm_chip_find_get() - reserved the first available TPM chip * - * The return'd chip has been tpm_try_get_ops'd and must be released via - * tpm_put_ops + * Finds the first available TPM chip and reserves its class device and + * operations. + * + * Return: a reserved &struct tpm_chip instance */ -struct tpm_chip *tpm_chip_find_get(int chip_num) +struct tpm_chip *tpm_chip_find_get(void) { - struct tpm_chip *chip, *res = NULL; + struct tpm_chip *chip; + struct tpm_chip *res = NULL; int chip_prev; + int chip_num; mutex_lock(&idr_lock); - if (chip_num == TPM_ANY_NUM) { - chip_num = 0; - do { - chip_prev = chip_num; - chip = idr_get_next(&dev_nums_idr, &chip_num); - if (chip && !tpm_try_get_ops(chip)) { - res = chip; - break; - } - } while (chip_prev != chip_num); - } else { - chip = idr_find(&dev_nums_idr, chip_num); - if (chip && !tpm_try_get_ops(chip)) + chip_num = 0; + + do { + chip_prev = chip_num; + chip = idr_get_next(&dev_nums_idr, &chip_num); + if (chip && !tpm_try_get_ops(chip)) { res = chip; - } + break; + } + } while (chip_prev != chip_num); mutex_unlock(&idr_lock); diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index d8e2e5bca903..b3907d3556ce 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -802,18 +802,19 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf) } /** - * tpm_is_tpm2 - is the chip a TPM2 chip? - * @chip_num: tpm idx # or ANY + * tpm_is_tpm2 - do we a have a TPM2 chip? * - * Returns < 0 on error, and 1 or 0 on success depending whether the chip - * is a TPM2 chip. + * Return: + * 1 if we have a TPM2 chip. + * 0 if we don't have a TPM2 chip. + * A negative number for system errors (errno). */ -int tpm_is_tpm2(u32 chip_num) +int tpm_is_tpm2(void) { struct tpm_chip *chip; int rc; - chip = tpm_chip_find_get(chip_num); + chip = tpm_chip_find_get(); if (chip == NULL) return -ENODEV; @@ -826,22 +827,18 @@ int tpm_is_tpm2(u32 chip_num) EXPORT_SYMBOL_GPL(tpm_is_tpm2); /** - * tpm_pcr_read - read a pcr value - * @chip_num: tpm idx # or ANY - * @pcr_idx: pcr idx to retrieve - * @res_buf: TPM_PCR value - * size of res_buf is 20 bytes (or NULL if you don't care) + * tpm_pcr_read - read a PCR value from SHA1 bank + * @pcr_idx: the PCR to be retrieved + * @res_buf: the value of the PCR * - * The TPM driver should be built-in, but for whatever reason it - * isn't, protect against the chip disappearing, by incrementing - * the module usage count. + * Return: same as with tpm_transmit_cmd() */ -int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) +int tpm_pcr_read(int pcr_idx, u8 *res_buf) { struct tpm_chip *chip; int rc; - chip = tpm_chip_find_get(chip_num); + chip = tpm_chip_find_get(); if (chip == NULL) return -ENODEV; if (chip->flags & TPM_CHIP_FLAG_TPM2) @@ -882,16 +879,17
Re: [Part2 PATCH v6 17/38] crypto: ccp: Implement SEV_PDH_GEN ioctl command
On Thu, Oct 19, 2017 at 09:33:52PM -0500, Brijesh Singh wrote: > The SEV_PDH_GEN command is used to re-generate the Platform > Diffie-Hellman (PDH) key. The command is defined in SEV spec section > 5.6. > > 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/psp-dev.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Borislav Petkov -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command
On Mon, Oct 23, 2017 at 07:15:30AM -0500, Brijesh Singh wrote: > I am not sure if I am able to understand your feedback. The > sev_platform_shutdown() is called unconditionally. How's that: If sev_do_cmd() fails and sev_do_cmd(SEV_CMD_SHUTDOWN, ...) in sev_platform_shutdown() fails, then the first ret from sev_do_cmd() is gone. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command
On 10/23/17 4:32 AM, Borislav Petkov wrote: ... >> +static int sev_ioctl_do_pek_pdh_gen(int cmd, struct sev_issue_cmd *argp) >> +{ >> +int ret, err; >> + >> +ret = sev_platform_init(NULL, &argp->error); >> +if (ret) >> +return ret; >> + >> +ret = sev_do_cmd(cmd, 0, &argp->error); > So this ret value gets potentially overwritten here. You need > to either handle the case properly when sev_do_cmd() fails and > sev_platform_shutdown() gets to issue SEV_CMD_SHUTDOWN (i.e., when it > gets overwritten), or not write into ret at all by initializing it to 0 > at function entry. > I am not sure if I am able to understand your feedback. The sev_platform_shutdown() is called unconditionally. 1) if sev_do_cmd() fails and sev_platform_shutdown() was success then 'ret' will contain the error code from sev_do_cmd(). 2) if sev_do_cmd() was success but sev_platform_shutdown() fails then 'ret' will contain the error code from sev_platform_shutdown() 3) if both sev_do_cmd() and sev_platform_shutdown() fails then 'ret' will contain error code from the sev_platform_shutdown().
Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command
On Thu, Oct 19, 2017 at 09:33:51PM -0500, Brijesh Singh wrote: > The SEV_PEK_GEN command is used to generate a new Platform Endorsement > Key (PEK). The command is defined in SEV spec section 5.6. > > 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/psp-dev.c | 21 + > 1 file changed, 21 insertions(+) > > diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c > index 5c921b36bc23..1d7212da25a5 100644 > --- a/drivers/crypto/ccp/psp-dev.c > +++ b/drivers/crypto/ccp/psp-dev.c > @@ -195,6 +195,24 @@ static int sev_ioctl_do_platform_status(struct > sev_issue_cmd *argp) > return ret; > } > > +static int sev_ioctl_do_pek_pdh_gen(int cmd, struct sev_issue_cmd *argp) > +{ > + int ret, err; > + > + ret = sev_platform_init(NULL, &argp->error); > + if (ret) > + return ret; > + > + ret = sev_do_cmd(cmd, 0, &argp->error); So this ret value gets potentially overwritten here. You need to either handle the case properly when sev_do_cmd() fails and sev_platform_shutdown() gets to issue SEV_CMD_SHUTDOWN (i.e., when it gets overwritten), or not write into ret at all by initializing it to 0 at function entry. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > +static int __sev_platform_init(struct sev_data_init *data, int *error) > +{ > + int rc = 0; > + > + mutex_lock(&fw_init_mutex); > + > + if (!fw_init_count) { I still don't like global semaphores. Can you get the status and check for PSTATE.INIT state and do the init only if the platform is in PSTATE.UNINIT? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [Part2 PATCH v6 15/38] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command
On Thu, Oct 19, 2017 at 09:33:50PM -0500, Brijesh Singh wrote: > The SEV_PLATFORM_STATUS command can be used by the platform owner to > get the current status of the platform. The command is defined in > SEV spec section 5.5. > > 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 > Improvements-by: Borislav Petkov > Signed-off-by: Brijesh Singh > --- > drivers/crypto/ccp/psp-dev.c | 24 > 1 file changed, 24 insertions(+) Reviewed-by: Borislav Petkov -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [Part2 PATCH v6 14/38] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command
On Thu, Oct 19, 2017 at 09:33:49PM -0500, Brijesh Singh wrote: > The SEV_FACTORY_RESET command can be used by the platform owner to > reset the non-volatile SEV related data. The command is defined in > SEV spec section 5.4 > > 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 > Improvements-by: Borislav Petkov > Signed-off-by: Brijesh Singh > --- > drivers/crypto/ccp/psp-dev.c | 28 +++- > 1 file changed, 27 insertions(+), 1 deletion(-) Reviewed-by: Borislav Petkov -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > AMD's new Secure Encrypted Virtualization (SEV) feature allows the > memory contents of virtual machines to be transparently encrypted with a > key unique to the VM. The programming and management of the encryption > keys are handled by the AMD Secure Processor (AMD-SP) which exposes the > commands for these tasks. The complete spec is available at: > > http://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf > > Extend the AMD-SP driver to provide the following support: > > - an in-kernel API to communicate with the SEV firmware. The API can be >used by the hypervisor to create encryption context for a SEV guest. > > - a userspace IOCTL to manage the platform certificates. Just minor cleanups: --- diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index e9966d5fc6c4..f9a9a6e6ab99 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -31,7 +31,7 @@ static DEFINE_MUTEX(sev_cmd_mutex); static DEFINE_MUTEX(fw_init_mutex); -static struct sev_misc_dev *sev_misc_dev; +static struct sev_misc_dev *misc_dev; static int fw_init_count; static struct psp_device *psp_alloc_struct(struct sp_device *sp) @@ -299,14 +299,14 @@ static int sev_ops_init(struct psp_device *psp) * sev_do_cmd() finds the right master device to which to issue the * command to the firmware. */ - if (!sev_misc_dev) { + if (!misc_dev) { struct miscdevice *misc; - sev_misc_dev = devm_kzalloc(dev, sizeof(*sev_misc_dev), GFP_KERNEL); - if (!sev_misc_dev) + misc_dev = devm_kzalloc(dev, sizeof(*misc_dev), GFP_KERNEL); + if (!misc_dev) return -ENOMEM; - misc = &sev_misc_dev->misc; + misc = &misc_dev->misc; misc->minor = MISC_DYNAMIC_MINOR; misc->name = DEVICE_NAME; misc->fops = &sev_fops; @@ -315,13 +315,13 @@ static int sev_ops_init(struct psp_device *psp) if (ret) return ret; - kref_init(&sev_misc_dev->refcount); + kref_init(&misc_dev->refcount); } else { - kref_get(&sev_misc_dev->refcount); + kref_get(&misc_dev->refcount); } init_waitqueue_head(&psp->sev_int_queue); - psp->sev_misc = sev_misc_dev; + psp->sev_misc = misc_dev; dev_info(dev, "registered SEV device\n"); return 0; @@ -340,9 +340,9 @@ static int sev_init(struct psp_device *psp) static void sev_exit(struct kref *ref) { - struct sev_misc_dev *sev_dev = container_of(ref, struct sev_misc_dev, refcount); + struct sev_misc_dev *misc_dev = container_of(ref, struct sev_misc_dev, refcount); - misc_deregister(&sev_dev->misc); + misc_deregister(&misc_dev->misc); } int psp_dev_init(struct sp_device *sp) @@ -405,7 +405,7 @@ void psp_dev_destroy(struct sp_device *sp) struct psp_device *psp = sp->psp_data; if (psp->sev_misc) - kref_put(&sev_misc_dev->refcount, sev_exit); + kref_put(&misc_dev->refcount, sev_exit); sp_free_psp_irq(sp, psp); } diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h index 21511419bfe6..eac850a97610 100644 --- a/include/linux/psp-sev.h +++ b/include/linux/psp-sev.h @@ -525,7 +525,7 @@ int sev_platform_shutdown(int *error); /** * sev_platform_status - perform SEV PLATFORM_STATUS command * - * @init: sev_data_status structure to be processed + * @status: sev_user_data_status structure to be processed * @error: SEV command return code * * Returns: -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --