Re: [PATCH 7/7] liquidio: use pcie_flr instead of duplicating it

[Felix Manlunas]

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, );
> - if (status & PCI_EXP_DEVSTA_TRPND) {
> - dev_info(>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,
> -   );
> - if (status & PCI_EXP_DEVSTA_TRPND)
> - dev_info(>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

[Dennis Dalessandro]

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.

[Thiago Jung Bauermann]

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

[Christoph Hellwig]

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

[Corentin Labbe]

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

[Corentin Labbe]

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

[Corentin Labbe]

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

[Corentin Labbe]

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

[Corentin Labbe]

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(>u.sha.opad_sg, ctx->u.sha.opad, block_size);
-- 
2.10.2

[PATCH 6/9] crypto: omap-sham - Use IPAD/OPAD constant

[Corentin Labbe]

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

[Corentin Labbe]

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

[Corentin Labbe]

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

[Corentin Labbe]

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

[Byczkowski, Jakub]

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, );
-   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 

Re: [PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

[Stephan Müller]

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(, 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(>ring[ring].egress_lock);
> + do {
> + rdesc = safexcel_ring_next_rptr(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,
> + "cipher: 

Re: [PATCH 2/3] crypto: cavium: Remove the individual encrypt/decrypt function for each algorithm

[Herbert Xu]

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

[Herbert Xu]

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

[Herbert Xu]

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

[Herbert Xu]

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)

[Herbert Xu]

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 v2] crypto: algif_aead - Require setkey before accept(2)

[Stephan Müller]

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)

[Stephan Müller]

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;
 }
 
 

Re: [PATCH] crypto: algif_aead - Require setkey before accept(2)

[Herbert Xu]

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)

[Stephan Müller]

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(>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

[Herbert Xu]

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 = >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

[Antoine Tenart]

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

[Igal Liberman]

[...]

> + 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)

[Herbert Xu]

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

[Herbert Xu]

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

[Ard Biesheuvel]

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

[Herbert Xu]

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

[Antoine Tenart]

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

[Antoine Tenart]

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 = <_syscon0 1 26>;
+   status = "disabled";
+   };
-- 
2.11.0

[PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

[Antoine Tenart]

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 * 

[PATCH v3 0/3] arm64: marvell: add cryptographic engine support for 7k/8k

[Antoine Tenart]

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

[Stephan Müller]

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

[Stephan Müller]

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

[Gilad Ben-Yossef]

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

Re: [PATCH] crypto: ecc : Wipe memory for p on errors.

[Loganaden Velvindron]

On Sun, Apr 23, 2017 at 09:14:18PM +0200, Stephan Müller wrote:
> Am Samstag, 22. April 2017, 13:31:34 CEST schrieb Loganaden Velvindron:
> 
> Hi Loganaden,
> 
> > -   kfree(p);
> > +   kzfree(p);
> 
> Why do you think this is necessary? p is not filled in this function as this 
> function only allocates the memory.
On 2nd reading, p doesn't contain any sensitive information in this function,
and doesn't need to be zero'ed.

Thank you Stephan. Please disregard the patch.


> 
> Ciao
> Stephan

Re: [PATCH v2 6/9] staging: ccree: add FIPS support

[Stephan Müller]

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/

Ciao
Stephan

Re: [PATCH v2 6/9] staging: ccree: add FIPS support

[Stephan Müller]

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.

Ciao
Stephan

Re: [PATCH v2 6/9] staging: ccree: add FIPS support

[Gilad Ben-Yossef]

On Sun, Apr 23, 2017 at 12:48 PM, Gilad Ben-Yossef  wrote:
> Hi,
>
> Thank you for the review.
>
> On Thu, Apr 20, 2017 at 4:39 PM, Stephan Müller  wrote:
>
>>> +/* The function verifies that tdes keys are not weak.*/
>>> +static int ssi_fips_verify_3des_keys(const u8 *key, unsigned int keylen)
>>> +{
>>> +#ifdef CCREE_FIPS_SUPPORT
>>> +tdes_keys_t *tdes_key = (tdes_keys_t*)key;
>>> +
>>> + /* verify key1 != key2 and key3 != key2*/
>>
>> I do not think that this check is necessary. There is no FIPS requirement or
>> IG that mandates this (unlike the XTS key check).
>>
>> If there would be such requirement, we would need a common service function
>> for all TDES implementations


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*.

Would it be acceptable if I offer a patch adding this check to
__des3_ede_setkey()
and use that in the ccree driver?

* http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf


Many thanks,
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] crypto: tcrypt: Zero iv before freeing it

[Codarren Velvindron]

On Sun, Apr 23, 2017 at 09:11:04PM +0200, Stephan Müller wrote:
> Am Sonntag, 23. April 2017, 18:12:04 CEST schrieb Codarren Velvindron:
> 
> Hi Codarren,
> 
> > Signed-off-by: Codarren Velvindron 
> > ---
> >  crypto/tcrypt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
> > index 9a11f3c..48a721c 100644
> > --- a/crypto/tcrypt.c
> > +++ b/crypto/tcrypt.c
> > @@ -384,7 +384,7 @@ static void test_aead_speed(const char *algo, int enc,
> > unsigned int secs, out_noaxbuf:
> > testmgr_free_buf(xbuf);
> >  out_noxbuf:
> > -   kfree(iv);
> > +   kzfree(iv);
> 
> Why would you think this is needed?
> 
>if (iv_len)
>   memset(iv, 0xff, iv_len);
> 
> 
> Ciao
> Stephan
Hello, 

Thanks for bringing this up to my attention.

Kindly disregard the patch.

Best Regards,
Codarren