Re: [PATCH 2/4] crypto: stm32 - Support for STM32 CRC32 crypto module

[Corentin Labbe]

On Tue, Mar 14, 2017 at 05:37:24PM +0100, Fabien Dessenne wrote:
> +static int stm32_crc_remove(struct platform_device *pdev)
> +{
> + struct stm32_crc *crc = platform_get_drvdata(pdev);
> +
> + spin_lock(_list.lock);
> + list_del(>list);
> + spin_unlock(_list.lock);
> +
> + crypto_unregister_shash(algs);
> +
> + if (crc && crc->clk)
> + clk_disable_unprepare(crc->clk);

Hello

No need to test for crc, if it was NULL, you dereference it just before in 
list_del.
Furthermore, It seems that clk_disable_unprepare() handle perfectly NULL, so no 
test is needed.

Regards
Corentin Labbe

Re: [PATCH v6 0/4] Broadcom SBA RAID support

[Shaohua Li]

On Tue, Mar 14, 2017 at 09:56:35AM -0700, Dan Williams wrote:
> On Mon, Mar 6, 2017 at 1:43 AM, Anup Patel  wrote:
> > The Broadcom SBA RAID is a stream-based device which provides
> > RAID5/6 offload.
> >
> > It requires a SoC specific ring manager (such as Broadcom FlexRM
> > ring manager) to provide ring-based programming interface. Due to
> > this, the Broadcom SBA RAID driver (mailbox client) implements
> > DMA device having one DMA channel using a set of mailbox channels
> > provided by Broadcom SoC specific ring manager driver (mailbox
> > controller).
> >
> > The Broadcom SBA RAID hardware requires PQ disk position instead
> > of PQ disk coefficient. To address this, we have added raid_gflog
> > table which will help driver to convert PQ disk coefficient to PQ
> > disk position.
> >
> > This patchset is based on Linux-4.11-rc1 and depends on patchset
> > "[PATCH v5 0/2] Broadcom FlexRM ring manager support"
> >
> > It is also available at sba-raid-v6 branch of
> > https://github.com/Broadcom/arm64-linux.git
> >
> [..]
> >
> > Anup Patel (4):
> >   lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position
> >   async_tx: Fix DMA_PREP_FENCE usage in do_async_gen_syndrome()
> >   dmaengine: Add Broadcom SBA RAID driver
> >   dt-bindings: Add DT bindings document for Broadcom SBA RAID driver
> 
> For the dmaengine and async_tx changes:
> 
> Acked-by: Dan Williams 
> 
> The raid change should get an ack from Shaohua.

For the raid6 part:

Acked-by: Shaohua Li 

Re: [PATCH v6 0/4] Broadcom SBA RAID support

[Dan Williams]

On Mon, Mar 6, 2017 at 1:43 AM, Anup Patel  wrote:
> The Broadcom SBA RAID is a stream-based device which provides
> RAID5/6 offload.
>
> It requires a SoC specific ring manager (such as Broadcom FlexRM
> ring manager) to provide ring-based programming interface. Due to
> this, the Broadcom SBA RAID driver (mailbox client) implements
> DMA device having one DMA channel using a set of mailbox channels
> provided by Broadcom SoC specific ring manager driver (mailbox
> controller).
>
> The Broadcom SBA RAID hardware requires PQ disk position instead
> of PQ disk coefficient. To address this, we have added raid_gflog
> table which will help driver to convert PQ disk coefficient to PQ
> disk position.
>
> This patchset is based on Linux-4.11-rc1 and depends on patchset
> "[PATCH v5 0/2] Broadcom FlexRM ring manager support"
>
> It is also available at sba-raid-v6 branch of
> https://github.com/Broadcom/arm64-linux.git
>
[..]
>
> Anup Patel (4):
>   lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position
>   async_tx: Fix DMA_PREP_FENCE usage in do_async_gen_syndrome()
>   dmaengine: Add Broadcom SBA RAID driver
>   dt-bindings: Add DT bindings document for Broadcom SBA RAID driver

For the dmaengine and async_tx changes:

Acked-by: Dan Williams 

The raid change should get an ack from Shaohua.

[PATCH 4/4] ARM: dts: stm32: enable CRC on stm32746g-eval board

[Fabien Dessenne]

Enable the CRC (CRC32 crypto) on stm32746g-eval board

Signed-off-by: Fabien Dessenne 
---
 arch/arm/boot/dts/stm32746g-eval.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/stm32746g-eval.dts 
b/arch/arm/boot/dts/stm32746g-eval.dts
index aa03fac..0dc18a0 100644
--- a/arch/arm/boot/dts/stm32746g-eval.dts
+++ b/arch/arm/boot/dts/stm32746g-eval.dts
@@ -89,6 +89,10 @@
clock-frequency = <2500>;
 };
 
+ {
+   status = "okay";
+};
+
  {
pinctrl-0 = <_pins_a>;
pinctrl-names = "default";
-- 
2.7.4

[PATCH 1/4] dt-bindings: Document STM32 CRC bindings

[Fabien Dessenne]

Document device tree bindings for the STM32 CRC (crypto CRC32)

Signed-off-by: Fabien Dessenne 
---
 .../devicetree/bindings/crypto/st,stm32-crc.txt  | 16 
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/st,stm32-crc.txt

diff --git a/Documentation/devicetree/bindings/crypto/st,stm32-crc.txt 
b/Documentation/devicetree/bindings/crypto/st,stm32-crc.txt
new file mode 100644
index 000..2312e8a
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/st,stm32-crc.txt
@@ -0,0 +1,16 @@
+* STMicroelectronics STM32 CRC
+
+Required properties:
+- compatible: shoudl be "st,stm32f7-crc".
+- reg: The address and length of the peripheral registers space
+- clocks: The input clock of the CRC instance
+
+Optional properties: none
+
+Example:
+
+crc: crc@40023000 {
+   compatible = "st,stm32f7-crc";
+   reg = <0x40023000 0x400>;
+   clocks = < 0 12>;
+};
-- 
2.7.4

[PATCH 2/4] crypto: stm32 - Support for STM32 CRC32 crypto module

[Fabien Dessenne]

This module registers a CRC32 ("Ethernet") and a CRC32C (Castagnoli)
algorithm that make use of the STMicroelectronics STM32 crypto hardware.

Theses algorithms are compatible with the little-endian generic ones.
Both algorithms use ~0 as default seed (key).
With CRC32C the output is xored with ~0.

Using TCRYPT CRC32C speed test, this shows up to 900% speedup compared
to the crc32c-generic algorithm.

Signed-off-by: Fabien Dessenne 
---
 drivers/crypto/Kconfig |   2 +
 drivers/crypto/Makefile|   1 +
 drivers/crypto/stm32/Kconfig   |   8 +
 drivers/crypto/stm32/Makefile  |   2 +
 drivers/crypto/stm32/stm32_crc32.c | 325 +
 5 files changed, 338 insertions(+)
 create mode 100644 drivers/crypto/stm32/Kconfig
 create mode 100644 drivers/crypto/stm32/Makefile
 create mode 100644 drivers/crypto/stm32/stm32_crc32.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 473d312..922b323 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -619,4 +619,6 @@ config CRYPTO_DEV_BCM_SPU
  Secure Processing Unit (SPU). The SPU driver registers ablkcipher,
  ahash, and aead algorithms with the kernel cryptographic API.
 
+source "drivers/crypto/stm32/Kconfig"
+
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 7396094..95bf2f9 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
 obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
 obj-$(CONFIG_CRYPTO_DEV_S5P) += s5p-sss.o
 obj-$(CONFIG_CRYPTO_DEV_SAHARA) += sahara.o
+obj-$(CONFIG_CRYPTO_DEV_STM32) += stm32/
 obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
 obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
 obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
diff --git a/drivers/crypto/stm32/Kconfig b/drivers/crypto/stm32/Kconfig
new file mode 100644
index 000..792335b
--- /dev/null
+++ b/drivers/crypto/stm32/Kconfig
@@ -0,0 +1,8 @@
+config CRYPTO_DEV_STM32
+   tristate "Support for STM32 crypto accelerators"
+   depends on ARCH_STM32
+   select CRYPTO_HASH
+   help
+  This enables support for the CRC32 hw accelerator which can be found
+ on STMicroelectronis STM32 SOC.
+
diff --git a/drivers/crypto/stm32/Makefile b/drivers/crypto/stm32/Makefile
new file mode 100644
index 000..73b4c6e
--- /dev/null
+++ b/drivers/crypto/stm32/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_CRYPTO_DEV_STM32) += stm32_cryp.o
+stm32_cryp-objs := stm32_crc32.o
diff --git a/drivers/crypto/stm32/stm32_crc32.c 
b/drivers/crypto/stm32/stm32_crc32.c
new file mode 100644
index 000..ba780a6
--- /dev/null
+++ b/drivers/crypto/stm32/stm32_crc32.c
@@ -0,0 +1,325 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2017
+ * Author: Fabien Dessenne 
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+
+#define DRIVER_NAME "stm32-crc32"
+#define CHKSUM_DIGEST_SIZE  4
+#define CHKSUM_BLOCK_SIZE   1
+
+/* Registers */
+#define CRC_DR  0x
+#define CRC_CR  0x0008
+#define CRC_INIT0x0010
+#define CRC_POL 0x0014
+
+/* Registers values */
+#define CRC_CR_RESETBIT(0)
+#define CRC_CR_REVERSE  (BIT(7) | BIT(6) | BIT(5))
+#define CRC_INIT_DEFAULT0x
+
+/* Polynomial reversed */
+#define POLY_CRC32  0xEDB88320
+#define POLY_CRC32C 0x82F63B78
+
+struct stm32_crc {
+   struct list_head list;
+   struct device*dev;
+   void __iomem *regs;
+   struct clk   *clk;
+   u8   pending_data[sizeof(u32)];
+   size_t   nb_pending_bytes;
+};
+
+struct stm32_crc_list {
+   struct list_head dev_list;
+   spinlock_t   lock; /* protect dev_list */
+};
+
+static struct stm32_crc_list crc_list = {
+   .dev_list = LIST_HEAD_INIT(crc_list.dev_list),
+   .lock = __SPIN_LOCK_UNLOCKED(crc_list.lock),
+};
+
+struct stm32_crc_ctx {
+   u32 key;
+   u32 poly;
+};
+
+struct stm32_crc_desc_ctx {
+   u32partial; /* crc32c: partial in first 4 bytes of that struct */
+   struct stm32_crc *crc;
+};
+
+static int stm32_crc32_cra_init(struct crypto_tfm *tfm)
+{
+   struct stm32_crc_ctx *mctx = crypto_tfm_ctx(tfm);
+
+   mctx->key = CRC_INIT_DEFAULT;
+   mctx->poly = POLY_CRC32;
+   return 0;
+}
+
+static int stm32_crc32c_cra_init(struct crypto_tfm *tfm)
+{
+   struct stm32_crc_ctx *mctx = crypto_tfm_ctx(tfm);
+
+   mctx->key = CRC_INIT_DEFAULT;
+   mctx->poly = POLY_CRC32C;
+   return 0;
+}
+
+static int stm32_crc_setkey(struct crypto_shash *tfm, const u8 *key,
+   unsigned int keylen)
+{
+   struct stm32_crc_ctx *mctx = crypto_shash_ctx(tfm);
+
+   if (keylen != 

[PATCH 0/4] STM32 CRC crypto driver

[Fabien Dessenne]

This set of patches adds a new crypto driver for STMicroelectronics stm32f746.
The drivers uses the crypto API and provides with an HW-enabled CRC32 algorithm.
It was developed and tested (tcrypt / testmgr) on evaluation board stm32746g.

Fabien Dessenne (4):
  dt-bindings: Document STM32 CRC bindings
  crypto: stm32 - Support for STM32 CRC32 crypto module
  ARM: dts: stm32: Add CRC support to stm32f746
  ARM: dts: stm32: enable CRC on stm32746g-eval board

 .../devicetree/bindings/crypto/st,stm32-crc.txt|  16 +
 arch/arm/boot/dts/stm32746g-eval.dts   |   4 +
 arch/arm/boot/dts/stm32f746.dtsi   |   7 +
 drivers/crypto/Kconfig |   2 +
 drivers/crypto/Makefile|   1 +
 drivers/crypto/stm32/Kconfig   |   8 +
 drivers/crypto/stm32/Makefile  |   2 +
 drivers/crypto/stm32/stm32_crc32.c | 325 +
 8 files changed, 365 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/st,stm32-crc.txt
 create mode 100644 drivers/crypto/stm32/Kconfig
 create mode 100644 drivers/crypto/stm32/Makefile
 create mode 100644 drivers/crypto/stm32/stm32_crc32.c

-- 
2.7.4

[PATCH 3/4] ARM: dts: stm32: Add CRC support to stm32f746

[Fabien Dessenne]

Add CRC (CRC32 crypto) support to stm32f746.

Signed-off-by: Fabien Dessenne 
---
 arch/arm/boot/dts/stm32f746.dtsi | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index f321ffe..755fb92 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -289,6 +289,13 @@
};
};
 
+   crc: crc@40023000 {
+   compatible = "st,stm32f7-crc";
+   reg = <0x40023000 0x400>;
+   clocks = < 0 12>;
+   status = "disabled";
+   };
+
rcc: rcc@40023800 {
#clock-cells = <2>;
compatible = "st,stm32f42xx-rcc", "st,stm32-rcc";
-- 
2.7.4

Re: crypto: deadlock between crypto_alg_sem/rtnl_mutex/genl_mutex

[Sowmini Varadhan]

On (03/14/17 09:14), Dmitry Vyukov wrote:
> Another one now involving rds_tcp_listen_stop
   :
> kworker/u4:1/19 is trying to acquire lock:
>  (sk_lock-AF_INET){+.+.+.}, at: [] lock_sock
> include/net/sock.h:1460 [inline]
>  (sk_lock-AF_INET){+.+.+.}, at: []
> rds_tcp_listen_stop+0x5c/0x150 net/rds/tcp_listen.c:288
> 
> but task is already holding lock:
>  (rtnl_mutex){+.+.+.}, at: [] rtnl_lock+0x17/0x20
> net/core/rtnetlink.c:70

Is this also a false positive?

genl_lock_dumpit takes the genl_lock and then waits on the rtnl_lock
(e.g., out of tipc_nl_bearer_dump).

netdev_run_todo takes the rtnl_lock and then wants lock_sock()
for the TCP/IPv4 socket. 

Why is lockdep seeing a circular dependancy here? Same pattern
seems to be happening  for 
  http://www.spinics.net/lists/netdev/msg423368.html
and maybe also http://www.spinics.net/lists/netdev/msg423323.html?

--Sowmini

> Chain exists of:
>   sk_lock-AF_INET --> genl_mutex --> rtnl_mutex
> 
>  Possible unsafe locking scenario:
> 
>CPU0CPU1
>
>   lock(rtnl_mutex);
>lock(genl_mutex);
>lock(rtnl_mutex);
>   lock(sk_lock-AF_INET);
> 
>  *** DEADLOCK ***
> 
> 4 locks held by kworker/u4:1/19:
>  #0:  ("%s""netns"){.+.+.+}, at: []
> __write_once_size include/linux/compiler.h:283 [inline]
>  #0:  ("%s""netns"){.+.+.+}, at: [] atomic64_set
> arch/x86/include/asm/atomic64_64.h:33 [inline]
>  #0:  ("%s""netns"){.+.+.+}, at: [] atomic_long_set
> include/asm-generic/atomic-long.h:56 [inline]
>  #0:  ("%s""netns"){.+.+.+}, at: [] set_work_data
> kernel/workqueue.c:617 [inline]
>  #0:  ("%s""netns"){.+.+.+}, at: []
> set_work_pool_and_clear_pending kernel/workqueue.c:644 [inline]
>  #0:  ("%s""netns"){.+.+.+}, at: []
> process_one_work+0xab3/0x1c10 kernel/workqueue.c:2089
>  #1:  (net_cleanup_work){+.+.+.}, at: []
> process_one_work+0xb07/0x1c10 kernel/workqueue.c:2093
>  #2:  (net_mutex){+.+.+.}, at: []
> cleanup_net+0x22b/0xa90 net/core/net_namespace.c:429
>  #3:  (rtnl_mutex){+.+.+.}, at: []
> rtnl_lock+0x17/0x20 net/core/rtnetlink.c:70
> 

Re: [PATCH V2 2/3] crypto: ccp - Enable support for AES GCM on v5 CCPs

[Stephan Müller]

Am Dienstag, 14. März 2017, 15:34:00 CET schrieb Gary R Hook:

Hi Gary,

> On 03/14/2017 02:17 AM, Stephan Müller wrote:
> > Am Montag, 13. März 2017, 20:35:07 CET schrieb Gary R Hook:
> > 
> > Hi Gary,
> 
> Is it acceptable to snip stuff out? Most of this code seems irrelevant
> to this discussion

Let us snip it :-)

> >> > 
> >> > But then, the key is 4 bytes longer than a normal AES key as it
> >> > contains
> >> > the leading 32 bits of the IV.
> >> 
> >> I had my wires crossed due to an incomplete understanding of an AEAD
> >> cipher
> >> in general, and GCM in particular. I'm hopeful that someone can help me
> >> understand:
> >> 
> >> For the AES GCM encryption tests in testmgr.h, where there is an IV,
> >> they're all
> >> 12 bytes in length. As I understand AES GCM the IV can be anywhere from
> >> 1 to 2^64
> >> bits in length; the value of 96 makes for convenience and efficiency.
> >> But it's
> >> neither a requirement nor restriction.
> > 
> > That is correct. For longer IVs, you would need to use Ghash to compress
> > it to
> > 96 bits. The remaining 32 bits to get to one AES block is the counter
> > that is
> > used for the CTR AES mode in GCM.
> 
> Yes, understood. It's all falling into place now. What seems to be missing
> (to me) is a way for the transform to indicate that it allows all valid
> (GCM)
> IV lengths, as opposed to the (specified in the data structure) 12 bytes.

The kernel crypto API does not support varying IV sizes. So, simply stay with 
12 / 8 bytes as explained should suffice.

> I
> get the context of IPSec, but I would think AF_ALG allowing access to the
> transforms means that we can't rely upon a context. And there seems to be no
> way for an implementation to let a user know about any IV restrictions (or
> not).

In algif_aead, we simply have:

unsigned ivsize =
crypto_aead_ivsize(crypto_aead_reqtfm(>aead_req));
...
if (con.iv && con.iv->ivlen != ivsize)
return -EINVAL;

Thus, if the user space caller does not provide exactly ivsize bytes of IV, he 
gets an error.
> 
> Do we just let the implementation return an error when it can't handle
> something?
> 
> Or (highly possible) am I missing the obvious?
> 
> >> There are no tests (in testmgr.h) that use an IV length other than  0 or
> >> 96.> 
> > See aes_gcm_rfc4106_enc_tv_template for other types of IV.
> 
> All 8 bytes, it seems, which makes sense for 4106.
> 
> >> My comment about RFC4106 has to do with requiring an IV 0f 96 bits + a
> >> word
> >> that
> >> is incremented for each block (making every nonce unique, per the
> >> requirement).
> >> But let's ignore that, please.
> >> 
> >> It looks as if:
> >> 
> >> What seems to be missing is the ability to register a (GCM) transform
> >> that can
> >> handle an IV of arbitrary (allowable) length. I have to specify the
> >> length (ivsize)
> >> when I register an algorithm, and everything I see in the existing code
> >> appears
> >> to expect a GCM ivsize to be 96 bits, period (or zero). This is what I
> >> meant when
> >> I referenced RFC4106: I perceive restrictions not in my code, but n the
> >> way GCM seems
> >> to be supported in the crypto AEAD framework. A complete GCM
> >> implementation would not
> >> seem to have a restriction to a specific IV length (rather, a range of
> >> allowed
> >> values).
> > 
> > 96 bits is the use case in IPSEC. As the kernel crypto API transforms
> > are used
> > for IPSEC. Nobody would prevent you from supporting other IV sizes. But
> > then
> > you would need to add a Ghash operation to compress it to the right
> > length. No
> > other GCM implementation has that and hence the limitation.
> 
> Of course. That's the component that I'm missing, and I want to understand
> whether there's a compelling need.

I would not think that there is any need for it. If there would be, a generic 
helper for the IV compression should be added instead of having the algos 
implementing it itself over and over again.
> 
> > But 96 bits is not the common case. See the 4106 implementations, you
> > see the
> > ivsize being 8. This is correct because setkey requires AES keysize + 4
> > bytes
> > in length (see crypto_rfc4106_setkey for an example). The trailing 4
> > bytes of
> > the key are the initial 4 bytes of the GCM IV.
> 
> Yes. The RFC4106 document is pretty clear on the layout of the IV.
> 
> > My comment was about your comment to refer to RFC4106. I just wanted to
> > understand your code and and make sense of your comments. :-)
> > 
> >> Is my reading of the GCM description in error? Do we need/want the
> >> ability
> >> to have a flexible IV length for GCM? What am I not understanding?
> > 
> > In your case, just change the wording in the comment slightly and we are
> > all good.
> 
> Will do. I appreciate the discussion! Very helpful.
> 

Ciao
Stephan

Re: [PATCH V2 2/3] crypto: ccp - Enable support for AES GCM on v5 CCPs

[Gary R Hook]

On 03/14/2017 02:17 AM, Stephan Müller wrote:

Am Montag, 13. März 2017, 20:35:07 CET schrieb Gary R Hook:

Hi Gary,


Is it acceptable to snip stuff out? Most of this code seems irrelevant 
to this discussion





On 03/03/2017 7:15 AM, Stephan Mueller wrote:
> Am Donnerstag, 2. März 2017, 22:26:54 CET schrieb Gary R Hook:
>
> Hi Gary,

Thanks for your comments, Stephan.

> > A version 5 device provides the primitive commands
> > required for AES GCM. This patch adds support for
> > en/decryption.
> >
> > Signed-off-by: Gary R Hook 
> > ---
> >
> >  drivers/crypto/ccp/Makefile|1
> >  drivers/crypto/ccp/ccp-crypto-aes-galois.c |  257
> >
> >  drivers/crypto/ccp/ccp-crypto-main.c
> > |
> > 12 +
> >
> >  drivers/crypto/ccp/ccp-crypto.h|   14 ++
> >  drivers/crypto/ccp/ccp-dev-v5.c|2
> >  drivers/crypto/ccp/ccp-dev.h   |1
> >  drivers/crypto/ccp/ccp-ops.c   |  252
> >
> > +++ include/linux/ccp.h|
> > 9 +
> >
> >  8 files changed, 548 insertions(+)
> >  create mode 100644 drivers/crypto/ccp/ccp-crypto-aes-galois.c
> >
> > diff --git a/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > b/drivers/crypto/ccp/ccp-crypto-aes-galois.c new file mode 100644
> > index 000..8bc18c9
> > --- /dev/null
> > +++ b/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > @@ -0,0 +1,257 @@
> > +/*
> > + * AMD Cryptographic Coprocessor (CCP) AES GCM crypto API support
> > + *
> > + * Copyright (C) 2016 Advanced Micro Devices, Inc.
> > + *
> > + * Author: Gary R Hook 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "ccp-crypto.h"
> > +
> > +#defineAES_GCM_IVSIZE  12
> > +
> > +static int ccp_aes_gcm_complete(struct crypto_async_request *async_req,
> > int ret) +{
> > +   return ret;
> > +}
> > +
> > +static int ccp_aes_gcm_setkey(struct crypto_aead *tfm, const u8 *key,
> > + unsigned int key_len)
> > +{
> > +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> > +
> > +   switch (key_len) {
> > +   case AES_KEYSIZE_128:
> > +   ctx->u.aes.type = CCP_AES_TYPE_128;
> > +   break;
> > +   case AES_KEYSIZE_192:
> > +   ctx->u.aes.type = CCP_AES_TYPE_192;
> > +   break;
> > +   case AES_KEYSIZE_256:
> > +   ctx->u.aes.type = CCP_AES_TYPE_256;
> > +   break;
> > +   default:
> > +   crypto_aead_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> > +   return -EINVAL;
> > +   }
> > +
> > +   ctx->u.aes.mode = CCP_AES_MODE_GCM;
> > +   ctx->u.aes.key_len = key_len;
> > +
> > +   memcpy(ctx->u.aes.key, key, key_len);
> > +   sg_init_one(>u.aes.key_sg, ctx->u.aes.key, key_len);
> > +
> > +   return 0;
> > +}
> > +
> > +static int ccp_aes_gcm_setauthsize(struct crypto_aead *tfm,
> > +  unsigned int authsize)
> > +{
> > +   return 0;
> > +}
> > +
> > +static int ccp_aes_gcm_crypt(struct aead_request *req, bool encrypt)
> > +{
> > +   struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> > +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> > +   struct ccp_aes_req_ctx *rctx = aead_request_ctx(req);
> > +   struct scatterlist *iv_sg = NULL;
> > +   unsigned int iv_len = 0;
> > +   int i;
> > +   int ret = 0;
> > +
> > +   if (!ctx->u.aes.key_len)
> > +   return -EINVAL;
> > +
> > +   if (ctx->u.aes.mode != CCP_AES_MODE_GCM)
> > +   return -EINVAL;
> > +
> > +   if (!req->iv)
> > +   return -EINVAL;
> > +
> > +   /*
> > +* 5 parts:
> > +*   plaintext/ciphertext input
> > +*   AAD
> > +*   key
> > +*   IV
> > +*   Destination+tag buffer
> > +*/
> > +
> > +   /* According to the way AES GCM has been implemented here,
> > +* per RFC 4106 it seems, the provided IV is fixed at 12 bytes,
>
> When you have that restriction, should the cipher be called
> rfc4106(gcm(aes))?
>
> But then, the key is 4 bytes longer than a normal AES key as it contains
> the leading 32 bits of the IV.

I had my wires crossed due to an incomplete understanding of an AEAD cipher
in general, and GCM in particular. I'm hopeful that someone can help me
understand:

For the AES GCM encryption tests in testmgr.h, where there is an IV,
they're all
12 bytes in length. As I understand AES GCM the IV can be anywhere from
1 to 2^64
bits in length; the value of 96 makes for convenience and efficiency.
But it's
neither a requirement nor restriction.


That is correct. For longer IVs, you would need to use Ghash to compress
it to
96 bits. The remaining 32 bits to get to one AES block is 

[PATCH 2/2] hwrng: geode - Revert managed API changes

[Prarit Bhargava]

After commit e9afc746299d ("hwrng: geode - Use linux/io.h instead of
asm/io.h") the geode-rng driver uses devres with pci_dev->dev to keep
track of resources, but does not actually register a PCI driver.  This
results in the following issues:

1.  The driver leaks memory because the driver does not attach to a
device.  The driver only uses the PCI device as a reference.   devm_*()
functions will release resources on driver detach, which the geode-rng
driver will never do.  As a result,

2.  The driver cannot be reloaded because there is always a use of the
ioport and region after the first load of the driver.

Revert the changes made by  e9afc746299d ("hwrng: geode - Use linux/io.h
instead of asm/io.h").

Signed-off-by: Prarit Bhargava 
Fixes: 6e9b5e76882c ("hwrng: geode - Migrate to managed API")
Cc: Matt Mackall 
Cc: Herbert Xu 
Cc: Corentin LABBE 
Cc: PrasannaKumar Muralidharan 
Cc: Wei Yongjun 
Cc: linux-crypto@vger.kernel.org
Cc: linux-ge...@lists.infradead.org
---
 drivers/char/hw_random/geode-rng.c |   50 +---
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/drivers/char/hw_random/geode-rng.c 
b/drivers/char/hw_random/geode-rng.c
index e7a245942029..e1d421a36a13 100644
--- a/drivers/char/hw_random/geode-rng.c
+++ b/drivers/char/hw_random/geode-rng.c
@@ -31,6 +31,9 @@
 #include 
 #include 
 
+
+#define PFXKBUILD_MODNAME ": "
+
 #define GEODE_RNG_DATA_REG   0x50
 #define GEODE_RNG_STATUS_REG 0x54
 
@@ -82,6 +85,7 @@ static int geode_rng_data_present(struct hwrng *rng, int wait)
 
 static int __init mod_init(void)
 {
+   int err = -ENODEV;
struct pci_dev *pdev = NULL;
const struct pci_device_id *ent;
void __iomem *mem;
@@ -89,27 +93,43 @@ static int __init mod_init(void)
 
for_each_pci_dev(pdev) {
ent = pci_match_id(pci_tbl, pdev);
-   if (ent) {
-   rng_base = pci_resource_start(pdev, 0);
-   if (rng_base == 0)
-   return -ENODEV;
-
-   mem = devm_ioremap(>dev, rng_base, 0x58);
-   if (!mem)
-   return -ENOMEM;
-   geode_rng.priv = (unsigned long)mem;
-
-   pr_info("AMD Geode RNG detected\n");
-   return devm_hwrng_register(>dev, _rng);
-   }
+   if (ent)
+   goto found;
}
-
/* Device not found. */
-   return -ENODEV;
+   goto out;
+
+found:
+   rng_base = pci_resource_start(pdev, 0);
+   if (rng_base == 0)
+   goto out;
+   err = -ENOMEM;
+   mem = ioremap(rng_base, 0x58);
+   if (!mem)
+   goto out;
+   geode_rng.priv = (unsigned long)mem;
+
+   pr_info("AMD Geode RNG detected\n");
+   err = hwrng_register(_rng);
+   if (err) {
+   pr_err(PFX "RNG registering failed (%d)\n",
+  err);
+   goto err_unmap;
+   }
+out:
+   return err;
+
+err_unmap:
+   iounmap(mem);
+   goto out;
 }
 
 static void __exit mod_exit(void)
 {
+   void __iomem *mem = (void __iomem *)geode_rng.priv;
+
+   hwrng_unregister(_rng);
+   iounmap(mem);
 }
 
 module_init(mod_init);
-- 
1.7.9.3

[PATCH 1/2] hwrng: amd - Revert managed API changes

[Prarit Bhargava]

After commit 31b2a73c9c5f ("hwrng: amd - Migrate to managed API"), the
amd-rng driver uses devres with pci_dev->dev to keep track of resources,
but does not actually register a PCI driver.  This results in the
following issues:

1. The message

WARNING: CPU: 2 PID: 621 at drivers/base/dd.c:349 driver_probe_device+0x38c

is output when the i2c_amd756 driver loads and attempts to register a PCI
driver.  The PCI & device subsystems assume that no resources have been
registered for the device, and the WARN_ON() triggers since amd-rng has
already do so.

2.  The driver leaks memory because the driver does not attach to a
device.  The driver only uses the PCI device as a reference.   devm_*()
functions will release resources on driver detach, which the amd-rng
driver will never do.  As a result,

3.  The driver cannot be reloaded because there is always a use of the
ioport and region after the first load of the driver.

Revert the changes made by 31b2a73c9c5f ("hwrng: amd - Migrate to managed
API").

Signed-off-by: Prarit Bhargava 
Fixes: 31b2a73c9c5f ("hwrng: amd - Migrate to managed API").
Cc: Matt Mackall 
Cc: Herbert Xu 
Cc: Corentin LABBE 
Cc: PrasannaKumar Muralidharan 
Cc: Wei Yongjun 
Cc: linux-crypto@vger.kernel.org
Cc: linux-ge...@lists.infradead.org
---
 drivers/char/hw_random/amd-rng.c |   42 ++
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index 4a99ac756f08..9959c762da2f 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -55,6 +55,7 @@
 struct amd768_priv {
void __iomem *iobase;
struct pci_dev *pcidev;
+   u32 pmbase;
 };
 
 static int amd_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
@@ -148,33 +149,58 @@ static int __init mod_init(void)
if (pmbase == 0)
return -EIO;
 
-   priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
 
-   if (!devm_request_region(>dev, pmbase + PMBASE_OFFSET,
-   PMBASE_SIZE, DRV_NAME)) {
+   if (!request_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE, DRV_NAME)) {
dev_err(>dev, DRV_NAME " region 0x%x already in use!\n",
pmbase + 0xF0);
-   return -EBUSY;
+   err = -EBUSY;
+   goto out;
}
 
-   priv->iobase = devm_ioport_map(>dev, pmbase + PMBASE_OFFSET,
-   PMBASE_SIZE);
+   priv->iobase = ioport_map(pmbase + PMBASE_OFFSET, PMBASE_SIZE);
if (!priv->iobase) {
pr_err(DRV_NAME "Cannot map ioport\n");
-   return -ENOMEM;
+   err = -EINVAL;
+   goto err_iomap;
}
 
amd_rng.priv = (unsigned long)priv;
+   priv->pmbase = pmbase;
priv->pcidev = pdev;
 
pr_info(DRV_NAME " detected\n");
-   return devm_hwrng_register(>dev, _rng);
+   err = hwrng_register(_rng);
+   if (err) {
+   pr_err(DRV_NAME " registering failed (%d)\n", err);
+   goto err_hwrng;
+   }
+   return 0;
+
+err_hwrng:
+   ioport_unmap(priv->iobase);
+err_iomap:
+   release_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE);
+out:
+   kfree(priv);
+   return err;
 }
 
 static void __exit mod_exit(void)
 {
+   struct amd768_priv *priv;
+
+   priv = (struct amd768_priv *)amd_rng.priv;
+
+   hwrng_unregister(_rng);
+
+   ioport_unmap(priv->iobase);
+
+   release_region(priv->pmbase + PMBASE_OFFSET, PMBASE_SIZE);
+
+   kfree(priv);
 }
 
 module_init(mod_init);
-- 
1.7.9.3

[PATCH 0/2] hwrng: revert managed API changes for amd and geode

[Prarit Bhargava]

When booting top-of-tree the following WARN_ON triggers in the kernel on
a 15h AMD system.

WARNING: CPU: 2 PID: 621 at drivers/base/dd.c:349 driver_probe_device+0x38c
Modules linked in: i2c_amd756(+) amd_rng sg pcspkr parport_pc(+) parport k8
CPU: 2 PID: 621 Comm: systemd-udevd Not tainted 4.11.0-0.rc1.git0.1.el7_UNS
Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./TYAN High-End 
Call Trace:
dump_stack+0x63/0x8e
__warn+0xd1/0xf0
warn_slowpath_null+0x1d/0x20
driver_probe_device+0x38c/0x470
__driver_attach+0xc9/0xf0
? driver_probe_device+0x470/0x470
bus_for_each_dev+0x5d/0x90
driver_attach+0x1e/0x20
bus_add_driver+0x1d0/0x290
driver_register+0x60/0xe0
? 0xa0037000
__pci_register_driver+0x4c/0x50
amd756_driver_init+0x1e/0x1000 [i2c_amd756]
do_one_initcall+0x51/0x1b0
? __vunmap+0x85/0xd0
? do_init_module+0x27/0x1fa
do_init_module+0x60/0x1fa
load_module+0x15d1/0x1ad0
? m_show+0x1c0/0x1c0
SYSC_finit_module+0xa9/0xd0

There are PCI devices that contain both a RNG and SMBUS device.  The
RNG device is initialized by the amd-rng driver but the driver does not
register against the device.  The SMBUS device is initialized by the
i2c-amd756 driver and registers against the device and hits the WARN_ON()
because the amd-rng driver has already allocated resources against the
device.

The amd-rng driver was incorrectly migrated to the device resource model
(devres), and after code inspection I found that the geode-rng driver was also
incorrectly migrated.  These drivers are using devres but do not register a
driver against the device, and both drivers are expecting a memory cleanup on
a driver detach that will never happen.  This results in a memory leak when the
driver is unloaded and the inability to reload the driver.

Revert 31b2a73c9c5f ("hwrng: amd - Migrate to managed API"), and 6e9b5e76882c
("hwrng: geode - Migrate to managed API").

Signed-off-by: Prarit Bhargava 
Fixes: 31b2a73c9c5f ("hwrng: amd - Migrate to managed API").
Fixes: 6e9b5e76882c ("hwrng: geode - Migrate to managed API")
Cc: Matt Mackall 
Cc: Herbert Xu 
Cc: Corentin LABBE 
Cc: PrasannaKumar Muralidharan 
Cc: Wei Yongjun 
Cc: linux-crypto@vger.kernel.org
Cc: linux-ge...@lists.infradead.org

Prarit Bhargava (2):
  hwrng: amd - Revert managed API changes
  hwrng: geode - Revert managed API changes

 drivers/char/hw_random/amd-rng.c   |   42 --
 drivers/char/hw_random/geode-rng.c |   50 +---
 2 files changed, 69 insertions(+), 23 deletions(-)

-- 
1.7.9.3

Re: crypto: deadlock between crypto_alg_sem/rtnl_mutex/genl_mutex

[Dmitry Vyukov]

On Tue, Mar 14, 2017 at 11:25 AM, Herbert Xu
 wrote:
> On Tue, Mar 14, 2017 at 10:44:10AM +0100, Dmitry Vyukov wrote:
>>
>> Yes, please.
>> Disregarding some reports is not a good way long term.
>
> Please try this patch.

Applied on bots. I should have a conclusion within a day.
Thanks!


> ---8<---
> Subject: netlink: Annotate nlk cb_mutex by protocol
>
> Currently all occurences of nlk->cb_mutex are annotated by lockdep
> as a single class.  This causes a false lcokdep cycle involving
> genl and crypto_user.
>
> This patch fixes it by dividing cb_mutex into individual classes
> based on the netlink protocol.  As genl and crypto_user do not
> use the same netlink protocol this breaks the false dependency
> loop.
>
> Reported-by: Dmitry Vyukov 
> Signed-off-by: Herbert Xu 
>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 7b73c7c..596eaff 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -96,6 +96,44 @@ static inline int netlink_is_kernel(struct sock *sk)
>
>  static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
>
> +static struct lock_class_key nlk_cb_mutex_keys[MAX_LINKS];
> +
> +static const char *const nlk_cb_mutex_key_strings[MAX_LINKS + 1] = {
> +   "nlk_cb_mutex-ROUTE",
> +   "nlk_cb_mutex-1",
> +   "nlk_cb_mutex-USERSOCK",
> +   "nlk_cb_mutex-FIREWALL",
> +   "nlk_cb_mutex-SOCK_DIAG",
> +   "nlk_cb_mutex-NFLOG",
> +   "nlk_cb_mutex-XFRM",
> +   "nlk_cb_mutex-SELINUX",
> +   "nlk_cb_mutex-ISCSI",
> +   "nlk_cb_mutex-AUDIT",
> +   "nlk_cb_mutex-FIB_LOOKUP",
> +   "nlk_cb_mutex-CONNECTOR",
> +   "nlk_cb_mutex-NETFILTER",
> +   "nlk_cb_mutex-IP6_FW",
> +   "nlk_cb_mutex-DNRTMSG",
> +   "nlk_cb_mutex-KOBJECT_UEVENT",
> +   "nlk_cb_mutex-GENERIC",
> +   "nlk_cb_mutex-17",
> +   "nlk_cb_mutex-SCSITRANSPORT",
> +   "nlk_cb_mutex-ECRYPTFS",
> +   "nlk_cb_mutex-RDMA",
> +   "nlk_cb_mutex-CRYPTO",
> +   "nlk_cb_mutex-SMC",
> +   "nlk_cb_mutex-23",
> +   "nlk_cb_mutex-24",
> +   "nlk_cb_mutex-25",
> +   "nlk_cb_mutex-26",
> +   "nlk_cb_mutex-27",
> +   "nlk_cb_mutex-28",
> +   "nlk_cb_mutex-29",
> +   "nlk_cb_mutex-30",
> +   "nlk_cb_mutex-31",
> +   "nlk_cb_mutex-MAX_LINKS"
> +};
> +
>  static int netlink_dump(struct sock *sk);
>  static void netlink_skb_destructor(struct sk_buff *skb);
>
> @@ -585,6 +623,9 @@ static int __netlink_create(struct net *net, struct 
> socket *sock,
> } else {
> nlk->cb_mutex = >cb_def_mutex;
> mutex_init(nlk->cb_mutex);
> +   lockdep_set_class_and_name(nlk->cb_mutex,
> +  nlk_cb_mutex_keys + protocol,
> +  
> nlk_cb_mutex_key_strings[protocol]);
> }
> init_waitqueue_head(>wait);
>
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: crypto: deadlock between crypto_alg_sem/rtnl_mutex/genl_mutex

[Herbert Xu]

On Tue, Mar 14, 2017 at 10:44:10AM +0100, Dmitry Vyukov wrote:
>
> Yes, please.
> Disregarding some reports is not a good way long term.

Please try this patch.

---8<---
Subject: netlink: Annotate nlk cb_mutex by protocol

Currently all occurences of nlk->cb_mutex are annotated by lockdep
as a single class.  This causes a false lcokdep cycle involving
genl and crypto_user.

This patch fixes it by dividing cb_mutex into individual classes
based on the netlink protocol.  As genl and crypto_user do not
use the same netlink protocol this breaks the false dependency
loop.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7b73c7c..596eaff 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -96,6 +96,44 @@ static inline int netlink_is_kernel(struct sock *sk)
 
 static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
 
+static struct lock_class_key nlk_cb_mutex_keys[MAX_LINKS];
+
+static const char *const nlk_cb_mutex_key_strings[MAX_LINKS + 1] = {
+   "nlk_cb_mutex-ROUTE",
+   "nlk_cb_mutex-1",
+   "nlk_cb_mutex-USERSOCK",
+   "nlk_cb_mutex-FIREWALL",
+   "nlk_cb_mutex-SOCK_DIAG",
+   "nlk_cb_mutex-NFLOG",
+   "nlk_cb_mutex-XFRM",
+   "nlk_cb_mutex-SELINUX",
+   "nlk_cb_mutex-ISCSI",
+   "nlk_cb_mutex-AUDIT",
+   "nlk_cb_mutex-FIB_LOOKUP",
+   "nlk_cb_mutex-CONNECTOR",
+   "nlk_cb_mutex-NETFILTER",
+   "nlk_cb_mutex-IP6_FW",
+   "nlk_cb_mutex-DNRTMSG",
+   "nlk_cb_mutex-KOBJECT_UEVENT",
+   "nlk_cb_mutex-GENERIC",
+   "nlk_cb_mutex-17",
+   "nlk_cb_mutex-SCSITRANSPORT",
+   "nlk_cb_mutex-ECRYPTFS",
+   "nlk_cb_mutex-RDMA",
+   "nlk_cb_mutex-CRYPTO",
+   "nlk_cb_mutex-SMC",
+   "nlk_cb_mutex-23",
+   "nlk_cb_mutex-24",
+   "nlk_cb_mutex-25",
+   "nlk_cb_mutex-26",
+   "nlk_cb_mutex-27",
+   "nlk_cb_mutex-28",
+   "nlk_cb_mutex-29",
+   "nlk_cb_mutex-30",
+   "nlk_cb_mutex-31",
+   "nlk_cb_mutex-MAX_LINKS"
+};
+
 static int netlink_dump(struct sock *sk);
 static void netlink_skb_destructor(struct sk_buff *skb);
 
@@ -585,6 +623,9 @@ static int __netlink_create(struct net *net, struct socket 
*sock,
} else {
nlk->cb_mutex = >cb_def_mutex;
mutex_init(nlk->cb_mutex);
+   lockdep_set_class_and_name(nlk->cb_mutex,
+  nlk_cb_mutex_keys + protocol,
+  nlk_cb_mutex_key_strings[protocol]);
}
init_waitqueue_head(>wait);
 
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: crypto: deadlock between crypto_alg_sem/rtnl_mutex/genl_mutex

[Dmitry Vyukov]

On Tue, Mar 14, 2017 at 10:16 AM, Herbert Xu
 wrote:
> On Sun, Mar 05, 2017 at 04:08:39PM +0100, Dmitry Vyukov wrote:
>>
>> -> #1 (genl_mutex){+.+.+.}:
>>validate_chain kernel/locking/lockdep.c:2267 [inline]
>>__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
>>lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
>>__mutex_lock_common kernel/locking/mutex.c:756 [inline]
>>__mutex_lock+0x172/0x1730 kernel/locking/mutex.c:893
>>mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
>>genl_lock net/netlink/genetlink.c:32 [inline]
>>genl_lock_dumpit+0x41/0x90 net/netlink/genetlink.c:478
>>netlink_dump+0x54d/0xd40 net/netlink/af_netlink.c:2127
>>__netlink_dump_start+0x4e5/0x760 net/netlink/af_netlink.c:2217
>>genl_family_rcv_msg+0xd9d/0x1040 net/netlink/genetlink.c:546
>>genl_rcv_msg+0xa6/0x140 net/netlink/genetlink.c:620
>>netlink_rcv_skb+0x2ab/0x390 net/netlink/af_netlink.c:2298
>>genl_rcv+0x28/0x40 net/netlink/genetlink.c:631
>>netlink_unicast_kernel net/netlink/af_netlink.c:1231 [inline]
>>netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1257
>>netlink_sendmsg+0xa9f/0xe50 net/netlink/af_netlink.c:1803
>>sock_sendmsg_nosec net/socket.c:633 [inline]
>>sock_sendmsg+0xca/0x110 net/socket.c:643
>>sock_write_iter+0x326/0x600 net/socket.c:846
>>call_write_iter include/linux/fs.h:1733 [inline]
>>new_sync_write fs/read_write.c:497 [inline]
>>__vfs_write+0x483/0x740 fs/read_write.c:510
>>vfs_write+0x187/0x530 fs/read_write.c:558
>>SYSC_write fs/read_write.c:605 [inline]
>>SyS_write+0xfb/0x230 fs/read_write.c:597
>>entry_SYSCALL_64_fastpath+0x1f/0xc2
>>
>> -> #0 (nlk->cb_mutex){+.+.+.}:
>>check_prev_add kernel/locking/lockdep.c:1830 [inline]
>>check_prevs_add+0xa8f/0x19f0 kernel/locking/lockdep.c:1940
>>validate_chain kernel/locking/lockdep.c:2267 [inline]
>>__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
>>lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
>>__mutex_lock_common kernel/locking/mutex.c:756 [inline]
>>__mutex_lock+0x172/0x1730 kernel/locking/mutex.c:893
>>mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
>>__netlink_dump_start+0xf4/0x760 net/netlink/af_netlink.c:2187
>>netlink_dump_start include/linux/netlink.h:165 [inline]
>>crypto_user_rcv_msg+0x2ad/0x4f0 crypto/crypto_user.c:517
>>netlink_rcv_skb+0x2ab/0x390 net/netlink/af_netlink.c:2298
>>crypto_netlink_rcv+0x2a/0x40 crypto/crypto_user.c:538
>>netlink_unicast_kernel net/netlink/af_netlink.c:1231 [inline]
>>netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1257
>>netlink_sendmsg+0xa9f/0xe50 net/netlink/af_netlink.c:1803
>>sock_sendmsg_nosec net/socket.c:633 [inline]
>>sock_sendmsg+0xca/0x110 net/socket.c:643
>>___sys_sendmsg+0x8fa/0x9f0 net/socket.c:1985
>>__sys_sendmsg+0x138/0x300 net/socket.c:2019
>>SYSC_sendmsg net/socket.c:2030 [inline]
>>SyS_sendmsg+0x2d/0x50 net/socket.c:2026
>>entry_SYSCALL_64_fastpath+0x1f/0xc2
>
> This looks like a false positive.  The cb_mutex in #1 is not the
> same as the cb_mutex in #0.  The cb_mutex in #0 comes is obtained
> by crypto_user which uses straight netlink.  The cb_mutex in #1
> is a genl netlink socket.
>
> I'll have a look to see if we can annotate this.

Yes, please.
Disregarding some reports is not a good way long term.

Re: XTS Crypto Not Found In /proc/crypto Even After Compiled for 4.10.1.

[Herbert Xu]

On Mon, Mar 13, 2017 at 07:06:01PM +0200, Krzysztof Kozlowski wrote:
> 
> I bisected this to commit f1c131b45410 ("crypto: xts - Convert to
> skcipher"). The s5p-sss driver stays the same... but the xts changes and
> as a result we have a NULL pointer dereference (actually of value
> 0004):
> [   18.930195] Unable to handle kernel NULL pointer dereference at virtual 
> address 0004
> ...
> [   18.972325] [] (post_crypt) from [] 
> (decrypt_done+0x4c/0x54)
> [   18.972343] [] (decrypt_done) from [] 
> (s5p_aes_interrupt+0x1bc/0x208)
> [   18.972360] [] (s5p_aes_interrupt) from [] 
> (irq_thread_fn+0x1c/0x54)

Well that's hardly surprising since xts prior to this commit used
simple AES as opposed to ecb(aes) so it couldn't have triggered
this bug.

> Any hints?

I'll have a look when I have time but no promises.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: crypto: deadlock between crypto_alg_sem/rtnl_mutex/genl_mutex

[Herbert Xu]

On Sun, Mar 05, 2017 at 06:36:12PM +0100, Dmitry Vyukov wrote:
>
> > -> #1 (genl_mutex){+.+.+.}:
> >validate_chain kernel/locking/lockdep.c:2267 [inline]
> >__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
> >lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
> >__mutex_lock_common kernel/locking/mutex.c:756 [inline]
> >__mutex_lock+0x172/0x1730 kernel/locking/mutex.c:893
> >mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
> >genl_lock net/netlink/genetlink.c:32 [inline]
> >genl_lock_dumpit+0x41/0x90 net/netlink/genetlink.c:478
> >netlink_dump+0x54d/0xd40 net/netlink/af_netlink.c:2127
> >__netlink_dump_start+0x4e5/0x760 net/netlink/af_netlink.c:2217
> >genl_family_rcv_msg+0xd9d/0x1040 net/netlink/genetlink.c:546
> >genl_rcv_msg+0xa6/0x140 net/netlink/genetlink.c:620
> >netlink_rcv_skb+0x2ab/0x390 net/netlink/af_netlink.c:2298
> >genl_rcv+0x28/0x40 net/netlink/genetlink.c:631
> >netlink_unicast_kernel net/netlink/af_netlink.c:1231 [inline]
> >netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1257
> >netlink_sendmsg+0xa9f/0xe50 net/netlink/af_netlink.c:1803
> >sock_sendmsg_nosec net/socket.c:633 [inline]
> >sock_sendmsg+0xca/0x110 net/socket.c:643
> >sock_write_iter+0x326/0x600 net/socket.c:846
> >call_write_iter include/linux/fs.h:1733 [inline]
> >new_sync_write fs/read_write.c:497 [inline]
> >__vfs_write+0x483/0x740 fs/read_write.c:510
> >vfs_write+0x187/0x530 fs/read_write.c:558
> >SYSC_write fs/read_write.c:605 [inline]
> >SyS_write+0xfb/0x230 fs/read_write.c:597
> >entry_SYSCALL_64_fastpath+0x1f/0xc2
> >
> > -> #0 (nlk->cb_mutex){+.+.+.}:
> >check_prev_add kernel/locking/lockdep.c:1830 [inline]
> >check_prevs_add+0xa8f/0x19f0 kernel/locking/lockdep.c:1940
> >validate_chain kernel/locking/lockdep.c:2267 [inline]
> >__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
> >lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
> >__mutex_lock_common kernel/locking/mutex.c:756 [inline]
> >__mutex_lock+0x172/0x1730 kernel/locking/mutex.c:893
> >mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
> >__netlink_dump_start+0xf4/0x760 net/netlink/af_netlink.c:2187
> >netlink_dump_start include/linux/netlink.h:165 [inline]
> >crypto_user_rcv_msg+0x2ad/0x4f0 crypto/crypto_user.c:517
> >netlink_rcv_skb+0x2ab/0x390 net/netlink/af_netlink.c:2298
> >crypto_netlink_rcv+0x2a/0x40 crypto/crypto_user.c:538
> >netlink_unicast_kernel net/netlink/af_netlink.c:1231 [inline]
> >netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1257
> >netlink_sendmsg+0xa9f/0xe50 net/netlink/af_netlink.c:1803
> >sock_sendmsg_nosec net/socket.c:633 [inline]
> >sock_sendmsg+0xca/0x110 net/socket.c:643
> >___sys_sendmsg+0x8fa/0x9f0 net/socket.c:1985
> >__sys_sendmsg+0x138/0x300 net/socket.c:2019
> >SYSC_sendmsg net/socket.c:2030 [inline]
> >SyS_sendmsg+0x2d/0x50 net/socket.c:2026
> >entry_SYSCALL_64_fastpath+0x1f/0xc2

Ditto.  Please disregard any reports involving genl_mutex and
cb_mutex where the latter comes from crypto_user.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: crypto: deadlock between crypto_alg_sem/rtnl_mutex/genl_mutex

[Herbert Xu]

On Sun, Mar 05, 2017 at 04:08:39PM +0100, Dmitry Vyukov wrote:
>
> -> #1 (genl_mutex){+.+.+.}:
>validate_chain kernel/locking/lockdep.c:2267 [inline]
>__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
>lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
>__mutex_lock_common kernel/locking/mutex.c:756 [inline]
>__mutex_lock+0x172/0x1730 kernel/locking/mutex.c:893
>mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
>genl_lock net/netlink/genetlink.c:32 [inline]
>genl_lock_dumpit+0x41/0x90 net/netlink/genetlink.c:478
>netlink_dump+0x54d/0xd40 net/netlink/af_netlink.c:2127
>__netlink_dump_start+0x4e5/0x760 net/netlink/af_netlink.c:2217
>genl_family_rcv_msg+0xd9d/0x1040 net/netlink/genetlink.c:546
>genl_rcv_msg+0xa6/0x140 net/netlink/genetlink.c:620
>netlink_rcv_skb+0x2ab/0x390 net/netlink/af_netlink.c:2298
>genl_rcv+0x28/0x40 net/netlink/genetlink.c:631
>netlink_unicast_kernel net/netlink/af_netlink.c:1231 [inline]
>netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1257
>netlink_sendmsg+0xa9f/0xe50 net/netlink/af_netlink.c:1803
>sock_sendmsg_nosec net/socket.c:633 [inline]
>sock_sendmsg+0xca/0x110 net/socket.c:643
>sock_write_iter+0x326/0x600 net/socket.c:846
>call_write_iter include/linux/fs.h:1733 [inline]
>new_sync_write fs/read_write.c:497 [inline]
>__vfs_write+0x483/0x740 fs/read_write.c:510
>vfs_write+0x187/0x530 fs/read_write.c:558
>SYSC_write fs/read_write.c:605 [inline]
>SyS_write+0xfb/0x230 fs/read_write.c:597
>entry_SYSCALL_64_fastpath+0x1f/0xc2
> 
> -> #0 (nlk->cb_mutex){+.+.+.}:
>check_prev_add kernel/locking/lockdep.c:1830 [inline]
>check_prevs_add+0xa8f/0x19f0 kernel/locking/lockdep.c:1940
>validate_chain kernel/locking/lockdep.c:2267 [inline]
>__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
>lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
>__mutex_lock_common kernel/locking/mutex.c:756 [inline]
>__mutex_lock+0x172/0x1730 kernel/locking/mutex.c:893
>mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
>__netlink_dump_start+0xf4/0x760 net/netlink/af_netlink.c:2187
>netlink_dump_start include/linux/netlink.h:165 [inline]
>crypto_user_rcv_msg+0x2ad/0x4f0 crypto/crypto_user.c:517
>netlink_rcv_skb+0x2ab/0x390 net/netlink/af_netlink.c:2298
>crypto_netlink_rcv+0x2a/0x40 crypto/crypto_user.c:538
>netlink_unicast_kernel net/netlink/af_netlink.c:1231 [inline]
>netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1257
>netlink_sendmsg+0xa9f/0xe50 net/netlink/af_netlink.c:1803
>sock_sendmsg_nosec net/socket.c:633 [inline]
>sock_sendmsg+0xca/0x110 net/socket.c:643
>___sys_sendmsg+0x8fa/0x9f0 net/socket.c:1985
>__sys_sendmsg+0x138/0x300 net/socket.c:2019
>SYSC_sendmsg net/socket.c:2030 [inline]
>SyS_sendmsg+0x2d/0x50 net/socket.c:2026
>entry_SYSCALL_64_fastpath+0x1f/0xc2

This looks like a false positive.  The cb_mutex in #1 is not the
same as the cb_mutex in #0.  The cb_mutex in #0 comes is obtained
by crypto_user which uses straight netlink.  The cb_mutex in #1
is a genl netlink socket.

I'll have a look to see if we can annotate this.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: crypto: deadlock between crypto_alg_sem/rtnl_mutex/genl_mutex

[Dmitry Vyukov]

On Mon, Mar 6, 2017 at 10:36 AM, Dmitry Vyukov  wrote:
> On Sun, Mar 5, 2017 at 6:36 PM, Dmitry Vyukov  wrote:
>> On Sun, Mar 5, 2017 at 4:08 PM, Dmitry Vyukov  wrote:
>>> Hello,
>>>
>>> I am getting the following deadlock reports while running syzkaller
>>> fuzzer on net-next/8d70eeb84ab277377c017af6a21d0a337025dede:
>>>
>>> ==
>>> [ INFO: possible circular locking dependency detected ]
>>> 4.10.0+ #5 Not tainted
>>> ---
>>> syz-executor6/6143 is trying to acquire lock:
>>>  (nlk->cb_mutex){+.+.+.}, at: []
>>> __netlink_dump_start+0xf4/0x760 net/netlink/af_netlink.c:2187
>>>
>>> but task is already holding lock:
>>>  (crypto_alg_sem){+.}, at: []
>>> crypto_user_rcv_msg+0x136/0x4f0 crypto/crypto_user.c:507
>>>
>>> which lock already depends on the new lock.
>>>
>>>
>>> the existing dependency chain (in reverse order) is:
>>>
>>> -> #4 (crypto_alg_sem){+.}:
>>>validate_chain kernel/locking/lockdep.c:2267 [inline]
>>>__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
>>>lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
>>>down_read+0x9b/0x150 kernel/locking/rwsem.c:23
>>>crypto_alg_lookup+0x23/0x50 crypto/api.c:199
>>>crypto_larval_lookup.part.10+0x9a/0x3b0 crypto/api.c:217
>>>crypto_larval_lookup crypto/api.c:211 [inline]
>>>crypto_alg_mod_lookup+0x77/0x1b0 crypto/api.c:270
>>>crypto_alloc_base+0x50/0x1e0 crypto/api.c:416
>>>crypto_alloc_cipher include/linux/crypto.h:1407 [inline]
>>>tcp_fastopen_reset_cipher+0xc2/0x2e0 net/ipv4/tcp_fastopen.c:48
>>>tcp_fastopen_init_key_once+0x114/0x120 net/ipv4/tcp_fastopen.c:29
>>>do_tcp_setsockopt.isra.36+0x140a/0x20a0 net/ipv4/tcp.c:2684
>>>tcp_setsockopt+0xb0/0xd0 net/ipv4/tcp.c:2733
>>>sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2731
>>>SYSC_setsockopt net/socket.c:1786 [inline]
>>>SyS_setsockopt+0x25c/0x390 net/socket.c:1765
>>>entry_SYSCALL_64_fastpath+0x1f/0xc2
>>>
>>> -> #3 (sk_lock-AF_INET){+.+.+.}:
>>>validate_chain kernel/locking/lockdep.c:2267 [inline]
>>>__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
>>>lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
>>>lock_sock_nested+0xcb/0x120 net/core/sock.c:2536
>>>lock_sock include/net/sock.h:1460 [inline]
>>>rds_tcp_listen_stop+0x57/0x140 net/rds/tcp_listen.c:284
>>>rds_tcp_kill_sock net/rds/tcp.c:529 [inline]
>>>rds_tcp_dev_event+0x383/0xc50 net/rds/tcp.c:568
>>>notifier_call_chain+0x1b5/0x2b0 kernel/notifier.c:93
>>>__raw_notifier_call_chain kernel/notifier.c:394 [inline]
>>>raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
>>>call_netdevice_notifiers_info+0x51/0x90 net/core/dev.c:1646
>>>call_netdevice_notifiers net/core/dev.c:1662 [inline]
>>>netdev_run_todo+0x3b2/0xa30 net/core/dev.c:7530
>>>rtnl_unlock+0xe/0x10 net/core/rtnetlink.c:104
>>>default_device_exit_batch+0x504/0x620 net/core/dev.c:8334
>>>ops_exit_list.isra.6+0x100/0x150 net/core/net_namespace.c:144
>>>cleanup_net+0x551/0xa90 net/core/net_namespace.c:463
>>>process_one_work+0xbd0/0x1c10 kernel/workqueue.c:2096
>>>worker_thread+0x223/0x1990 kernel/workqueue.c:2230
>>>kthread+0x326/0x3f0 kernel/kthread.c:229
>>>ret_from_fork+0x31/0x40 arch/x86/entry/entry_64.S:430
>>>
>>> -> #2 (rtnl_mutex){+.+.+.}:
>>>validate_chain kernel/locking/lockdep.c:2267 [inline]
>>>__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3340
>>>lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3755
>>>__mutex_lock_common kernel/locking/mutex.c:756 [inline]
>>>__mutex_lock+0x172/0x1730 kernel/locking/mutex.c:893
>>>mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
>>>rtnl_lock+0x17/0x20 net/core/rtnetlink.c:70
>>>tipc_nl_bearer_dump+0x3ef/0x720 net/tipc/bearer.c:774
>>>genl_lock_dumpit+0x68/0x90 net/netlink/genetlink.c:479
>>>netlink_dump+0x54d/0xd40 net/netlink/af_netlink.c:2127
>>>__netlink_dump_start+0x4e5/0x760 net/netlink/af_netlink.c:2217
>>>genl_family_rcv_msg+0xd9d/0x1040 net/netlink/genetlink.c:546
>>>genl_rcv_msg+0xa6/0x140 net/netlink/genetlink.c:620
>>>netlink_rcv_skb+0x2ab/0x390 net/netlink/af_netlink.c:2298
>>>genl_rcv+0x28/0x40 net/netlink/genetlink.c:631
>>>netlink_unicast_kernel net/netlink/af_netlink.c:1231 [inline]
>>>netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1257
>>>netlink_sendmsg+0xa9f/0xe50 net/netlink/af_netlink.c:1803
>>>sock_sendmsg_nosec net/socket.c:633 [inline]
>>>sock_sendmsg+0xca/0x110 net/socket.c:643
>>>sock_write_iter+0x326/0x600 

Re: [PATCH] crypto: ccp - Assign DMA commands to the channel's CCP

[Stephan Müller]

Am Montag, 13. März 2017, 20:35:07 CET schrieb Gary R Hook:

Hi Gary,

> On 03/03/2017 7:15 AM, Stephan Mueller wrote:
> > Am Donnerstag, 2. März 2017, 22:26:54 CET schrieb Gary R Hook:
> > 
> > Hi Gary,
> 
> Thanks for your comments, Stephan.
> 
> > > A version 5 device provides the primitive commands
> > > required for AES GCM. This patch adds support for
> > > en/decryption.
> > > 
> > > Signed-off-by: Gary R Hook 
> > > ---
> > > 
> > >  drivers/crypto/ccp/Makefile|1
> > >  drivers/crypto/ccp/ccp-crypto-aes-galois.c |  257
> > > 
> > >  drivers/crypto/ccp/ccp-crypto-main.c  
> > > |
> > > 12 +
> > > 
> > >  drivers/crypto/ccp/ccp-crypto.h|   14 ++
> > >  drivers/crypto/ccp/ccp-dev-v5.c|2
> > >  drivers/crypto/ccp/ccp-dev.h   |1
> > >  drivers/crypto/ccp/ccp-ops.c   |  252
> > > 
> > > +++ include/linux/ccp.h|
> > > 9 +
> > > 
> > >  8 files changed, 548 insertions(+)
> > >  create mode 100644 drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > > 
> > > diff --git a/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > > b/drivers/crypto/ccp/ccp-crypto-aes-galois.c new file mode 100644
> > > index 000..8bc18c9
> > > --- /dev/null
> > > +++ b/drivers/crypto/ccp/ccp-crypto-aes-galois.c
> > > @@ -0,0 +1,257 @@
> > > +/*
> > > + * AMD Cryptographic Coprocessor (CCP) AES GCM crypto API support
> > > + *
> > > + * Copyright (C) 2016 Advanced Micro Devices, Inc.
> > > + *
> > > + * Author: Gary R Hook 
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include "ccp-crypto.h"
> > > +
> > > +#defineAES_GCM_IVSIZE  12
> > > +
> > > +static int ccp_aes_gcm_complete(struct crypto_async_request *async_req,
> > > int ret) +{
> > > +   return ret;
> > > +}
> > > +
> > > +static int ccp_aes_gcm_setkey(struct crypto_aead *tfm, const u8 *key,
> > > + unsigned int key_len)
> > > +{
> > > +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> > > +
> > > +   switch (key_len) {
> > > +   case AES_KEYSIZE_128:
> > > +   ctx->u.aes.type = CCP_AES_TYPE_128;
> > > +   break;
> > > +   case AES_KEYSIZE_192:
> > > +   ctx->u.aes.type = CCP_AES_TYPE_192;
> > > +   break;
> > > +   case AES_KEYSIZE_256:
> > > +   ctx->u.aes.type = CCP_AES_TYPE_256;
> > > +   break;
> > > +   default:
> > > +   crypto_aead_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> > > +   return -EINVAL;
> > > +   }
> > > +
> > > +   ctx->u.aes.mode = CCP_AES_MODE_GCM;
> > > +   ctx->u.aes.key_len = key_len;
> > > +
> > > +   memcpy(ctx->u.aes.key, key, key_len);
> > > +   sg_init_one(>u.aes.key_sg, ctx->u.aes.key, key_len);
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +static int ccp_aes_gcm_setauthsize(struct crypto_aead *tfm,
> > > +  unsigned int authsize)
> > > +{
> > > +   return 0;
> > > +}
> > > +
> > > +static int ccp_aes_gcm_crypt(struct aead_request *req, bool encrypt)
> > > +{
> > > +   struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> > > +   struct ccp_ctx *ctx = crypto_aead_ctx(tfm);
> > > +   struct ccp_aes_req_ctx *rctx = aead_request_ctx(req);
> > > +   struct scatterlist *iv_sg = NULL;
> > > +   unsigned int iv_len = 0;
> > > +   int i;
> > > +   int ret = 0;
> > > +
> > > +   if (!ctx->u.aes.key_len)
> > > +   return -EINVAL;
> > > +
> > > +   if (ctx->u.aes.mode != CCP_AES_MODE_GCM)
> > > +   return -EINVAL;
> > > +
> > > +   if (!req->iv)
> > > +   return -EINVAL;
> > > +
> > > +   /*
> > > +* 5 parts:
> > > +*   plaintext/ciphertext input
> > > +*   AAD
> > > +*   key
> > > +*   IV
> > > +*   Destination+tag buffer
> > > +*/
> > > +
> > > +   /* According to the way AES GCM has been implemented here,
> > > +* per RFC 4106 it seems, the provided IV is fixed at 12 bytes,
> > 
> > When you have that restriction, should the cipher be called
> > rfc4106(gcm(aes))?
> > 
> > But then, the key is 4 bytes longer than a normal AES key as it contains
> > the leading 32 bits of the IV.
> 
> I had my wires crossed due to an incomplete understanding of an AEAD cipher
> in general, and GCM in particular. I'm hopeful that someone can help me
> understand:
> 
> For the AES GCM encryption tests in testmgr.h, where there is an IV,
> they're all
> 12 bytes in length. As I understand AES GCM the IV can be anywhere from
> 1 to 2^64
> bits in length; the value of 96 makes for convenience and efficiency.
> But it's
>