[PATCH] crypto: CTR DRBG - in-place cipher operation
The cipher implementations of the kernel crypto API favor in-place
cipher operations. Thus, switch the CTR cipher operation in the DRBG to
perform in-place operations. This is implemented by using the output
buffer as input buffer and zeroizing it before the cipher operation to
implement a CTR encryption of a NULL buffer.
The speed improvement is quite visibile with the following comparison
using the LRNG implementation.
Without the patch set:
16 bytes| 12.267661 MB/s|61338304 bytes | 500213 ns
32 bytes| 23.603770 MB/s| 118018848 bytes | 500073 ns
64 bytes| 46.732262 MB/s| 233661312 bytes | 500241 ns
128 bytes| 90.038042 MB/s| 450190208 bytes | 500244 ns
256 bytes| 160.399616 MB/s| 801998080 bytes | 500393 ns
512 bytes| 259.878400 MB/s| 1299392000 bytes | 501675 ns
1024 bytes| 386.050662 MB/s| 1930253312 bytes | 501661 ns
2048 bytes| 493.641728 MB/s| 2468208640 bytes | 501598 ns
4096 bytes| 581.835981 MB/s| 2909179904 bytes | 503426 ns
With the patch set:
16 bytes | 17.051142 MB/s | 85255712 bytes | 500854 ns
32 bytes | 32.695898 MB/s |163479488 bytes | 500544 ns
64 bytes | 64.490739 MB/s |322453696 bytes | 500954 ns
128 bytes |123.285043 MB/s |616425216 bytes | 500201 ns
256 bytes |233.434573 MB/s | 1167172864 bytes | 500573 ns
512 bytes |384.405197 MB/s | 1922025984 bytes | 500671 ns
1024 bytes |566.313370 MB/s | 2831566848 bytes | 501080 ns
2048 bytes |744.518042 MB/s | 3722590208 bytes | 500926 ns
4096 bytes |867.501670 MB/s | 4337508352 bytes | 502181 ns
Signed-off-by: Stephan Mueller
---
crypto/drbg.c | 34 ++
include/crypto/drbg.h | 2 --
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index ee302fd229ad..bc52d9562611 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -261,8 +261,7 @@ static int drbg_fini_sym_kernel(struct drbg_state *drbg);
static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
u8 *inbuf, u32 inbuflen,
u8 *outbuf, u32 outlen);
-#define DRBG_CTR_NULL_LEN 128
-#define DRBG_OUTSCRATCHLEN DRBG_CTR_NULL_LEN
+#define DRBG_OUTSCRATCHLEN 256
/* BCC function for CTR DRBG as defined in 10.4.3 */
static int drbg_ctr_bcc(struct drbg_state *drbg,
@@ -555,8 +554,7 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
}
/* 10.2.1.5.2 step 4.1 */
- ret = drbg_kcapi_sym_ctr(drbg, drbg->ctr_null_value, DRBG_CTR_NULL_LEN,
-buf, len);
+ ret = drbg_kcapi_sym_ctr(drbg, NULL, 0, buf, len);
if (ret)
return ret;
@@ -1644,9 +1642,6 @@ static int drbg_fini_sym_kernel(struct drbg_state *drbg)
skcipher_request_free(drbg->ctr_req);
drbg->ctr_req = NULL;
- kfree(drbg->ctr_null_value_buf);
- drbg->ctr_null_value = NULL;
-
kfree(drbg->outscratchpadbuf);
drbg->outscratchpadbuf = NULL;
@@ -1697,15 +1692,6 @@ static int drbg_init_sym_kernel(struct drbg_state *drbg)
crypto_req_done, >ctr_wait);
alignmask = crypto_skcipher_alignmask(sk_tfm);
- drbg->ctr_null_value_buf = kzalloc(DRBG_CTR_NULL_LEN + alignmask,
- GFP_KERNEL);
- if (!drbg->ctr_null_value_buf) {
- drbg_fini_sym_kernel(drbg);
- return -ENOMEM;
- }
- drbg->ctr_null_value = (u8 *)PTR_ALIGN(drbg->ctr_null_value_buf,
- alignmask + 1);
-
drbg->outscratchpadbuf = kmalloc(DRBG_OUTSCRATCHLEN + alignmask,
GFP_KERNEL);
if (!drbg->outscratchpadbuf) {
@@ -1716,7 +1702,7 @@ static int drbg_init_sym_kernel(struct drbg_state *drbg)
alignmask + 1);
sg_init_table(>sg_in, 1);
- sg_init_table(>sg_out, 1);
+ sg_init_one(>sg_out, drbg->outscratchpad, DRBG_OUTSCRATCHLEN);
return alignmask;
}
@@ -1747,10 +1733,18 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
u8 *outbuf, u32 outlen)
{
struct scatterlist *sg_in = >sg_in, *sg_out = >sg_out;
+ u32 scratchpad_use = min_t(u32, outlen, DRBG_OUTSCRATCHLEN);
int ret;
- sg_set_buf(sg_in, inbuf, inlen);
- sg_set_buf(sg_out, drbg->outscratchpad, DRBG_OUTSCRATCHLEN);
+ if (inbuf) {
+ /* Use caller-provided input buffer */
+ sg_set_buf(sg_in, inbuf, inlen);
+ } else {
+ /* Use scratchpad for in-place operation */
+
Re: [PATCH 1/3] dt-bindings: Add bindings for Hisilicon SEC crypto accelerators.
On Fri, 20 Jul 2018 10:30:10 -0600
Rob Herring wrote:
> On Mon, Jul 16, 2018 at 11:43:40AM +0100, Jonathan Cameron wrote:
> > The hip06 and hip07 SoCs contain a number of these crypto units which
> > accelerate AES and DES operations.
> >
> > Signed-off-by: Jonathan Cameron
> > ---
> > .../bindings/crypto/hisilicon,hip07-sec.txt| 69
> > ++
> > 1 file changed, 69 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/crypto/hisilicon,hip07-sec.txt
> > b/Documentation/devicetree/bindings/crypto/hisilicon,hip07-sec.txt
> > new file mode 100644
> > index ..00b838706c98
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/hisilicon,hip07-sec.txt
> > @@ -0,0 +1,69 @@
> > +* Hisilicon hip07 Security Accelerator (SEC)
> > +
> > +Required properties:
> > +- compatible: Must contain one of
> > + - "hisilicon,hip06-sec"
> > + - "hisilicon,hip07-sec"
> > +- reg: Memory addresses and lengths of the memory regions used by the
> > driver.
>
> You know my feelings about the word "driver" in bindings. :)
Oops. Will fix. Oddly rare I actually write one of these docs so making newbie
mistakes :(
>
> > + Region 0 has registers to control the backend processing engines.
> > + Region 1 has registers for functionality common to all queues.
> > + Regions 2-18 have registers for the individual queues which are isolated
> > + both in hardware and within the driver.
>
> It's always 16 queues? Might state that somewhere.
Technically complex because some of them may be in use by the secure world
and hence not available and not seen in the DT.
Right now the driver doesn't support that, and it's not happening on any
existing platforms but I was trying to avoid stating there were definitely
16 available. I guess if that happens I'll fix the binding when they do it.
(moderately unlikely to happen now given age of this platform, but you never
know).
>
> > +- interrupts: Interrupt specifiers.
> > + Refer to interrupt-controller/interrupts.txt for generic interrupt
> > client node
> > + bindings.
> > + Interrupt 0 is for the SEC unit error queue.
> > + Interrupt 2N + 1 is the completion interrupt for queue N.
> > + Interrupt 2N + 2 is the error interrupt for queue N.
> > +- dma-coherent: The driver assumes coherent dma is possible.
> > +
> > +Optional properties:
> > +- iommus: The SEC units are behind smmu-v3 iommus.
> > + Refer to iommu/arm,smmu-v3.txt for more information.
> > +
> > +Example:
> > +Second socket, first unit chosen to illustrate need for 64 bit addresses.
>
> I don't follow the 64-bit address comment.
Without it the address is in the 32bit range so internal review raised
the question of why we needed to provide 64 bit registers as 32 bit ones
are more compact.
reg = <0xd00 0x1
etc
>
> > +
> > +p1_sec_a: sec@d200 {
>
> crypto@...
>
> The unit-address should be from the 1st reg entry and why isn't the
> 0x400 part included?
Will fix here an obviously in the DT patch itself.
>
> > + compatible = "hisilicon,hip07-sec";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
>
> These aren't needed here as there are no child nodes.
Oops I always forget those.
>
> > + reg = <0x400 0xd000 0x0 0x1
> > + 0x400 0xd200 0x0 0x1
> > + 0x400 0xd201 0x0 0x1
> > + 0x400 0xd202 0x0 0x1
> > + 0x400 0xd203 0x0 0x1
> > + 0x400 0xd204 0x0 0x1
> > + 0x400 0xd205 0x0 0x1
> > + 0x400 0xd206 0x0 0x1
> > + 0x400 0xd207 0x0 0x1
> > + 0x400 0xd208 0x0 0x1
> > + 0x400 0xd209 0x0 0x1
> > + 0x400 0xd20a 0x0 0x1
> > + 0x400 0xd20b 0x0 0x1
> > + 0x400 0xd20c 0x0 0x1
> > + 0x400 0xd20d 0x0 0x1
> > + 0x400 0xd20e 0x0 0x1
> > + 0x400 0xd20f 0x0 0x1
> > + 0x400 0xd210 0x0 0x1>;
> > + interrupt-parent = <_mbigen_sec_a>;
> > + iommus = <_smmu_alg_a 0x600>;
> > + dma-coherent;
> > + interrupts = <576 4>,
> > +<577 1>,<578 4>,
>
> space needed after the comma.
>
> > +<579 1>,<580 4>,
> > +<581 1>,<582 4>,
> > +<583 1>,<584 4>,
> > +<585 1>,<586 4>,
> > +<587 1>,<588 4>,
> > +<589 1>,<590 4>,
> > +<591 1>,<592 4>,
> > +<593 1>,<594 4>,
> > +<595 1>,<596 4>,
> > +<597 1>,<598 4>,
> > +<599 1>,<600 4>,
> > +<601 1>,<602 4>,
> > +<603 1>,<604 4>,
> > +<605 1>,<606 4>,
> > +<607 1>,<608 4>;
> > +};
> > --
> > 2.16.2
Thanks Rob.
Jonathan
> >
> >
Re: [PATCH 1/3] dt-bindings: Add bindings for Hisilicon SEC crypto accelerators.
On Mon, Jul 16, 2018 at 11:43:40AM +0100, Jonathan Cameron wrote:
> The hip06 and hip07 SoCs contain a number of these crypto units which
> accelerate AES and DES operations.
>
> Signed-off-by: Jonathan Cameron
> ---
> .../bindings/crypto/hisilicon,hip07-sec.txt| 69
> ++
> 1 file changed, 69 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/crypto/hisilicon,hip07-sec.txt
> b/Documentation/devicetree/bindings/crypto/hisilicon,hip07-sec.txt
> new file mode 100644
> index ..00b838706c98
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/hisilicon,hip07-sec.txt
> @@ -0,0 +1,69 @@
> +* Hisilicon hip07 Security Accelerator (SEC)
> +
> +Required properties:
> +- compatible: Must contain one of
> + - "hisilicon,hip06-sec"
> + - "hisilicon,hip07-sec"
> +- reg: Memory addresses and lengths of the memory regions used by the driver.
You know my feelings about the word "driver" in bindings. :)
> + Region 0 has registers to control the backend processing engines.
> + Region 1 has registers for functionality common to all queues.
> + Regions 2-18 have registers for the individual queues which are isolated
> + both in hardware and within the driver.
It's always 16 queues? Might state that somewhere.
> +- interrupts: Interrupt specifiers.
> + Refer to interrupt-controller/interrupts.txt for generic interrupt client
> node
> + bindings.
> + Interrupt 0 is for the SEC unit error queue.
> + Interrupt 2N + 1 is the completion interrupt for queue N.
> + Interrupt 2N + 2 is the error interrupt for queue N.
> +- dma-coherent: The driver assumes coherent dma is possible.
> +
> +Optional properties:
> +- iommus: The SEC units are behind smmu-v3 iommus.
> + Refer to iommu/arm,smmu-v3.txt for more information.
> +
> +Example:
> +Second socket, first unit chosen to illustrate need for 64 bit addresses.
I don't follow the 64-bit address comment.
> +
> +p1_sec_a: sec@d200 {
crypto@...
The unit-address should be from the 1st reg entry and why isn't the
0x400 part included?
> + compatible = "hisilicon,hip07-sec";
> + #address-cells = <2>;
> + #size-cells = <2>;
These aren't needed here as there are no child nodes.
> + reg = <0x400 0xd000 0x0 0x1
> +0x400 0xd200 0x0 0x1
> +0x400 0xd201 0x0 0x1
> +0x400 0xd202 0x0 0x1
> +0x400 0xd203 0x0 0x1
> +0x400 0xd204 0x0 0x1
> +0x400 0xd205 0x0 0x1
> +0x400 0xd206 0x0 0x1
> +0x400 0xd207 0x0 0x1
> +0x400 0xd208 0x0 0x1
> +0x400 0xd209 0x0 0x1
> +0x400 0xd20a 0x0 0x1
> +0x400 0xd20b 0x0 0x1
> +0x400 0xd20c 0x0 0x1
> +0x400 0xd20d 0x0 0x1
> +0x400 0xd20e 0x0 0x1
> +0x400 0xd20f 0x0 0x1
> +0x400 0xd210 0x0 0x1>;
> + interrupt-parent = <_mbigen_sec_a>;
> + iommus = <_smmu_alg_a 0x600>;
> + dma-coherent;
> + interrupts = <576 4>,
> + <577 1>,<578 4>,
space needed after the comma.
> + <579 1>,<580 4>,
> + <581 1>,<582 4>,
> + <583 1>,<584 4>,
> + <585 1>,<586 4>,
> + <587 1>,<588 4>,
> + <589 1>,<590 4>,
> + <591 1>,<592 4>,
> + <593 1>,<594 4>,
> + <595 1>,<596 4>,
> + <597 1>,<598 4>,
> + <599 1>,<600 4>,
> + <601 1>,<602 4>,
> + <603 1>,<604 4>,
> + <605 1>,<606 4>,
> + <607 1>,<608 4>;
> +};
> --
> 2.16.2
>
>
Re: [PATCH 1/2] crypto: DH - update test for public key verification
On Wed, Jul 11, 2018 at 08:35:49PM +0200, Stephan Müller wrote: > By adding a zero byte-length for the DH parameter Q value, the public > key verification test is disabled for the given test. > > Reported-by: Eric Biggers > Signed-off-by: Stephan Mueller Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: sharah: Unregister correct algorithms for SAHARA 3
On Sun, Jul 15, 2018 at 12:27:06AM +0200, Michael Müller wrote: > This patch fixes two typos related to unregistering algorithms supported by > SAHARAH 3. In sahara_register_algs the wrong algorithms are unregistered > in case of an error. In sahara_unregister_algs the wrong array is used to > determine the iteration count. > > Signed-off-by: Michael Müller Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 2/2] crypto: ECDH - fix typo of P-192 b value
On Wed, Jul 11, 2018 at 08:36:23PM +0200, Stephan Müller wrote: > Fix the b value to be compliant with FIPS 186-4 D.1.2.1. This fix is > required to make sure the SP800-56A public key test passes for P-192. > > Signed-off-by: Stephan Mueller Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 2/2] crypto: DRBG - use caller buffer if suitable
On Fri, Jul 20, 2018 at 07:09:05AM +0200, Stephan Mueller wrote: > > Maybe I have a different understanding of how such interface should look like. > > Can you give me some more detail on how you envision such virtual address > interface should work? It should look like shash. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: inside-secure - initialize first_rdesc to make GCC happy
On Fri, Jul 13, 2018 at 05:43:16PM +0200, Antoine Tenart wrote: > In the cipher safexcel_send_req function, GCC warns that > first_rdesc may be used uninitialized. While this should never > happen, this patch removes the warning by initializing this > variable to NULL to make GCC happy. > > This was reported by the kbuild test robot. > > Signed-off-by: Antoine Tenart Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 1/2] crypto: DRBG - eliminate constant reinitialization of SGL
On Tue, Jul 10, 2018 at 05:56:33PM +0200, Stephan Müller wrote: > The CTR DRBG requires two SGLs pointing to input/output buffers for the > CTR AES operation. The used SGLs always have only one entry. Thus, the > SGL can be initialized during allocation time, preventing a > re-initialization of the SGLs during each call. > > The performance is increased by about 1 to 3 percent depending on the > size of the requested buffer size. > > Signed-off-by: Stephan Mueller Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 2/2] crypto: DRBG - use caller buffer if suitable
On Fri, Jul 20, 2018 at 08:08:22AM +0200, Stephan Mueller wrote: > > - should it be synchronous like blkcipher? It should be synchronous. > - the TFMs (cipher Impls and templates) all operate on SGLs - should a virt > API simply convert a virt address into an SGL? If so, the problem that > triggered this thread is still not solved but only pushed to a different > layer. If it should not just be an SGL wrapper, should all TFMs be changed? Obviously if we're going to do this then we would be converting the underlying algorithms over to the virtual address-based interface just like shash. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: inside-secure - switch to SPDX identifiers
On Fri, Jul 13, 2018 at 04:51:37PM +0200, Antoine Tenart wrote: > Use the appropriate SPDX license identifiers and drop the license text. > This patch is only cosmetic. > > Signed-off-by: Antoine Tenart Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: dh - fix memory leak
On Tue, Jul 10, 2018 at 09:22:52AM -0500, Gustavo A. R. Silva wrote:
> In case memory resources for *base* were allocated, release them
> before return.
>
> Addresses-Coverity-ID: 1471702 ("Resource leak")
> Fixes: e3fe0ae12962 ("crypto: dh - add public key verification test")
> Signed-off-by: Gustavo A. R. Silva
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 2/2] crypto: DRBG - use caller buffer if suitable
>> On Fri, Jul 20, 2018 at 07:09:05AM +0200, Stephan Mueller wrote: >> >> Maybe I have a different understanding of how such interface should look >> like. >> >> Can you give me some more detail on how you envision such virtual address >> interface should work? > > It should look like shash. > Ok, but then: - should it be synchronous like blkcipher? - the TFMs (cipher Impls and templates) all operate on SGLs - should a virt API simply convert a virt address into an SGL? If so, the problem that triggered this thread is still not solved but only pushed to a different layer. If it should not just be an SGL wrapper, should all TFMs be changed? Thanks
