Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH] hwrng: core - Allocate memory during module init
From: PrasannaKumar Muralidharan @ 2016-09-04  8:29 UTC (permalink / raw)
  To: mpm, herbert, jslaby, peter, lee.jones, linux-crypto; +Cc: prasannatsmkumar

In core rng_buffer and rng_fillbuf is allocated in hwrng_register only
once and it is freed during module exit. This patch moves allocating
rng_buffer and rng_fillbuf from hwrng_register to rng core's init. This
avoids checking whether rng_buffer and rng_fillbuf was allocated from
every hwrng_register call. Also moving them to module init makes it
explicit that it is freed in module exit.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 drivers/char/hw_random/core.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 9203f2d..ec6846b 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -449,22 +449,6 @@ int hwrng_register(struct hwrng *rng)
 		goto out;
 
 	mutex_lock(&rng_mutex);
-
-	/* kmalloc makes this safe for virt_to_page() in virtio_rng.c */
-	err = -ENOMEM;
-	if (!rng_buffer) {
-		rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL);
-		if (!rng_buffer)
-			goto out_unlock;
-	}
-	if (!rng_fillbuf) {
-		rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL);
-		if (!rng_fillbuf) {
-			kfree(rng_buffer);
-			goto out_unlock;
-		}
-	}
-
 	/* Must not register two RNGs with the same name. */
 	err = -EEXIST;
 	list_for_each_entry(tmp, &rng_list, list) {
@@ -573,6 +557,17 @@ EXPORT_SYMBOL_GPL(devm_hwrng_unregister);
 
 static int __init hwrng_modinit(void)
 {
+	/* kmalloc makes this safe for virt_to_page() in virtio_rng.c */
+	rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL);
+	if (!rng_buffer)
+		return -ENOMEM;
+
+	rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL);
+	if (!rng_fillbuf) {
+		kfree(rng_buffer);
+		return -ENOMEM;
+	}
+
 	return register_miscdev();
 }
 
-- 
2.5.0

^ permalink raw reply related

* Re: [cryptodev:master 65/80] drivers/pci/quirks.c:843:6: error: 'struct pci_dev' has no member named 'sriov'
From: Ananth Jasty @ 2016-09-04  1:40 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Ananth Jasty, kbuild-all, linux-crypto, Herbert Xu, Omer Khaliq
In-Reply-To: <201609040817.Rs5sdqia%fengguang.wu@intel.com>

As this fix is purely SRIOV related, an ifdef guard for SRIOV support should be sufficient to prevent this header dependency.

Ananth

On Sep 3, 2016, at 5:16 PM, kbuild test robot <fengguang.wu@intel.com> wrote:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
> head:   10faa8c0d6c3b22466f97713a9533824a2ea1c57
> commit: 21b5b8eebbae427d7d890b7dd1e43a63aca7c26c [65/80] PCI: quirk fixup for cavium invalid sriov link value.
> config: arm64-kexec_dev_defconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
> reproduce:
>        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>        chmod +x ~/bin/make.cross
>        git checkout 21b5b8eebbae427d7d890b7dd1e43a63aca7c26c
>        # save the attached .config to linux build tree
>        make.cross ARCH=arm64 
> 
> All errors (new ones prefixed by >>):
> 
>   drivers/pci/quirks.c: In function 'quirk_cavium_sriov_rnm_link':
>>> drivers/pci/quirks.c:843:6: error: 'struct pci_dev' has no member named 'sriov'
>      dev->sriov->link = dev->devfn;
>         ^
> 
> vim +843 drivers/pci/quirks.c
> 
>   837	#ifdef CONFIG_ARM64
>   838	
>   839	static void quirk_cavium_sriov_rnm_link(struct pci_dev *dev)
>   840	{
>   841		/* Fix for improper SRIOV configuration on Cavium cn88xx  RNM device */
>   842		if (dev->subsystem_device == 0xa118)
>> 843			dev->sriov->link = dev->devfn;
>   844	}
>   845	DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, 0xa018, quirk_cavium_sriov_rnm_link);
>   846	#endif
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> <.config.gz>

^ permalink raw reply

* [cryptodev:master 65/80] drivers/pci/quirks.c:843:6: error: 'struct pci_dev' has no member named 'sriov'
From: kbuild test robot @ 2016-09-04  0:16 UTC (permalink / raw)
  To: Ananth Jasty; +Cc: kbuild-all, linux-crypto, Herbert Xu, Omer Khaliq

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
head:   10faa8c0d6c3b22466f97713a9533824a2ea1c57
commit: 21b5b8eebbae427d7d890b7dd1e43a63aca7c26c [65/80] PCI: quirk fixup for cavium invalid sriov link value.
config: arm64-kexec_dev_defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 21b5b8eebbae427d7d890b7dd1e43a63aca7c26c
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/pci/quirks.c: In function 'quirk_cavium_sriov_rnm_link':
>> drivers/pci/quirks.c:843:6: error: 'struct pci_dev' has no member named 'sriov'
      dev->sriov->link = dev->devfn;
         ^

vim +843 drivers/pci/quirks.c

   837	#ifdef CONFIG_ARM64
   838	
   839	static void quirk_cavium_sriov_rnm_link(struct pci_dev *dev)
   840	{
   841		/* Fix for improper SRIOV configuration on Cavium cn88xx  RNM device */
   842		if (dev->subsystem_device == 0xa118)
 > 843			dev->sriov->link = dev->devfn;
   844	}
   845	DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, 0xa018, quirk_cavium_sriov_rnm_link);
   846	#endif

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 17985 bytes --]

^ permalink raw reply

* [PATCH] crypto: qce: Initialize core src clock @100Mhz
From: Iaroslav Gridin @ 2016-09-03 16:45 UTC (permalink / raw)
  To: herbert
  Cc: davem, linux-crypto, linux-kernel, andy.gross, david.brown,
	linux-arm-msm, linux-soc, Iaroslav Gridin

Without that, QCE performance is about 2x less.

Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
---
 drivers/crypto/qce/core.c | 18 +++++++++++++++++-
 drivers/crypto/qce/core.h |  2 +-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 0cde513..657354c 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -193,6 +193,10 @@ static int qce_crypto_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	qce->core_src = devm_clk_get(qce->dev, "core_src");
+	if (IS_ERR(qce->core_src))
+		return PTR_ERR(qce->core_src);
+
 	qce->core = devm_clk_get(qce->dev, "core");
 	if (IS_ERR(qce->core))
 		return PTR_ERR(qce->core);
@@ -205,10 +209,20 @@ static int qce_crypto_probe(struct platform_device *pdev)
 	if (IS_ERR(qce->bus))
 		return PTR_ERR(qce->bus);
 
-	ret = clk_prepare_enable(qce->core);
+	ret = clk_prepare_enable(qce->core_src);
 	if (ret)
 		return ret;
 
+	ret = clk_set_rate(qce->core_src, 100000000);
+	if (ret) {
+		dev_warn(qce->dev, "Unable to set QCE core src clk @100Mhz, performance might be degraded\n");
+		goto err_clks_core_src;
+	}
+
+	ret = clk_prepare_enable(qce->core);
+	if (ret)
+		goto err_clks_core_src;
+
 	ret = clk_prepare_enable(qce->iface);
 	if (ret)
 		goto err_clks_core;
@@ -247,6 +261,8 @@ err_clks_iface:
 	clk_disable_unprepare(qce->iface);
 err_clks_core:
 	clk_disable_unprepare(qce->core);
+err_clks_core_src:
+	clk_disable_unprepare(qce->core_src);
 	return ret;
 }
 
diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index 549965d..c5f8d08 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -42,7 +42,7 @@ struct qce_device {
 	int result;
 	void __iomem *base;
 	struct device *dev;
-	struct clk *core, *iface, *bus;
+	struct clk *core, *iface, *bus, *core_src;
 	struct qce_dma_data dma;
 	int burst_size;
 	unsigned int pipe_pair_id;
-- 
2.9.3

^ permalink raw reply related

* Re: Who will copy the AAD data to dest. buffer
From: Stephan Mueller @ 2016-09-03 13:19 UTC (permalink / raw)
  To: Harsh Jain; +Cc: Herbert Xu, linux-crypto
In-Reply-To: <CAFXBA=ndKKGhfQdWtRuAAQdeBCeoKyS6-obExPHFjBsrj4OjXw@mail.gmail.com>

Am Samstag, 3. September 2016, 10:45:08 CEST schrieb Harsh Jain:

Hi Harsh,

> Thanks Herbert for clarification. It means Libkcapi documentation
> needs update of chapter  "Aead Cipher API".

Fixed, I will release a new version shortly.

Thanks for the pointer.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] crypto: mv_cesa: remove NO_IRQ reference
From: Boris Brezillon @ 2016-09-03  6:54 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Herbert Xu, Romain Perier, Arnaud Ebalard, Thomas Petazzoni,
	David S. Miller, linux-crypto, linux-kernel
In-Reply-To: <20160902232648.2119621-1-arnd@arndb.de>

On Sat,  3 Sep 2016 01:26:40 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> Drivers should not use NO_IRQ, as we are trying to get rid of that.
> In this case, the call to irq_of_parse_and_map() is both wrong
> (as it returns '0' on failure, not NO_IRQ) and unnecessary
> (as platform_get_irq() does the same thing)
> 
> This removes the call to irq_of_parse_and_map() and checks for
> the error code correctly.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> ---
>  drivers/crypto/mv_cesa.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> It would be good if someone could test this on a machine that boots
> from DT to ensure the conversion was correct.
> 
> diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
> index e6b658faef63..104e9ce9400a 100644
> --- a/drivers/crypto/mv_cesa.c
> +++ b/drivers/crypto/mv_cesa.c
> @@ -1091,11 +1091,8 @@ static int mv_probe(struct platform_device *pdev)
>  
>  	cp->max_req_size = cp->sram_size - SRAM_CFG_SPACE;
>  
> -	if (pdev->dev.of_node)
> -		irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> -	else
> -		irq = platform_get_irq(pdev, 0);
> -	if (irq < 0 || irq == NO_IRQ) {
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
>  		ret = irq;
>  		goto err;
>  	}

^ permalink raw reply

* Re: [dm-devel] dm-crypt: flush crypt_queue on suspend? on REQ_FLUSH?
From: Eric Wheeler @ 2016-09-03  6:26 UTC (permalink / raw)
  To: dm-devel; +Cc: linux-block, linux-crypto
In-Reply-To: <alpine.LRH.2.11.1609021430390.5413@mail.ewheeler.net>

On Fri, 2 Sep 2016, Eric Wheeler wrote:
> We have a KVM => dm-crypt => dm-thin stack and a snapshot may have 
> partially completed queued IO out-of-order.  EXT4 is giving errors like 
> this after mounting a snapshot, but only on files recently modified near 
> the snapshot time.  This might imply out-of-order writes since, 
> presumably, the ext4 journal would handle deletions in journal order and 
> snapshots should be safe at any time:
> 
> 	kernel: EXT4-fs error (device dm-2): ext4_lookup:1441: inode #1196093: comm rsync: deleted inode referenced: 1188710 
> 
> Notably, the ext4 error did not present prior to the snapshot.  We have 
> rsync logs from hours before that didn't report this message, but all 
> later rsyncs do.
> 
> Perhaps related, or perhaps not, crypt_map() notes the following:
> 	1914          * If bio is REQ_FLUSH or REQ_DISCARD, just bypass crypt queues.
> 	1915          * - for REQ_FLUSH device-mapper core ensures that no IO is in-flight
> 	1916          * - for REQ_DISCARD caller must use flush if IO ordering matters
> 
> However, crypt_map returns DM_MAPIO_SUBMITTED after calling 
> kcryptd_queue_crypt(io) which processes asynchronously on 
> cc->crypto_queue.  In crypt_ctr(), alloc_workqueue sets max_active to 
> num_online_cpus() unless 'same_cpu_crypt' is set in the dm table (mine is 
> not), so if I understand correctly, completion could happen out of order 
> with CPUs >1.
> 
> Does the dm stack know that the bio (dm_crypt_io) hasn't been completed 
> [since] it was put on a work queue [and then a write_thread]?  If so, I 
> would like to understand that dm bio-tracking mechanism, so please point 
> me at documentation or code.
> 
> If not, then does crypt_map() need a workqueue_flush(cc->crypto_queue) on 
> REQ_FLUSH?  
>
> I'm new to the dm-crypt code, so please educate me if I'm missing 
> something here.

On second look, simply adding workqueue_flush isn't enough since I've not 
set 'submit_from_crypt_cpus'; it's the cc->write_thread's job to get the 
bio's submitted via kcryptd_io_write() after pulling them from a red-black 
tree that kcryptd_crypt_write_io_submit() inserted into.  Is the red-black 
tree guaranteed to pop off in order, or at least to flush in order when a 
REQ_FLUSH comes through?

If not, then I think this means I need to set both 'same_cpu_crypt' 
(DM_CRYPT_SAME_CPU) and 'submit_from_crypt_cpus' (DM_CRYPT_NO_OFFLOAD) to 
guarantee ordering by restricting the workqueue to serial work 
(DM_CRYPT_SAME_CPU) and force immediate generic_make_request inside 
kcryptd_crypt_write_io_submit (DM_CRYPT_NO_OFFLOAD) instead of queuing to 
the cc->write_thread.

Your thoughts?

-Eric

 
> Thank you for your help!
> 
> --
> Eric Wheeler
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

^ permalink raw reply

* Re: Who will copy the AAD data to dest. buffer
From: Harsh Jain @ 2016-09-03  5:15 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, Stephan Mueller
In-Reply-To: <20160902144203.GA13135@gondor.apana.org.au>

Thanks Herbert for clarification. It means Libkcapi documentation
needs update of chapter  "Aead Cipher API".

Regards
Harsh Jain



On Fri, Sep 2, 2016 at 8:12 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Sep 02, 2016 at 08:05:04PM +0530, Harsh Jain wrote:
>> Hi Herbert,
>>
>> Is copy of AAD data to destination buffer when dst != src is mandatory
>> requirements for crypto drivers or we can skip this copy. Actually I
>> am bit confused, In following link Stephen had mentioned caller will
>> memcpy the AAD to destination buffer but authenc.c also copies the AAD
>> to dest. buffer.
>>
>> http://www.chronox.de/libkcapi/html/ch02s02.html
>
> It has to be copied if src != dst.
>
>> Secondly When AAD data remains unchanged in AEAD encryption/decryption
>> operations. Why we copy the same data to destination buffer?
>
> This greatly simplifies the implementation of the AEAD algorithms
> because we can throw away src and use the dst only.  For example,
> authenc hashes the AAD and ciphertext.  If we didn't force the
> copy it would have to hash them separately, meaning the use of
> the slow init/update/final interface.  With the copy it can use
> the digest interface.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH] crypto: caam: add missing header dependencies
From: Baoyou Xie @ 2016-09-03  3:29 UTC (permalink / raw)
  To: herbert, davem, horia.geanta, srinivas.kandagatla,
	alexandru.porosanu, arnd
  Cc: linux-crypto, linux-kernel, baoyou.xie, xie.baoyou

We get 1 warning when building kernel with W=1:
drivers/crypto/caam/ctrl.c:398:5: warning: no previous prototype for 'caam_get_era' [-Wmissing-prototypes]

In fact, this function is declared in drivers/crypto/caam/ctrl.h
and be exported, thus can be refrenced by modules, so this patch 
adds missing header dependencies.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/crypto/caam/ctrl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 0ec112e..46e72ed 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -14,6 +14,7 @@
 #include "jr.h"
 #include "desc_constr.h"
 #include "error.h"
+#include "ctrl.h"
 
 bool caam_little_end;
 EXPORT_SYMBOL(caam_little_end);
-- 
2.7.4

^ permalink raw reply related

* [PATCH] crypto: mv_cesa: remove NO_IRQ reference
From: Arnd Bergmann @ 2016-09-02 23:26 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Romain Perier, Boris Brezillon, Arnaud Ebalard, Thomas Petazzoni,
	Arnd Bergmann, David S. Miller, linux-crypto, linux-kernel

Drivers should not use NO_IRQ, as we are trying to get rid of that.
In this case, the call to irq_of_parse_and_map() is both wrong
(as it returns '0' on failure, not NO_IRQ) and unnecessary
(as platform_get_irq() does the same thing)

This removes the call to irq_of_parse_and_map() and checks for
the error code correctly.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/crypto/mv_cesa.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

It would be good if someone could test this on a machine that boots
from DT to ensure the conversion was correct.

diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index e6b658faef63..104e9ce9400a 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -1091,11 +1091,8 @@ static int mv_probe(struct platform_device *pdev)
 
 	cp->max_req_size = cp->sram_size - SRAM_CFG_SPACE;
 
-	if (pdev->dev.of_node)
-		irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
-	else
-		irq = platform_get_irq(pdev, 0);
-	if (irq < 0 || irq == NO_IRQ) {
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
 		ret = irq;
 		goto err;
 	}
-- 
2.9.0

^ permalink raw reply related

* dm-crypt: flush crypt_queue on suspend? on REQ_FLUSH?
From: Eric Wheeler @ 2016-09-02 22:13 UTC (permalink / raw)
  To: dm-devel; +Cc: linux-crypto, linux-block

Hello all,

We have a KVM => dm-crypt => dm-thin stack in and a snapshot may have 
partially completed queued IO out-of-order.  EXT4 is giving errors like 
this after mounting a snapshot, but only on files recently modified near 
the snapshot time.  This might imply out-of-order writes since, 
presumably, the ext4 journal would handle deletions in journal order and 
snapshots should be safe at any time:

	kernel: EXT4-fs error (device dm-2): ext4_lookup:1441: inode #1196093: comm rsync: deleted inode referenced: 1188710 

Notably, the ext4 error did not present prior to the snapshot.  We have 
rsync logs from hours before that didn't report this message, but all 
later rsyncs do.

Perhaps related, or perhaps not, crypt_map() notes the following:
	1914          * If bio is REQ_FLUSH or REQ_DISCARD, just bypass crypt queues.
	1915          * - for REQ_FLUSH device-mapper core ensures that no IO is in-flight
	1916          * - for REQ_DISCARD caller must use flush if IO ordering matters

However, crypt_map returns DM_MAPIO_SUBMITTED after calling 
kcryptd_queue_crypt(io) which processes asynchronously on 
cc->crypto_queue.  In crypt_ctr(), alloc_workqueue sets max_active to 
num_online_cpus() unless 'same_cpu_crypt' is set in the dm table (mine is 
not), so if I understand correctly, completion could happen out of order 
with CPUs >1.

Does the dm stack know that the bio (dm_crypt_io) hasn't been completed 
even though it was put on a work queue?  If so, I would like to understand 
that dm bio-tracking mechanism, so please point me at documentation or 
code.

If not, then does crypt_map() need a workqueue_flush(cc->crypto_queue) on 
REQ_FLUSH?  

I'm new to the dm-crypt code, so please educate me if I'm missing 
something here.

Thank you for your help!

--
Eric Wheeler

^ permalink raw reply

* [PATCH v2 2/2] crypto: qat - fix resource release omissions
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel
  Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902144753.31334-1-lambert.quentin@gmail.com>

In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.

This issue was found with Hector.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/crypto/qat/qat_common/qat_uclo.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
 			(PID_MINOR_REV & handle->hal_handle->revision_id);
 	if (qat_uclo_check_uof_compat(obj_handle)) {
 		pr_err("QAT: UOF incompatible\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_err;
 	}
 	obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
 	if (!obj_handle->obj_hdr->file_buff ||

^ permalink raw reply

* [PATCH v2 1/2] crypto: qat - introduces a variable to handle error codes
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel
  Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902144753.31334-1-lambert.quentin@gmail.com>

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
 	struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
 	unsigned int ae;
+	int ret;
 
 	obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
 					GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
 	    !qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
 				    &obj_handle->str_table)) {
 		pr_err("QAT: UOF doesn't have effective images\n");
+		ret = -EFAULT;
 		goto out_err;
 	}
 	obj_handle->uimage_num =
 		qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
 				    ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-	if (!obj_handle->uimage_num)
+	if (!obj_handle->uimage_num) {
+		ret = -EFAULT;
 		goto out_err;
+	}
 	if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
 		pr_err("QAT: Bad object\n");
+		ret = -EFAULT;
 		goto out_check_uof_aemask_err;
 	}
 	qat_uclo_init_uword_num(handle);
 	qat_uclo_map_initmem_table(&obj_handle->encap_uof_obj,
 				   &obj_handle->init_mem_tab);
-	if (qat_uclo_set_ae_mode(handle))
+	if (qat_uclo_set_ae_mode(handle)) {
+		ret = -EFAULT;
 		goto out_check_uof_aemask_err;
+	}
 	return 0;
 out_check_uof_aemask_err:
 	for (ae = 0; ae < obj_handle->uimage_num; ae++)
 		kfree(obj_handle->ae_uimage[ae].page);
 out_err:
 	kfree(obj_handle->uword_buf);
-	return -EFAULT;
+	return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

^ permalink raw reply

* [PATCH v2 0/2] add omitted release in qat_common
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel,
	Quentin Lambert
  Cc: kernel-janitors
In-Reply-To: <20160902144400.31216-1-lambert.quentin@gmail.com>

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

-changes since v1
I failed to send the first version properly and it was missing a proper commit
message, just ignore it.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
---

^ permalink raw reply

* [PATCH 1/2][RESEND] crypto: qat - introduces a variable to handle error codes
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel
  Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902144400.31216-1-lambert.quentin@gmail.com>

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
 	struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
 	unsigned int ae;
+	int ret;
 
 	obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
 					GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
 	    !qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
 				    &obj_handle->str_table)) {
 		pr_err("QAT: UOF doesn't have effective images\n");
+		ret = -EFAULT;
 		goto out_err;
 	}
 	obj_handle->uimage_num =
 		qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
 				    ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-	if (!obj_handle->uimage_num)
+	if (!obj_handle->uimage_num) {
+		ret = -EFAULT;
 		goto out_err;
+	}
 	if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
 		pr_err("QAT: Bad object\n");
+		ret = -EFAULT;
 		goto out_check_uof_aemask_err;
 	}
 	qat_uclo_init_uword_num(handle);
 	qat_uclo_map_initmem_table(&obj_handle->encap_uof_obj,
 				   &obj_handle->init_mem_tab);
-	if (qat_uclo_set_ae_mode(handle))
+	if (qat_uclo_set_ae_mode(handle)) {
+		ret = -EFAULT;
 		goto out_check_uof_aemask_err;
+	}
 	return 0;
 out_check_uof_aemask_err:
 	for (ae = 0; ae < obj_handle->uimage_num; ae++)
 		kfree(obj_handle->ae_uimage[ae].page);
 out_err:
 	kfree(obj_handle->uword_buf);
-	return -EFAULT;
+	return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

^ permalink raw reply

* [PATCH 2/2] crypto: qat - fix resource release omissions
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel
  Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902144400.31216-1-lambert.quentin@gmail.com>

In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.

This issue was found with Hector.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/crypto/qat/qat_common/qat_uclo.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
 			(PID_MINOR_REV & handle->hal_handle->revision_id);
 	if (qat_uclo_check_uof_compat(obj_handle)) {
 		pr_err("QAT: UOF incompatible\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_err;
 	}
 	obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
 	if (!obj_handle->obj_hdr->file_buff ||

^ permalink raw reply

* [PATCH 0/2][RESEND] add omitted release in qat_common
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel,
	Quentin Lambert
  Cc: kernel-janitors
In-Reply-To: <20160902143759.31125-1-lambert.quentin@gmail.com>

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
---

^ permalink raw reply

* Re: Who will copy the AAD data to dest. buffer
From: Herbert Xu @ 2016-09-02 14:42 UTC (permalink / raw)
  To: Harsh Jain; +Cc: linux-crypto, Stephan Mueller
In-Reply-To: <CAFXBA=m-Ued+9EGmfwa210cm6HZGxN3P-g_Fsapzcw+a3FKdZw@mail.gmail.com>

On Fri, Sep 02, 2016 at 08:05:04PM +0530, Harsh Jain wrote:
> Hi Herbert,
> 
> Is copy of AAD data to destination buffer when dst != src is mandatory
> requirements for crypto drivers or we can skip this copy. Actually I
> am bit confused, In following link Stephen had mentioned caller will
> memcpy the AAD to destination buffer but authenc.c also copies the AAD
> to dest. buffer.
> 
> http://www.chronox.de/libkcapi/html/ch02s02.html

It has to be copied if src != dst.

> Secondly When AAD data remains unchanged in AEAD encryption/decryption
> operations. Why we copy the same data to destination buffer?

This greatly simplifies the implementation of the AEAD algorithms
because we can throw away src and use the dst only.  For example,
authenc hashes the AAD and ciphertext.  If we didn't force the
copy it would have to hash them separately, meaning the use of
the slow init/update/final interface.  With the copy it can use
the digest interface.

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

^ permalink raw reply

* [PATCH 2/2] crypto: qat - fix resource release omissions
From: Quentin Lambert @ 2016-09-02 14:37 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel
  Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902143759.31125-1-lambert.quentin@gmail.com>

This issue was found with Hector.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/crypto/qat/qat_common/qat_uclo.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
 			(PID_MINOR_REV & handle->hal_handle->revision_id);
 	if (qat_uclo_check_uof_compat(obj_handle)) {
 		pr_err("QAT: UOF incompatible\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_err;
 	}
 	obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
 	if (!obj_handle->obj_hdr->file_buff ||

^ permalink raw reply

* [PATCH 1/2] crypto: qat - introduces a variable to handle error codes
From: Quentin Lambert @ 2016-09-02 14:37 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel
  Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902143759.31125-1-lambert.quentin@gmail.com>

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
 	struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
 	unsigned int ae;
+	int ret;
 
 	obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
 					GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
 	    !qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
 				    &obj_handle->str_table)) {
 		pr_err("QAT: UOF doesn't have effective images\n");
+		ret = -EFAULT;
 		goto out_err;
 	}
 	obj_handle->uimage_num =
 		qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
 				    ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-	if (!obj_handle->uimage_num)
+	if (!obj_handle->uimage_num) {
+		ret = -EFAULT;
 		goto out_err;
+	}
 	if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
 		pr_err("QAT: Bad object\n");
+		ret = -EFAULT;
 		goto out_check_uof_aemask_err;
 	}
 	qat_uclo_init_uword_num(handle);
 	qat_uclo_map_initmem_table(&obj_handle->encap_uof_obj,
 				   &obj_handle->init_mem_tab);
-	if (qat_uclo_set_ae_mode(handle))
+	if (qat_uclo_set_ae_mode(handle)) {
+		ret = -EFAULT;
 		goto out_check_uof_aemask_err;
+	}
 	return 0;
 out_check_uof_aemask_err:
 	for (ae = 0; ae < obj_handle->uimage_num; ae++)
 		kfree(obj_handle->ae_uimage[ae].page);
 out_err:
 	kfree(obj_handle->uword_buf);
-	return -EFAULT;
+	return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

^ permalink raw reply

* [PATCH 0/2] add omitted release in qat_common
From: Quentin Lambert @ 2016-09-02 14:37 UTC (permalink / raw)
  To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
	David S. Miller, qat-linux, linux-crypto, linux-kernel,
	Quentin Lambert
  Cc: kernel-janitors

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
---

^ permalink raw reply

* Who will copy the AAD data to dest. buffer
From: Harsh Jain @ 2016-09-02 14:35 UTC (permalink / raw)
  To: Herbert Xu, linux-crypto, Stephan Mueller

Hi Herbert,

Is copy of AAD data to destination buffer when dst != src is mandatory
requirements for crypto drivers or we can skip this copy. Actually I
am bit confused, In following link Stephen had mentioned caller will
memcpy the AAD to destination buffer but authenc.c also copies the AAD
to dest. buffer.

http://www.chronox.de/libkcapi/html/ch02s02.html

Secondly When AAD data remains unchanged in AEAD encryption/decryption
operations. Why we copy the same data to destination buffer?

Thanks & Regards
Harsh Jain

^ permalink raw reply

* Re: CONFIG_FIPS without module loading support?
From: Herbert Xu @ 2016-09-02 14:22 UTC (permalink / raw)
  To: NTU; +Cc: linux-crypto
In-Reply-To: <CAM5Ud7Pw84GcjpooX_5VfBuRTgU8+HMKkM6GBnD1z=JEw0PkXw@mail.gmail.com>

NTU <neotheuser@gmail.com> wrote:
> Ok, can we merge that in?

We can if you send us a patch :)
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v2 3/4] hw_random: jz4780-rng: Add RNG node to jz4780.dtsi
From: PrasannaKumar Muralidharan @ 2016-09-02 12:57 UTC (permalink / raw)
  To: Paul Burton
  Cc: mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ, Herbert Xu,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Ralf Baechle, Greg KH,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	harvey.hunt-1AXoQHu6uovQT0dZR+AlfA, prarit-H+wXaHxf7aLQT0dZR+AlfA,
	Florian Fainelli, joshua.henderson-UWL1GkI3JZL3oGB3hsPCZA,
	narmstrong-rdvid1DuHRBWk0Htik3J/w, Linus Walleij,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA
In-Reply-To: <4a7fb1cb-e0d4-31b7-7016-35adb63a659d-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

> I don't like this change. The RNG registers are documented as a part of
> the same hardware block as the clock & power stuff which the CGU driver
> handles, and indeed in the M200 SoC there is a power-related register
> after the RNG registers. So shortening the range covered by the CGU
> driver is not the right way to go.

Could not find M200 SoC PM in ingenic's website or ftp. So did not notice this.

> Perhaps you could instead have the CGU driver make use of the syscon
> infrastructure to expose a regmap which your RNG driver could pick up & use?

I will see how to use syscon and provide an updated patch.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 3/4] hw_random: jz4780-rng: Add RNG node to jz4780.dtsi
From: Paul Burton @ 2016-09-02 12:47 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: mpm, herbert, robh+dt, mark.rutland, ralf, gregkh,
	boris.brezillon, harvey.hunt, prarit, f.fainelli,
	joshua.henderson, narmstrong, linus.walleij, linux-crypto,
	devicetree, linux-mips
In-Reply-To: <1472321697-3094-4-git-send-email-prasannatsmkumar@gmail.com>



On 27/08/16 19:14, PrasannaKumar Muralidharan wrote:
> This patch adds RNG node to jz4780.dtsi.
> 
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index b868b42..f11d139 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -36,7 +36,7 @@
>  
>  	cgu: jz4780-cgu@10000000 {
>  		compatible = "ingenic,jz4780-cgu";
> -		reg = <0x10000000 0x100>;
> +		reg = <0x10000000 0xD8>;

Hi PrasannaKumar,

I don't like this change. The RNG registers are documented as a part of
the same hardware block as the clock & power stuff which the CGU driver
handles, and indeed in the M200 SoC there is a power-related register
after the RNG registers. So shortening the range covered by the CGU
driver is not the right way to go.

Perhaps you could instead have the CGU driver make use of the syscon
infrastructure to expose a regmap which your RNG driver could pick up & use?

Thanks,
    Paul

>  
>  		clocks = <&ext>, <&rtc>;
>  		clock-names = "ext", "rtc";
> @@ -44,6 +44,11 @@
>  		#clock-cells = <1>;
>  	};
>  
> +	rng: jz4780-rng@100000D8 {
> +		compatible = "ingenic,jz4780-rng";
> +		reg = <0x100000D8 0x8>;
> +	};
> +
>  	uart0: serial@10030000 {
>  		compatible = "ingenic,jz4780-uart";
>  		reg = <0x10030000 0x100>;
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox