Re: [PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver
Hi Stephan, On Mon, Apr 24, 2017 at 02:59:05PM +0200, Stephan Müller wrote: > Am Montag, 24. April 2017, 09:54:06 CEST schrieb Antoine Tenart: > > > +struct safexcel_cipher_ctx { > > + struct safexcel_context base; > > + struct safexcel_crypto_priv *priv; > > + > > + enum safexcel_cipher_direction direction; > > + u32 mode; > > + > > + __le32 key[8]; > > Can you please help me find the location where this memory is zeroized when > released? It's not, I'll fix this. > > +static void safexcel_cipher_token(struct safexcel_cipher_ctx *ctx, > > + struct crypto_async_request *async, > > + struct safexcel_command_desc *cdesc, > > + u32 length) > > +{ > > + struct ablkcipher_request *req = ablkcipher_request_cast(async); > > + struct safexcel_token *token; > > + unsigned offset = 0; > > + > > + if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) { > > + offset = AES_BLOCK_SIZE / sizeof(u32); > > + memcpy(cdesc->control_data.token, req->info, AES_BLOCK_SIZE); > > + > > + cdesc->control_data.options |= EIP197_OPTION_4_TOKEN_IV_CMD; > > + } > > + > > + token = (struct safexcel_token *)(cdesc->control_data.token + offset); > > + > > + token[0].opcode = EIP197_TOKEN_OPCODE_DIRECTION; > > + token[0].packet_length = length; > > + token[0].stat = EIP197_TOKEN_STAT_LAST_PACKET; > > + token[0].instructions = EIP197_TOKEN_INS_LAST | > > + EIP197_TOKEN_INS_TYPE_CRYTO | > > + EIP197_TOKEN_INS_TYPE_OUTPUT; > > +} > > + > > +static int safexcel_aes_setkey(struct crypto_ablkcipher *ctfm, const u8 > > *key, +unsigned int len) > > +{ > > You still use ablkcipher. I thought that it is on its way out in favor of the > skcipher API. Why do you stick to ablkcipher? > > Note, a change could be as simple as s/ablkcipher/skcipher/g Because I wasn't aware of this :) I'll update. > > + struct crypto_tfm *tfm = crypto_ablkcipher_tfm(ctfm); > > + struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm); > > + struct crypto_aes_ctx aes; > > + int ret, i; > > + > > + ret = crypto_aes_expand_key(&aes, key, len); > > + if (ret) { > > + crypto_ablkcipher_set_flags(ctfm, CRYPTO_TFM_RES_BAD_KEY_LEN); > > + return ret; > > + } > > + > > + for (i = 0; i < len / sizeof(u32); i++) { > > + if (ctx->key[i] != cpu_to_le32(aes.key_enc[i])) { > > + ctx->base.needs_inv = true; > > + break; > > + } > > + } > > + > > + for (i = 0; i < len / sizeof(u32); i++) > > + ctx->key[i] = cpu_to_le32(aes.key_enc[i]); > > + > > + ctx->key_len = len; > > memzero_explicit(aes)? OK, I'll update. > > +static int safexcel_aes_send(struct crypto_async_request *async, > > +int ring, struct safexcel_request *request, > > +int *commands, int *results) > > +{ > > + struct ablkcipher_request *req = ablkcipher_request_cast(async); > > + struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(req->base.tfm); > > + struct safexcel_crypto_priv *priv = ctx->priv; > > + struct safexcel_command_desc *cdesc; > > + struct safexcel_result_desc *rdesc; > > + struct scatterlist *sg; > > + int nr_src, nr_dst, n_cdesc = 0, n_rdesc = 0, queued = req->nbytes; > > + int i, ret = 0; > > + > > + request->req = &req->base; > > + > > + if (req->src == req->dst) { > > + nr_src = dma_map_sg(priv->dev, req->src, > > + sg_nents_for_len(req->src, req->nbytes), > > + DMA_BIDIRECTIONAL); > > + nr_dst = nr_src; > > + if (!nr_src) > > + return -EINVAL; > > + } else { > > + nr_src = dma_map_sg(priv->dev, req->src, > > + sg_nents_for_len(req->src, req->nbytes), > > + DMA_TO_DEVICE); > > + if (!nr_src) > > + return -EINVAL; > > + > > + nr_dst = dma_map_sg(priv->dev, req->dst, > > + sg_nents_for_len(req->dst, req->nbytes), > > + DMA_FROM_DEVICE); > > + if (!nr_dst) { > > + dma_unmap_sg(priv->dev, req->src, > > +sg_nents_for_len(req->src, req->nbytes), > > +DMA_TO_DEVICE); > > + return -EINVAL; > > + } > > + } > > + > > + memcpy(ctx->base.ctxr->data, ctx->key, ctx->key_len); > > Is ctxr->data properly zeroized? No, I'll update. Thanks for the review! Antoine -- Antoine Ténart, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH 7/7] liquidio: use pcie_flr instead of duplicating it
From: Christoph Hellwig Date: Fri, 14 Apr 2017 21:11:31 +0200 > Signed-off-by: Christoph Hellwig > --- > drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 15 +-- > 1 file changed, 1 insertion(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c > b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c > index 9d5e03502c76..afdbf7fa016e 100644 > --- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c > +++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c > @@ -869,20 +869,7 @@ static void octeon_pci_flr(struct octeon_device *oct) > pci_write_config_word(oct->pci_dev, PCI_COMMAND, > PCI_COMMAND_INTX_DISABLE); > > - /* Wait for Transaction Pending bit clean */ > - msleep(100); > - pcie_capability_read_word(oct->pci_dev, PCI_EXP_DEVSTA, &status); > - if (status & PCI_EXP_DEVSTA_TRPND) { > - dev_info(&oct->pci_dev->dev, "Function reset incomplete after > 100ms, sleeping for 5 seconds\n"); > - ssleep(5); > - pcie_capability_read_word(oct->pci_dev, PCI_EXP_DEVSTA, > - &status); > - if (status & PCI_EXP_DEVSTA_TRPND) > - dev_info(&oct->pci_dev->dev, "Function reset still > incomplete after 5s, reset anyway\n"); > - } > - pcie_capability_set_word(oct->pci_dev, PCI_EXP_DEVCTL, > - PCI_EXP_DEVCTL_BCR_FLR); > - mdelay(100); > + pcie_flr(oct->pci_dev); > > pci_cfg_access_unlock(oct->pci_dev); > > -- > 2.11.0 > This patch works. I tested it on a LiquidIO NIC and the "next" branch of the PCI git tree. But the patch causes a gcc warning: .../lio_vf_main.c: In function 'octeon_pci_flr': .../lio_vf_main.c:862:6: warning: unused variable 'status' [-Wunused-variable] Can you rework the patch to get rid of the warning? Thanks.
Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it
On 04/24/2017 10:35 AM, Christoph Hellwig wrote: On Mon, Apr 24, 2017 at 02:16:31PM +, Byczkowski, Jakub wrote: Tested-by: Jakub Byczkowski Are you (and Doug) ok with queueing this up in the PCI tree? We are fine however Doug wants to handle it. -Denny
Re: [PATCH 3/6] ima: Simplify policy_func_show.
Am Freitag, 21. April 2017, 09:57:56 BRT schrieb Mimi Zohar: > On Thu, 2017-04-20 at 17:40 -0300, Thiago Jung Bauermann wrote: > > @@ -949,49 +936,16 @@ void ima_policy_stop(struct seq_file *m, void *v) > > > > #define pt(token) policy_tokens[token + Opt_err].pattern > > #define mt(token) mask_tokens[token] > > > > -#define ft(token) func_tokens[token] > > > > /* > > > > * policy_func_show - display the ima_hooks policy rule > > */ > > > > static void policy_func_show(struct seq_file *m, enum ima_hooks func) > > { > > > > - char tbuf[64] = {0,}; > > - > > - switch (func) { > > - case FILE_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_file)); > > - break; > > - case MMAP_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_mmap)); > > - break; > > - case BPRM_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_bprm)); > > - break; > > - case MODULE_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_module)); > > - break; > > - case FIRMWARE_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_firmware)); > > - break; > > - case POST_SETATTR: > > - seq_printf(m, pt(Opt_func), ft(func_post)); > > - break; > > - case KEXEC_KERNEL_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_kexec_kernel)); > > - break; > > - case KEXEC_INITRAMFS_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs)); > > - break; > > - case POLICY_CHECK: > > - seq_printf(m, pt(Opt_func), ft(func_policy)); > > - break; > > - default: > > - snprintf(tbuf, sizeof(tbuf), "%d", func); > > - seq_printf(m, pt(Opt_func), tbuf); > > - break; > > - } > > - seq_puts(m, " "); > > + if (func > 0 && func < MAX_CHECK) > > + seq_printf(m, "func=%s ", func_tokens[func]); > > + else > > + seq_printf(m, "func=%d ", func); > > The only time this can happen is when __kernel_read_file_id() is > updated without updating the read_idmap[]. Perhaps we can display the > number and the appropriate __kernel_read_file_id string. >From what I understood of the code func comes from ima_parse_rule, so that condition would only happen if ima_parse_rule got out of sync with func_tokens. Since that code only initializes func with constants from enum ima_hooks and this patch makes ima_hooks automatically sync with func_tokens, the else branch is more like a "can't happen" safety net. read_idmap is only used in ima_post_read_file, and I couldn't see a relation between that code path and the one for ima_policy_show. -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it
On Mon, Apr 24, 2017 at 02:16:31PM +, Byczkowski, Jakub wrote: > Tested-by: Jakub Byczkowski Are you (and Doug) ok with queueing this up in the PCI tree?
[PATCH 2/9] crypto: brcm - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/bcm/cipher.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/bcm/cipher.c b/drivers/crypto/bcm/cipher.c index cc0d5b9..304c7ed 100644 --- a/drivers/crypto/bcm/cipher.c +++ b/drivers/crypto/bcm/cipher.c @@ -2510,8 +2510,8 @@ static int ahash_hmac_setkey(struct crypto_ahash *ahash, const u8 *key, memcpy(ctx->opad, ctx->ipad, blocksize); for (index = 0; index < blocksize; index++) { - ctx->ipad[index] ^= 0x36; - ctx->opad[index] ^= 0x5c; + ctx->ipad[index] ^= HMAC_IPAD_VALUE; + ctx->opad[index] ^= HMAC_OPAD_VALUE; } flow_dump(" ipad: ", ctx->ipad, blocksize); -- 2.10.2
[PATCH 7/9] crypto: qat - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/qat/qat_common/qat_algs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c index 20f35df..6843fe2 100644 --- a/drivers/crypto/qat/qat_common/qat_algs.c +++ b/drivers/crypto/qat/qat_common/qat_algs.c @@ -178,8 +178,8 @@ static int qat_alg_do_precomputes(struct icp_qat_hw_auth_algo_blk *hash, for (i = 0; i < block_size; i++) { char *ipad_ptr = ipad + i; char *opad_ptr = opad + i; - *ipad_ptr ^= 0x36; - *opad_ptr ^= 0x5C; + *ipad_ptr ^= HMAC_IPAD_VALUE; + *opad_ptr ^= HMAC_OPAD_VALUE; } if (crypto_shash_init(shash)) -- 2.10.2
[PATCH 4/9] crypto: marvell - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/marvell/hash.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c index 77c0fb9..a6df61f 100644 --- a/drivers/crypto/marvell/hash.c +++ b/drivers/crypto/marvell/hash.c @@ -1164,8 +1164,8 @@ static int mv_cesa_ahmac_pad_init(struct ahash_request *req, memcpy(opad, ipad, blocksize); for (i = 0; i < blocksize; i++) { - ipad[i] ^= 0x36; - opad[i] ^= 0x5c; + ipad[i] ^= HMAC_IPAD_VALUE; + opad[i] ^= HMAC_OPAD_VALUE; } return 0; -- 2.10.2
[PATCH 8/9] crypto: mediatek - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/mediatek/mtk-sha.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/mediatek/mtk-sha.c b/drivers/crypto/mediatek/mtk-sha.c index 2226f12..cbcb522 100644 --- a/drivers/crypto/mediatek/mtk-sha.c +++ b/drivers/crypto/mediatek/mtk-sha.c @@ -825,8 +825,8 @@ static int mtk_sha_setkey(struct crypto_ahash *tfm, const u8 *key, memcpy(bctx->opad, bctx->ipad, bs); for (i = 0; i < bs; i++) { - bctx->ipad[i] ^= 0x36; - bctx->opad[i] ^= 0x5c; + bctx->ipad[i] ^= HMAC_IPAD_VALUE; + bctx->opad[i] ^= HMAC_OPAD_VALUE; } return 0; -- 2.10.2
[PATCH 9/9] crypto: ccp - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/ccp/ccp-crypto-sha.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/ccp/ccp-crypto-sha.c b/drivers/crypto/ccp/ccp-crypto-sha.c index 6b46eea..3834366 100644 --- a/drivers/crypto/ccp/ccp-crypto-sha.c +++ b/drivers/crypto/ccp/ccp-crypto-sha.c @@ -308,8 +308,8 @@ static int ccp_sha_setkey(struct crypto_ahash *tfm, const u8 *key, } for (i = 0; i < block_size; i++) { - ctx->u.sha.ipad[i] = ctx->u.sha.key[i] ^ 0x36; - ctx->u.sha.opad[i] = ctx->u.sha.key[i] ^ 0x5c; + ctx->u.sha.ipad[i] = ctx->u.sha.key[i] ^ HMAC_IPAD_VALUE; + ctx->u.sha.opad[i] = ctx->u.sha.key[i] ^ HMAC_OPAD_VALUE; } sg_init_one(&ctx->u.sha.opad_sg, ctx->u.sha.opad, block_size); -- 2.10.2
[PATCH 6/9] crypto: omap-sham - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/omap-sham.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c index d0b16e5..11e11a6 100644 --- a/drivers/crypto/omap-sham.c +++ b/drivers/crypto/omap-sham.c @@ -1326,8 +1326,8 @@ static int omap_sham_setkey(struct crypto_ahash *tfm, const u8 *key, memcpy(bctx->opad, bctx->ipad, bs); for (i = 0; i < bs; i++) { - bctx->ipad[i] ^= 0x36; - bctx->opad[i] ^= 0x5c; + bctx->ipad[i] ^= HMAC_IPAD_VALUE; + bctx->opad[i] ^= HMAC_OPAD_VALUE; } } -- 2.10.2
[PATCH 3/9] crypto: ixp4xx - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/ixp4xx_crypto.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c index 771dd26..3b7680c 100644 --- a/drivers/crypto/ixp4xx_crypto.c +++ b/drivers/crypto/ixp4xx_crypto.c @@ -90,8 +90,6 @@ #define CTL_FLAG_PERFORM_AEAD 0x0008 #define CTL_FLAG_MASK 0x000f -#define HMAC_IPAD_VALUE 0x36 -#define HMAC_OPAD_VALUE 0x5C #define HMAC_PAD_BLOCKLEN SHA1_BLOCK_SIZE #define MD5_DIGEST_SIZE 16 -- 2.10.2
[PATCH 5/9] crypto: mv_cesa - Use IPAD/OPAD constant
This patch simply replace all occurrence of HMAC IPAD/OPAD value by their define. Signed-off-by: Corentin Labbe --- drivers/crypto/mv_cesa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c index 451fa18..41ae190 100644 --- a/drivers/crypto/mv_cesa.c +++ b/drivers/crypto/mv_cesa.c @@ -822,8 +822,8 @@ static int mv_hash_setkey(struct crypto_ahash *tfm, const u8 * key, memcpy(opad, ipad, bs); for (i = 0; i < bs; i++) { - ipad[i] ^= 0x36; - opad[i] ^= 0x5c; + ipad[i] ^= HMAC_IPAD_VALUE; + opad[i] ^= HMAC_OPAD_VALUE; } rc = crypto_shash_init(shash) ? : -- 2.10.2
[PATCH 1/9] crypto: add hmac IPAD/OPAD constant
Many HMAC users directly use directly 0x36/0x5c values. It's better with crypto to use a name instead of directly some crypto constant. This patch simply add HMAC_IPAD_VALUE/HMAC_OPAD_VALUE defines. Signed-off-by: Corentin Labbe --- crypto/hmac.c | 4 ++-- include/crypto/hash.h | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crypto/hmac.c b/crypto/hmac.c index 72e38c0..4a997ce 100644 --- a/crypto/hmac.c +++ b/crypto/hmac.c @@ -74,8 +74,8 @@ static int hmac_setkey(struct crypto_shash *parent, memcpy(opad, ipad, bs); for (i = 0; i < bs; i++) { - ipad[i] ^= 0x36; - opad[i] ^= 0x5c; + ipad[i] ^= HMAC_IPAD_VALUE; + opad[i] ^= HMAC_OPAD_VALUE; } return crypto_shash_init(shash) ?: diff --git a/include/crypto/hash.h b/include/crypto/hash.h index b5727bc..0f51ff1 100644 --- a/include/crypto/hash.h +++ b/include/crypto/hash.h @@ -922,4 +922,7 @@ static inline void shash_desc_zero(struct shash_desc *desc) sizeof(*desc) + crypto_shash_descsize(desc->tfm)); } +#define HMAC_IPAD_VALUE 0x36 +#define HMAC_OPAD_VALUE 0x5c + #endif /* _CRYPTO_HASH_H */ -- 2.10.2
RE: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it
Tested-by: Jakub Byczkowski -Original Message- From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-ow...@vger.kernel.org] On Behalf Of Christoph Hellwig Sent: Friday, April 14, 2017 9:11 PM To: Bjorn Helgaas ; Cabiddu, Giovanni ; Benedetto, Salvatore ; Marciniszyn, Mike ; Dalessandro, Dennis ; Derek Chickles ; Satanand Burla ; Felix Manlunas ; Raghu Vatsavayi ; Kirsher, Jeffrey T Cc: linux-...@vger.kernel.org; qat-linux ; linux-crypto@vger.kernel.org; linux-r...@vger.kernel.org; net...@vger.kernel.org; linux-ker...@vger.kernel.org Subject: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it Signed-off-by: Christoph Hellwig --- drivers/infiniband/hw/hfi1/chip.c | 4 ++-- drivers/infiniband/hw/hfi1/hfi.h | 1 - drivers/infiniband/hw/hfi1/pcie.c | 30 -- 3 files changed, 2 insertions(+), 33 deletions(-) diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c index 121a4c920f1b..d037f72e4d96 100644 --- a/drivers/infiniband/hw/hfi1/chip.c +++ b/drivers/infiniband/hw/hfi1/chip.c @@ -13610,14 +13610,14 @@ static void init_chip(struct hfi1_devdata *dd) dd_dev_info(dd, "Resetting CSRs with FLR\n"); /* do the FLR, the DC reset will remain */ - hfi1_pcie_flr(dd); + pcie_flr(dd->pcidev); /* restore command and BARs */ restore_pci_variables(dd); if (is_ax(dd)) { dd_dev_info(dd, "Resetting CSRs with FLR\n"); - hfi1_pcie_flr(dd); + pcie_flr(dd->pcidev); restore_pci_variables(dd); } } else { diff --git a/drivers/infiniband/hw/hfi1/hfi.h b/drivers/infiniband/hw/hfi1/hfi.h index 0808e3c3ba39..40d7559fa723 100644 --- a/drivers/infiniband/hw/hfi1/hfi.h +++ b/drivers/infiniband/hw/hfi1/hfi.h @@ -1764,7 +1764,6 @@ int hfi1_pcie_init(struct pci_dev *, const struct pci_device_id *); void hfi1_pcie_cleanup(struct pci_dev *); int hfi1_pcie_ddinit(struct hfi1_devdata *, struct pci_dev *); void hfi1_pcie_ddcleanup(struct hfi1_devdata *); -void hfi1_pcie_flr(struct hfi1_devdata *); int pcie_speeds(struct hfi1_devdata *); void request_msix(struct hfi1_devdata *, u32 *, struct hfi1_msix_entry *); void hfi1_enable_intx(struct pci_dev *); diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c index 0829fce06172..c81556e84831 100644 --- a/drivers/infiniband/hw/hfi1/pcie.c +++ b/drivers/infiniband/hw/hfi1/pcie.c @@ -240,36 +240,6 @@ void hfi1_pcie_ddcleanup(struct hfi1_devdata *dd) iounmap(dd->piobase); } -/* - * Do a Function Level Reset (FLR) on the device. - * Based on static function drivers/pci/pci.c:pcie_flr(). - */ -void hfi1_pcie_flr(struct hfi1_devdata *dd) -{ - int i; - u16 status; - - /* no need to check for the capability - we know the device has it */ - - /* wait for Transaction Pending bit to clear, at most a few ms */ - for (i = 0; i < 4; i++) { - if (i) - msleep((1 << (i - 1)) * 100); - - pcie_capability_read_word(dd->pcidev, PCI_EXP_DEVSTA, &status); - if (!(status & PCI_EXP_DEVSTA_TRPND)) - goto clear; - } - - dd_dev_err(dd, "Transaction Pending bit is not clearing, proceeding with reset anyway\n"); - -clear: - pcie_capability_set_word(dd->pcidev, PCI_EXP_DEVCTL, -PCI_EXP_DEVCTL_BCR_FLR); - /* PCIe spec requires the function to be back within 100ms */ - msleep(100); -} - static void msix_setup(struct hfi1_devdata *dd, int pos, u32 *msixcnt, struct hfi1_msix_entry *hfi1_msix_entry) { -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
Re: [PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver
Am Montag, 24. April 2017, 09:54:06 CEST schrieb Antoine Tenart: Hi Antoine, > +struct safexcel_cipher_ctx { > + struct safexcel_context base; > + struct safexcel_crypto_priv *priv; > + > + enum safexcel_cipher_direction direction; > + u32 mode; > + > + __le32 key[8]; Can you please help me find the location where this memory is zeroized when released? > + unsigned int key_len; > +}; > + > +static void safexcel_cipher_token(struct safexcel_cipher_ctx *ctx, > + struct crypto_async_request *async, > + struct safexcel_command_desc *cdesc, > + u32 length) > +{ > + struct ablkcipher_request *req = ablkcipher_request_cast(async); > + struct safexcel_token *token; > + unsigned offset = 0; > + > + if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) { > + offset = AES_BLOCK_SIZE / sizeof(u32); > + memcpy(cdesc->control_data.token, req->info, AES_BLOCK_SIZE); > + > + cdesc->control_data.options |= EIP197_OPTION_4_TOKEN_IV_CMD; > + } > + > + token = (struct safexcel_token *)(cdesc->control_data.token + offset); > + > + token[0].opcode = EIP197_TOKEN_OPCODE_DIRECTION; > + token[0].packet_length = length; > + token[0].stat = EIP197_TOKEN_STAT_LAST_PACKET; > + token[0].instructions = EIP197_TOKEN_INS_LAST | > + EIP197_TOKEN_INS_TYPE_CRYTO | > + EIP197_TOKEN_INS_TYPE_OUTPUT; > +} > + > +static int safexcel_aes_setkey(struct crypto_ablkcipher *ctfm, const u8 > *key, + unsigned int len) > +{ You still use ablkcipher. I thought that it is on its way out in favor of the skcipher API. Why do you stick to ablkcipher? Note, a change could be as simple as s/ablkcipher/skcipher/g > + struct crypto_tfm *tfm = crypto_ablkcipher_tfm(ctfm); > + struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm); > + struct crypto_aes_ctx aes; > + int ret, i; > + > + ret = crypto_aes_expand_key(&aes, key, len); > + if (ret) { > + crypto_ablkcipher_set_flags(ctfm, CRYPTO_TFM_RES_BAD_KEY_LEN); > + return ret; > + } > + > + for (i = 0; i < len / sizeof(u32); i++) { > + if (ctx->key[i] != cpu_to_le32(aes.key_enc[i])) { > + ctx->base.needs_inv = true; > + break; > + } > + } > + > + for (i = 0; i < len / sizeof(u32); i++) > + ctx->key[i] = cpu_to_le32(aes.key_enc[i]); > + > + ctx->key_len = len; memzero_explicit(aes)? > + > + return 0; > +} > + > +static int safexcel_context_control(struct safexcel_cipher_ctx *ctx, > + struct safexcel_command_desc *cdesc) > +{ > + struct safexcel_crypto_priv *priv = ctx->priv; > + int ctrl_size; > + > + if (ctx->direction == SAFEXCEL_ENCRYPT) > + cdesc->control_data.control0 |= CONTEXT_CONTROL_TYPE_CRYPTO_OUT; > + else > + cdesc->control_data.control0 |= CONTEXT_CONTROL_TYPE_CRYPTO_IN; > + > + cdesc->control_data.control0 |= CONTEXT_CONTROL_KEY_EN; > + cdesc->control_data.control1 |= ctx->mode; > + > + switch (ctx->key_len) { > + case AES_KEYSIZE_128: > + cdesc->control_data.control0 |= > CONTEXT_CONTROL_CRYPTO_ALG_AES128; > + ctrl_size = 4; > + break; > + case AES_KEYSIZE_192: > + cdesc->control_data.control0 |= > CONTEXT_CONTROL_CRYPTO_ALG_AES192; > + ctrl_size = 6; > + break; > + case AES_KEYSIZE_256: > + cdesc->control_data.control0 |= > CONTEXT_CONTROL_CRYPTO_ALG_AES256; > + ctrl_size = 8; > + break; > + default: > + dev_err(priv->dev, "aes keysize not supported: %u\n", > + ctx->key_len); > + return -EINVAL; > + } > + cdesc->control_data.control0 |= CONTEXT_CONTROL_SIZE(ctrl_size); > + > + return 0; > +} > + > +static int safexcel_handle_result(struct safexcel_crypto_priv *priv, int > ring, + struct crypto_async_request *async, > + bool *should_complete, int *ret) > +{ > + struct ablkcipher_request *req = ablkcipher_request_cast(async); > + struct safexcel_result_desc *rdesc; > + int ndesc = 0; > + > + *ret = 0; > + > + spin_lock_bh(&priv->ring[ring].egress_lock); > + do { > + rdesc = safexcel_ring_next_rptr(priv, &priv->ring[ring].rdr); > + if (IS_ERR(rdesc)) { > + dev_err(priv->dev, > + "cipher: result: could not retrieve the result > descriptor\n"); > + *ret = PTR_ERR(rdesc); > + break; > + } > + > + if (rdesc->result_data.error_code) { > + dev_err(priv->dev, > +
Re: [PATCH 2/3] crypto: cavium: Remove the individual encrypt/decrypt function for each algorithm
On Fri, Apr 21, 2017 at 11:16:05AM +, George Cherian wrote: > > -int cvm_aes_encrypt_cbc(struct ablkcipher_request *req) > +static inline u32 cvm_cipher_type(const char *name) > { > - return cvm_enc_dec(req, true, AES_CBC); > -} > > -int cvm_aes_decrypt_cbc(struct ablkcipher_request *req) > -{ > - return cvm_enc_dec(req, false, AES_CBC); > + const struct cvm_cipher *cipher = cvm_cipher_table; > + > + while (cipher->name) { > + if (!strcmp(cipher->name, name)) > + break; > + cipher++; > + } > + > + return cipher->value; > } That's rather unwieldy. It's usually easier to embed the cipher type into the algo structure and then get to it via the alg object. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: Allow ecb(cipher_null) in FIPS mode
Milan Broz wrote: > The cipher_null is not a real cipher, FIPS mode should not restrict its use. > > It is used for several tests (for example in cryptsetup testsuite) and also > temporarily for reencryption of not yet encrypted device in > cryptsetup-reencrypt tool. > > Problem is easily reproducible with > cryptsetup benchmark -c null > > Signed-off-by: Milan Broz Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 1/2] crypto: scomp - allow registration of multiple scomps
On Fri, Apr 21, 2017 at 09:54:29PM +0100, Giovanni Cabiddu wrote: > Add crypto_register_scomps and crypto_unregister_scomps to allow > the registration of multiple implementations with one call. > > Signed-off-by: Giovanni Cabiddu All applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: crypto4xx: rename ce_ring_contol to ce_ring_control
On Fri, Apr 21, 2017 at 12:13:49PM +0100, Colin King wrote: > From: Colin Ian King > > trivial spelling mistake, missing r, rename to ce_ring_control > > Signed-off-by: Colin Ian King Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: algif_aead - Require setkey before accept(2)
On Mon, Apr 24, 2017 at 11:15:23AM +0200, Stephan Müller wrote: > Am Montag, 24. April 2017, 11:03:13 CEST schrieb Herbert Xu: > > Hi Herbert, > > > On Mon, Apr 24, 2017 at 11:01:50AM +0200, Stephan Müller wrote: > > > Shall I send an updated patch with aead_sock_destruct cleared? > > > > Yes please. > > Please find attached v2 with the discussed change. > > ---8<--- > > Some cipher implementations will crash if you try to use them > without calling setkey first. This patch adds a check so that > the accept(2) call will fail with -ENOKEY if setkey hasn't been > done on the socket yet. > > Signed-off-by: Stephan Mueller Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 0/2] Change CCP ISR handler model
On Fri, Apr 21, 2017 at 10:49:56AM -0500, Gary R Hook wrote: > he CCP has the ability to perform several operations > simultaneously, but only one interrupt. The current design > exposes a window when using MSI/MSI-X interrupts wherein > state can change but no interrupt is generated; this can > lead to a hang in the engine. Switch to a tasklet backend > which allows serializing state changes, handles processing > of the interrupts, and avoids the loss of task completion > status. > > --- > > Gary R Hook (2): > crypto: ccp - Change ISR handler method for a v3 CCP > crypto: ccp - Change ISR handler method for a v5 CCP All applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: algif_aead - Require setkey before accept(2)
Am Montag, 24. April 2017, 12:22:39 CEST schrieb Herbert Xu: Hi Herbert, > Patch applied. Thanks. Thank you. The patch regarding the memory management of algif_aead is affected by this change as well. Shall I roll a new version of that patch for algif_aead or do you want me to wait for another review round? Ciao Stephan
[PATCH v2] crypto: algif_aead - Require setkey before accept(2)
Am Montag, 24. April 2017, 11:03:13 CEST schrieb Herbert Xu: Hi Herbert, > On Mon, Apr 24, 2017 at 11:01:50AM +0200, Stephan Müller wrote: > > Shall I send an updated patch with aead_sock_destruct cleared? > > Yes please. Please find attached v2 with the discussed change. ---8<--- Some cipher implementations will crash if you try to use them without calling setkey first. This patch adds a check so that the accept(2) call will fail with -ENOKEY if setkey hasn't been done on the socket yet. Signed-off-by: Stephan Mueller --- crypto/algif_aead.c | 157 +--- 1 file changed, 149 insertions(+), 8 deletions(-) diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c index 5a80537..e0d55ea 100644 --- a/crypto/algif_aead.c +++ b/crypto/algif_aead.c @@ -44,6 +44,11 @@ struct aead_async_req { char iv[]; }; +struct aead_tfm { + struct crypto_aead *aead; + bool has_key; +}; + struct aead_ctx { struct aead_sg_list tsgl; struct aead_async_rsgl first_rsgl; @@ -723,24 +728,146 @@ static struct proto_ops algif_aead_ops = { .poll = aead_poll, }; +static int aead_check_key(struct socket *sock) +{ + int err = 0; + struct sock *psk; + struct alg_sock *pask; + struct aead_tfm *tfm; + struct sock *sk = sock->sk; + struct alg_sock *ask = alg_sk(sk); + + lock_sock(sk); + if (ask->refcnt) + goto unlock_child; + + psk = ask->parent; + pask = alg_sk(ask->parent); + tfm = pask->private; + + err = -ENOKEY; + lock_sock_nested(psk, SINGLE_DEPTH_NESTING); + if (!tfm->has_key) + goto unlock; + + if (!pask->refcnt++) + sock_hold(psk); + + ask->refcnt = 1; + sock_put(psk); + + err = 0; + +unlock: + release_sock(psk); +unlock_child: + release_sock(sk); + + return err; +} + +static int aead_sendmsg_nokey(struct socket *sock, struct msghdr *msg, + size_t size) +{ + int err; + + err = aead_check_key(sock); + if (err) + return err; + + return aead_sendmsg(sock, msg, size); +} + +static ssize_t aead_sendpage_nokey(struct socket *sock, struct page *page, + int offset, size_t size, int flags) +{ + int err; + + err = aead_check_key(sock); + if (err) + return err; + + return aead_sendpage(sock, page, offset, size, flags); +} + +static int aead_recvmsg_nokey(struct socket *sock, struct msghdr *msg, + size_t ignored, int flags) +{ + int err; + + err = aead_check_key(sock); + if (err) + return err; + + return aead_recvmsg(sock, msg, ignored, flags); +} + +static struct proto_ops algif_aead_ops_nokey = { + .family = PF_ALG, + + .connect= sock_no_connect, + .socketpair = sock_no_socketpair, + .getname= sock_no_getname, + .ioctl = sock_no_ioctl, + .listen = sock_no_listen, + .shutdown = sock_no_shutdown, + .getsockopt = sock_no_getsockopt, + .mmap = sock_no_mmap, + .bind = sock_no_bind, + .accept = sock_no_accept, + .setsockopt = sock_no_setsockopt, + + .release= af_alg_release, + .sendmsg= aead_sendmsg_nokey, + .sendpage = aead_sendpage_nokey, + .recvmsg= aead_recvmsg_nokey, + .poll = aead_poll, +}; + static void *aead_bind(const char *name, u32 type, u32 mask) { - return crypto_alloc_aead(name, type, mask); + struct aead_tfm *tfm; + struct crypto_aead *aead; + + tfm = kzalloc(sizeof(*tfm), GFP_KERNEL); + if (!tfm) + return ERR_PTR(-ENOMEM); + + aead = crypto_alloc_aead(name, type, mask); + if (IS_ERR(aead)) { + kfree(tfm); + return ERR_CAST(aead); + } + + tfm->aead = aead; + + return tfm; } static void aead_release(void *private) { - crypto_free_aead(private); + struct aead_tfm *tfm = private; + + crypto_free_aead(tfm->aead); + kfree(tfm); } static int aead_setauthsize(void *private, unsigned int authsize) { - return crypto_aead_setauthsize(private, authsize); + struct aead_tfm *tfm = private; + + return crypto_aead_setauthsize(tfm->aead, authsize); } static int aead_setkey(void *private, const u8 *key, unsigned int keylen) { - return crypto_aead_setkey(private, key, keylen); + struct aead_tfm *tfm = private; + int err; + + err = crypto_aead_setkey(tfm->aead, key, keylen); + tfm->has_key = !err; + + return err; } static void aead_sock_de
Re: [PATCH] crypto: algif_aead - Require setkey before accept(2)
On Mon, Apr 24, 2017 at 11:01:50AM +0200, Stephan Müller wrote: > > Shall I send an updated patch with aead_sock_destruct cleared? Yes please. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: algif_aead - Require setkey before accept(2)
Am Montag, 24. April 2017, 10:43:24 CEST schrieb Herbert Xu: Hi Herbert, > On Fri, Apr 21, 2017 at 06:35:07PM +0200, Stephan Müller wrote: > > After checking again, IMHO that is no unreleated cleanup or even a cleanup > > at all. > > > > void *private used to be struct crypto_aead and is now struct aead_tfm. > > struct crypto_aead is found in private->aead. Hence, the patch assigned > > private to tfm and then obtained the struct crypto_aead pointer. As this > > was not necessary before, it is a required extension IMHO. > > Fair enough. > > But what about the change in aead_sock_destruct? Can you explain why > it is no longer possible to obtain the tfm from ctx->aead_req? > > Thanks, aead_request_set_callback(&ctx->aead_req) is set in aead_accept_parent_nokey. aead_accept_parent_nokey is only invoked from aead_accept_parent if the key was set. My thought was: Let us assume a caller does not set a key, calls accept and then destruct. In this code path, ctx->aead_req is not initialized. Hence, I would think that only the path using struct aead_tfm is safe in any case. But I see that aead_sock_destruct is also linked in by aead_accept_parent_nokey. Hence, my initial idea was not correct as the destruct path is only callable when the accept_nokey is invoked. Shall I send an updated patch with aead_sock_destruct cleared? Ciao Stephan
Re: [Patch V5 1/7] crypto: Multi-buffer encryption infrastructure support
On Thu, Apr 20, 2017 at 01:50:34PM -0700, Megha Dey wrote: > > +static int simd_skcipher_decrypt_mb(struct skcipher_request *req) > +{ > + struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req); > + struct simd_skcipher_ctx_mb *ctx = crypto_skcipher_ctx(tfm); > + struct skcipher_request *subreq; > + struct crypto_skcipher *child; > + > + subreq = skcipher_request_ctx(req); > + *subreq = *req; > + > + child = &ctx->mcryptd_tfm->base; > + > + skcipher_request_set_tfm(subreq, child); > + > + return crypto_skcipher_decrypt(subreq); > +} Why did you add the code here in simd.c? This doesn't seem to have anything to do with kernel SIMD instructions. >From your later patch I see that what you want is simply a wrapper around a synchronous internal algorithm. That is indeed similar in purpose to simd.c, but I still find it weird to be adding this code here. My suggestion is to instead move this code to mcryptd.c. It's a bit fiddly because mcryptd already exists as a template. But you should still be able to create a seaprate mcryptd interface for skcipher in the way that simd does it. In time we can convert mcryptd hash to this model too. Also you adopted the simd compat naming scheme. Please don't do that as you're creating something brand new so there is no need to keep the existing compat (i.e., __XXX) naming scheme. In the new naming scheme, the internal algorithm should just carry the normal alg name (e.g., ecb(aes)) and driver name, while the mcryptd wrapped version will have the same algorithm name but carry a prefix on the driver name (which is simd- for simd.c, but you should probably used mcryptd-). Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver
Hi Igal, On Mon, Apr 24, 2017 at 08:50:32AM +, Igal Liberman wrote: > [...] > > > + priv->clk = of_clk_get(dev->of_node, 0); > > + if (!IS_ERR(priv->clk)) { > > + ret = clk_prepare_enable(priv->clk); > > + if (ret) { > > + dev_err(dev, "unable to enable clk (%d)\n", ret); > > + return ret; > > + } > > + } else { > > + /* The clock isn't mandatory */ > > + if (PTR_ERR(priv->clk) == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > + } > > + > > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > The correct address width of the engine in Marvell SoCs is 40bit. Oops. I'll update. Thanks! Antoine -- Antoine Ténart, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
RE: [PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver
[...] > + priv->clk = of_clk_get(dev->of_node, 0); > + if (!IS_ERR(priv->clk)) { > + ret = clk_prepare_enable(priv->clk); > + if (ret) { > + dev_err(dev, "unable to enable clk (%d)\n", ret); > + return ret; > + } > + } else { > + /* The clock isn't mandatory */ > + if (PTR_ERR(priv->clk) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + } > + > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); The correct address width of the engine in Marvell SoCs is 40bit. > + if (ret) > + goto err_clk; > + > + priv->context_pool = dmam_pool_create("safexcel-context", dev, > + sizeof(struct > safexcel_context_record), > + 1, 0); > + if (!priv->context_pool) { > + ret = -ENOMEM; > + goto err_clk; > + } > + Thanks, Igal
Re: [PATCH] crypto: algif_aead - Require setkey before accept(2)
On Fri, Apr 21, 2017 at 06:35:07PM +0200, Stephan Müller wrote: > > After checking again, IMHO that is no unreleated cleanup or even a cleanup at > all. > > void *private used to be struct crypto_aead and is now struct aead_tfm. > struct > crypto_aead is found in private->aead. Hence, the patch assigned private to > tfm and then obtained the struct crypto_aead pointer. As this was not > necessary before, it is a required extension IMHO. Fair enough. But what about the change in aead_sock_destruct? Can you explain why it is no longer possible to obtain the tfm from ctx->aead_req? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT
On Mon, Apr 24, 2017 at 09:04:19AM +0100, Ard Biesheuvel wrote: > > Yes please. OK, patch reverted. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT
On 24 April 2017 at 09:00, Herbert Xu wrote: > On Tue, Apr 18, 2017 at 10:34:01AM -0700, Matthias Kaehlcke wrote: >> El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit: >> >> > On 18 April 2017 at 15:47, Paul Gortmaker >> > wrote: >> > > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke >> > > wrote: >> > >> The operand is an integer constant, make the constness explicit by >> > >> adding the modifier. This is needed for clang to generate valid code >> > >> and also works with gcc. >> > > >> > > Actually it doesn't work with all gcc. I've got an older arm64 >> > > toolchain that I >> > > only use for syntax checking (and hence I don't care if it is the latest >> > > and >> > > greatest) and this commit breaks it: >> > > >> > > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid >> > > operand prefix '%c' >> > > asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val)); >> >> Sorry about that :( > > So what's the consensus? Should I revert this patch? > Yes please.
Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT
On Tue, Apr 18, 2017 at 10:34:01AM -0700, Matthias Kaehlcke wrote: > El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit: > > > On 18 April 2017 at 15:47, Paul Gortmaker > > wrote: > > > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke > > > wrote: > > >> The operand is an integer constant, make the constness explicit by > > >> adding the modifier. This is needed for clang to generate valid code > > >> and also works with gcc. > > > > > > Actually it doesn't work with all gcc. I've got an older arm64 toolchain > > > that I > > > only use for syntax checking (and hence I don't care if it is the latest > > > and > > > greatest) and this commit breaks it: > > > > > > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid > > > operand prefix '%c' > > > asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val)); > > Sorry about that :( So what's the consensus? Should I revert this patch? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[PATCH v3 3/3] MAINTAINERS: add a maintainer for the Inside Secure crypto driver
A new cryptographic engine driver was added in drivers/crypto/inside-secure. Add myself as a maintainer for this driver. Signed-off-by: Antoine Tenart --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index c265a5fe4848..7240b9bca638 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6487,6 +6487,12 @@ F: Documentation/input/multi-touch-protocol.txt F: drivers/input/input-mt.c K: \b(ABS|SYN)_MT_ +INSIDE SECURE CRYPTO DRIVER +M: Antoine Tenart +F: drivers/crypto/inside-secure/ +S: Maintained +L: linux-crypto@vger.kernel.org + INTEL ASoC BDW/HSW DRIVERS M: Jie Yang L: alsa-de...@alsa-project.org (moderated for non-subscribers) -- 2.11.0
[PATCH v3 1/3] Documentation/bindings: Document the SafeXel cryptographic engine driver
The Inside Secure Safexcel cryptographic engine is found on some Marvell SoCs (7k/8k). Document the bindings used by its driver. Signed-off-by: Antoine Tenart --- .../bindings/crypto/inside-secure-safexcel.txt | 27 ++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt diff --git a/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt b/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt new file mode 100644 index ..ff56b9384fcc --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt @@ -0,0 +1,27 @@ +Inside Secure SafeXcel cryptographic engine + +Required properties: +- compatible: Should be "inside-secure,safexcel-eip197". +- reg: Base physical address of the engine and length of memory mapped region. +- interrupts: Interrupt numbers for the rings and engine. +- interrupt-names: Should be "ring0", "ring1", "ring2", "ring3", "eip", "mem". + +Optional properties: +- clocks: Reference to the crypto engine clock. + +Example: + + crypto: crypto@80 { + compatible = "inside-secure,safexcel-eip197"; + reg = <0x80 0x20>; + interrupts = , +, +, +, +, +; + interrupt-names = "mem", "ring0", "ring1", "ring2", "ring3", + "eip"; + clocks = <&cpm_syscon0 1 26>; + status = "disabled"; + }; -- 2.11.0
[PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver
Add support for Inside Secure SafeXcel EIP197 cryptographic engine, which can be found on Marvell Armada 7k and 8k boards. This driver currently implements: ecb(aes), cbc(aes), sha1, sha224, sha256 and hmac(sah1) algorithms. Two firmwares are needed for this engine to work. Their are mostly used for more advanced operations than the ones supported (as of now), but we still need them to pass the data to the internal cryptographic engine. Signed-off-by: Antoine Tenart --- drivers/crypto/Kconfig | 17 + drivers/crypto/Makefile|1 + drivers/crypto/inside-secure/Makefile |2 + drivers/crypto/inside-secure/safexcel.c| 925 + drivers/crypto/inside-secure/safexcel.h| 579 + drivers/crypto/inside-secure/safexcel_cipher.c | 555 + drivers/crypto/inside-secure/safexcel_hash.c | 1045 drivers/crypto/inside-secure/safexcel_ring.c | 157 8 files changed, 3281 insertions(+) create mode 100644 drivers/crypto/inside-secure/Makefile create mode 100644 drivers/crypto/inside-secure/safexcel.c create mode 100644 drivers/crypto/inside-secure/safexcel.h create mode 100644 drivers/crypto/inside-secure/safexcel_cipher.c create mode 100644 drivers/crypto/inside-secure/safexcel_hash.c create mode 100644 drivers/crypto/inside-secure/safexcel_ring.c diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 473d31288ad8..d12a40450858 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -619,4 +619,21 @@ config CRYPTO_DEV_BCM_SPU Secure Processing Unit (SPU). The SPU driver registers ablkcipher, ahash, and aead algorithms with the kernel cryptographic API. +config CRYPTO_DEV_SAFEXCEL + tristate "Inside Secure's SafeXcel cryptographic engine driver" + depends on HAS_DMA && OF + depends on (ARM64 && ARCH_MVEBU) || (COMPILE_TEST && 64BIT) + select CRYPTO_AES + select CRYPTO_BLKCIPHER + select CRYPTO_HASH + select CRYPTO_HMAC + select CRYPTO_SHA1 + select CRYPTO_SHA256 + select CRYPTO_SHA512 + help + This driver interfaces with the SafeXcel EIP-197 cryptographic engine + designed by Inside Secure. Select this if you want to use CBC/ECB + chain mode, AES cipher mode and SHA1/SHA224/SHA256/SHA512 hash + algorithms. + endif # CRYPTO_HW diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile index 739609471169..7ed3e7940f76 100644 --- a/drivers/crypto/Makefile +++ b/drivers/crypto/Makefile @@ -36,3 +36,4 @@ obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/ obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/ +obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/ diff --git a/drivers/crypto/inside-secure/Makefile b/drivers/crypto/inside-secure/Makefile new file mode 100644 index ..302f07dde98c --- /dev/null +++ b/drivers/crypto/inside-secure/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += crypto_safexcel.o +crypto_safexcel-objs := safexcel.o safexcel_ring.o safexcel_cipher.o safexcel_hash.o diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c new file mode 100644 index ..9cb3d0832835 --- /dev/null +++ b/drivers/crypto/inside-secure/safexcel.c @@ -0,0 +1,925 @@ +/* + * Copyright (C) 2017 Marvell + * + * Antoine Tenart + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "safexcel.h" + +static u32 max_rings = EIP197_MAX_RINGS; +module_param(max_rings, uint, 0644); +MODULE_PARM_DESC(max_rings, "Maximum number of rings to use."); + +static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv) +{ + u32 val, htable_offset; + int i; + + /* Enable the record cache memory access */ + val = readl(priv->base + EIP197_CS_RAM_CTRL); + val &= ~EIP197_TRC_ENABLE_MASK; + val |= EIP197_TRC_ENABLE_0; + writel(val, priv->base + EIP197_CS_RAM_CTRL); + + /* Clear all ECC errors */ + writel(0, priv->base + EIP197_TRC_ECCCTRL); + + /* +* Make sure the cache memory is accessible by taking record cache into +* reset. +*/ + val = readl(priv->base + EIP197_TRC_PARAMS); + val |= EIP197_TRC_PARAMS_SW_RESET; + val &= ~EIP197_TRC_PARAMS_DATA_ACCESS; + writel(val, priv->base + EIP197_TRC_PARAMS); + + /* Clear all records */ + for (i = 0; i < EIP197_CS_RC_MAX; i++) { + u32 val, offset = EIP197_CLASSIFICATION_RAMS + i * EIP197_CS_RC_SIZE; + + writel(EIP197_CS_RC_NEXT(EIP197_RC_NULL)
[PATCH v3 0/3] arm64: marvell: add cryptographic engine support for 7k/8k
Hi all, This series adds support for the Inside Secure SafeXcel EIP197 cryptographic engine which can be found on Marvell Armada 7k and 8k boards. A new cryptographic engine driver is added, as well as the relevant device tree definition for the Armada 7040 DB and 8040 DB boards. This driver needs two firmwares to work correctly. These firmware are usually used for more advanced operations than the ones supported (as of now), but we still need them to pass the data to the internal cryptographic engine. This series was tested in various ways on both the Armada 7040 DB and the Armada 8040 DB: using the crypto framework self tests, using tcrypt and while performing various transfers with iperf on top of IPsec. This series is based on top of v4.11-rc1, and is available on a branch (which also contains the PPv2 support for 7k/8k, to ease the process of testing IPsec): https://github.com/atenart/linux v4.11-rc1/7k8k-crypto I can rebase if needed. Patches adding device tree nodes and modifying the arm64 defconfig have been applied to the mvebu tree already. I'd like to thank Ofer Heifetz and Igal Liberman who helped me to do this driver by adding functionalities, optimizing functions and testing (a lot). Thanks, Antoine Since v2: - Used sha*_zero_message_hash definitions instead of custom ones. - Used memzero_explicit() instead of memset() to avoid leaks. - Misc cosmetic updates. Since v1: - Fixed two compilation issues reported by Kbuild. - Removed all dma_to_phys() calls and used the dma addresses instead. - Added a call to dma_set_mask_and_coherent() before calling any DMA API helper. - Removed some DMA free functions to avoid double-freeing. - Do not rely on sg_nents_for_len() to get the number of sg anymore. - Added a dedicated kmalloc'ed cache to use for dma_map_single(). Antoine Tenart (3): Documentation/bindings: Document the SafeXel cryptographic engine driver crypto: inside-secure: add SafeXcel EIP197 crypto engine driver MAINTAINERS: add a maintainer for the Inside Secure crypto driver .../bindings/crypto/inside-secure-safexcel.txt | 27 + MAINTAINERS|6 + drivers/crypto/Kconfig | 17 + drivers/crypto/Makefile|1 + drivers/crypto/inside-secure/Makefile |2 + drivers/crypto/inside-secure/safexcel.c| 925 + drivers/crypto/inside-secure/safexcel.h| 579 +++ drivers/crypto/inside-secure/safexcel_cipher.c | 555 +++ drivers/crypto/inside-secure/safexcel_hash.c | 1045 drivers/crypto/inside-secure/safexcel_ring.c | 157 +++ 10 files changed, 3314 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt create mode 100644 drivers/crypto/inside-secure/Makefile create mode 100644 drivers/crypto/inside-secure/safexcel.c create mode 100644 drivers/crypto/inside-secure/safexcel.h create mode 100644 drivers/crypto/inside-secure/safexcel_cipher.c create mode 100644 drivers/crypto/inside-secure/safexcel_hash.c create mode 100644 drivers/crypto/inside-secure/safexcel_ring.c -- 2.11.0
Re: [PATCH v2 6/9] staging: ccree: add FIPS support
Am Montag, 24. April 2017, 09:07:45 CEST schrieb Gilad Ben-Yossef: Hi Gilad, > I guess we could change the function to indicate that a key is valid > for decryption but not encryption > and have the implementation limiting based on that if there is an > interest in SP800-131A compliance. I would be delighted to see and review a patch. Ciao Stephan
Re: [PATCH v2 6/9] staging: ccree: add FIPS support
Am Montag, 24. April 2017, 09:04:13 CEST schrieb Gilad Ben-Yossef: Hi Gilad, > > Thanks you for the clarification. As I think is obvious by now I am > not a FIPS expert by any stretch. > > Isn't the requirements on DRBG or KDF invocations pertain to key > generation only? > What happens if you don't derive the keys on the system, but wish to > use keys derived elsewhere? > I assumed the limitations on weak keys etc. were meant to protect > against that scenario and are > therefore independent from key generation requirements, but I may have > misunderstood that. That is exactly an important question. NIST lately moved away from a pure cipher-only view of cryptography to a more holistic view (i.e. where are ciphers used). That said, for 3DES there is no formal requirement that the 3 keys must be checked. NIST is fine when documentation says how the keys are generated by some logic outside the module. > > Anyway, if I understand you correctly, what you are saying is that > these checks might make an > implementation RFC 2451 and SP800-131A compliant respectively but they > are not required for > FIPS 140-2 compliance? did I understand that correctly? Correct. Regarding SP800-131A, it only says that a full 3key 3DES is required. It does not say whether/how the key shall be enforced not being identical. > > If so, since two 3DES implementation in the kernel already do the RFC > 2451 compliant check I assume > you would not object to the ccree driver using the same function, > disconnect from FIPS being set or > not, just as is being done with the other 3DES implementation. Absolutely. If possible, all 3DES implementations should use the same checking functions. The existing checking function regarding the prevention of 1key 3DES should be used by your code too. All I am saying that from a FIPS perspective, there is no need to extent the common function by a 3key 3DES enforcer. > > In addition, would it be OK if we did an extra check in the ccree > driver for SP800-131A key compliance > and disable encryption (but allow decryption) if the key fails the > check? again, this would be independent > from FIPS mode? My personal taste would be: changes that makes sense for all 3DES implementations should go to a common function. Otherwise, 3DES implementation A behaves differently than impl. B. That said, having a check that all three keys are non-identical would certainly be good (see my Ack to the patch from a year ago). But you cannot use the argument that FIPS mandates it to push it through. :-) Ciao Stephan PS: There is currently a new requirement being discussed for FIPS: 3DES operations should not allow more than 4GB of data to be encrypted with one key. This requirement would need technical enforcement. I am looking into this one for some time now.
Re: [PATCH v2 6/9] staging: ccree: add FIPS support
On Mon, Apr 24, 2017 at 9:21 AM, Stephan Müller wrote: > Am Montag, 24. April 2017, 08:16:50 CEST schrieb Stephan Müller: > > Hi Gilad, > >> > >> > int __des3_ede_setkey(u32 *expkey, u32 *flags, const u8 *key, >> > >> > unsigned int keylen) >> > >> > However, this does not check that k1 == k3. In this case DES3 >> > becomes 2DES (2-keys TDEA), the use of which was dropped post 2015 >> > by NIST Special Publication 800-131A*. >> >> It is correct that the RFC wants at least a 2key 3DES. And it is correct >> that SP800-131A mandates 3key 3DES post 2015. All I am saying is that FIPS >> 140-2 does *not* require a technical verification of the 3 keys being not >> identical. > > One side note: we had discussed a patch to this function in the past, see [1]. > > [1] https://patchwork.kernel.org/patch/8293441/ > Thanks, I was not aware of that. I guess we could change the function to indicate that a key is valid for decryption but not encryption and have the implementation limiting based on that if there is an interest in SP800-131A compliance. Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
Re: [PATCH v2 6/9] staging: ccree: add FIPS support
On Mon, Apr 24, 2017 at 9:16 AM, Stephan Müller wrote: > Am Montag, 24. April 2017, 08:06:09 CEST schrieb Gilad Ben-Yossef: > > Hi Gilad, >> >> Well, it turns out there is and we do :-) >> >> This is from crypto/des_generic.c: >> >> /* >> * RFC2451: >> * >> * For DES-EDE3, there is no known need to reject weak or >> * complementation keys. Any weakness is obviated by the use of >> * multiple keys. >> * >> * However, if the first two or last two independent 64-bit keys are >> * equal (k1 == k2 or k2 == k3), then the DES3 operation is simply the >> * same as DES. Implementers MUST reject keys that exhibit this >> * property. >> * >> */ >> int __des3_ede_setkey(u32 *expkey, u32 *flags, const u8 *key, >> unsigned int keylen) >> >> However, this does not check that k1 == k3. In this case DES3 >> becomes 2DES (2-keys TDEA), the use of which was dropped post 2015 >> by NIST Special Publication 800-131A*. > > It is correct that the RFC wants at least a 2key 3DES. And it is correct that > SP800-131A mandates 3key 3DES post 2015. All I am saying is that FIPS 140-2 > does *not* require a technical verification of the 3 keys being not identical. > > Note, formally, FIPS 140-2 requires that the 3 keys (i.e. all 192 bits) must > be obtained from *one* call to a DRBG or KDF (separate independent calls to, > say, obtain one key at a time is *not* permitted). Of course, fixing the > parity bits is allowed after obtaining the random number. >> >> Would it be acceptable if I offer a patch adding this check to >> __des3_ede_setkey() >> and use that in the ccree driver? > > I am not sure it makes sense as the core requirement is the *one* invocation > of the DRBG. Thanks you for the clarification. As I think is obvious by now I am not a FIPS expert by any stretch. Isn't the requirements on DRBG or KDF invocations pertain to key generation only? What happens if you don't derive the keys on the system, but wish to use keys derived elsewhere? I assumed the limitations on weak keys etc. were meant to protect against that scenario and are therefore independent from key generation requirements, but I may have misunderstood that. Anyway, if I understand you correctly, what you are saying is that these checks might make an implementation RFC 2451 and SP800-131A compliant respectively but they are not required for FIPS 140-2 compliance? did I understand that correctly? If so, since two 3DES implementation in the kernel already do the RFC 2451 compliant check I assume you would not object to the ccree driver using the same function, disconnect from FIPS being set or not, just as is being done with the other 3DES implementation. In addition, would it be OK if we did an extra check in the ccree driver for SP800-131A key compliance and disable encryption (but allow decryption) if the key fails the check? again, this would be independent from FIPS mode? Thanks again, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru