Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: algif for compression?
From: Stephan Müller @ 2017-04-03  7:26 UTC (permalink / raw)
  To: abed mohammad kamaluddin; +Cc: Herbert Xu, linux-crypto
In-Reply-To: <CACVarEQ14ZF9LGDkaNYtX3GRD5GOH62NBiPtC4z=i6RY4MtFDA@mail.gmail.com>

Am Montag, 3. April 2017, 08:10:19 CEST schrieb abed mohammad kamaluddin:

Hi abed,

> Hi Herbert,
> 
> We have implemented algif acomp interface for user space applications
> to utilize the kernel compression framework and the hardware drivers
> using it and have been testing it using userspace zlib library.
> 
> However, what we find lacking in the existing acomp implementation is
> the ability to pass context data between the applications and the
> drivers using the acomp interface. Currently the interface only allows
> src/dest data  and a flag argument with each request. There are two
> context pointers, one in acomp_req and another in crypto_tfm but they
> are for internal use and not available to applications through the
> api's. Would it be acceptable to add fields that need to be
> communicated b/w the driver and applications like history,
> csum/adler32, EOF, stream ctx  to acomp_req.
> 
> Or is there any other way, which I may have missed, through which we
> can pass ctx data between applications and drivers while using the
> kernel compression framework?

If you follow the AF_ALG implementations that are currently in the kernel, a 
calling application receives two file descriptors. The first file descriptor 
is the reference to a tfm, the second is the reference to a particular 
request.

Wouldn't those file descriptors be the reference you are looking for?

Ciao
Stephan

^ permalink raw reply

* Re: algif for compression?
From: abed mohammad kamaluddin @ 2017-04-03  8:41 UTC (permalink / raw)
  To: Stephan Müller; +Cc: Herbert Xu, linux-crypto
In-Reply-To: <12030384.uXSX3S3VSk@tauon.chronox.de>

Hi Stephen,

> If you follow the AF_ALG implementations that are currently in the kernel, a
> calling application receives two file descriptors. The first file descriptor
> is the reference to a tfm, the second is the reference to a particular
> request.
>
> Wouldn't those file descriptors be the reference you are looking for?

Those descriptors are sufficient to pass data from userspace
applications to the algif interface.
However the issue is passing those from the interface to the driver
using the current acomp API's.
This is the currently exposed interface to the comp framework. Am I
missing something here?

int (*compress)(struct acomp_req *req);
int (*decompress)(struct acomp_req *req);

struct acomp_req {
        struct crypto_async_request base;
        struct scatterlist *src;
        struct scatterlist *dst;
        unsigned int slen;
        unsigned int dlen;
        u32 flags;
        void *__ctx[] CRYPTO_MINALIGN_ATTR;
};


Thanks
Abed



On Mon, Apr 3, 2017 at 12:56 PM, Stephan Müller <smueller@chronox.de> wrote:
> Am Montag, 3. April 2017, 08:10:19 CEST schrieb abed mohammad kamaluddin:
>
> Hi abed,
>
>> Hi Herbert,
>>
>> We have implemented algif acomp interface for user space applications
>> to utilize the kernel compression framework and the hardware drivers
>> using it and have been testing it using userspace zlib library.
>>
>> However, what we find lacking in the existing acomp implementation is
>> the ability to pass context data between the applications and the
>> drivers using the acomp interface. Currently the interface only allows
>> src/dest data  and a flag argument with each request. There are two
>> context pointers, one in acomp_req and another in crypto_tfm but they
>> are for internal use and not available to applications through the
>> api's. Would it be acceptable to add fields that need to be
>> communicated b/w the driver and applications like history,
>> csum/adler32, EOF, stream ctx  to acomp_req.
>>
>> Or is there any other way, which I may have missed, through which we
>> can pass ctx data between applications and drivers while using the
>> kernel compression framework?
>
> If you follow the AF_ALG implementations that are currently in the kernel, a
> calling application receives two file descriptors. The first file descriptor
> is the reference to a tfm, the second is the reference to a particular
> request.
>
> Wouldn't those file descriptors be the reference you are looking for?
>
> Ciao
> Stephan

^ permalink raw reply

* [PATCH] crypto: caam - fix JR platform device subsequent (re)creations
From: Horia Geantă @ 2017-04-03 15:12 UTC (permalink / raw)
  To: Herbert Xu, Rob Herring, Russell King
  Cc: David S. Miller, linux-crypto, Dan Douglass, Ruchika Gupta
In-Reply-To: <CAL_JsqLHEycZLaxLZcAnDS4gp6D8xQDpgZXV1g3dokzF_EeQtQ@mail.gmail.com>

The way Job Ring platform devices are created and released does not
allow for multiple create-release cycles.

JR0 Platform device creation error
JR0 Platform device creation error
caam 2100000.caam: no queues configured, terminating
caam: probe of 2100000.caam failed with error -12

The reason is that platform devices are created for each job ring:

        for_each_available_child_of_node(nprop, np)
                if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
                    of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
                        ctrlpriv->jrpdev[ring] =
                                of_platform_device_create(np, NULL, dev);

which sets OF_POPULATED on the device node, but then it cleans these up:

        /* Remove platform devices for JobRs */
        for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
                if (ctrlpriv->jrpdev[ring])
                        of_device_unregister(ctrlpriv->jrpdev[ring]);
        }

which leaves OF_POPULATED set.

Use of_platform_populate / of_platform_depopulate instead.
This allows for a bit of driver clean-up, jrpdev is no longer needed.

Logic changes a bit too:
-exit in case of_platform_populate fails, since currently even QI backend
depends on JR; true, we no longer support the case when "some" of the JR
DT nodes are incorrect
-when cleaning up, caam_remove() would also depopulate RTIC in case
it would have been populated somewhere else - not the case for now

Fixes: 313ea293e9c4d ("crypto: caam - Add Platform driver for Job Ring")
Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
Suggested-by: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
Not sending this directly to -stable, since it does not apply cleanly.

 drivers/crypto/caam/ctrl.c   | 64 ++++++++++++++------------------------------
 drivers/crypto/caam/intern.h |  1 -
 2 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index b3a94d5eff26..f7792a99469a 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -305,15 +305,13 @@ static int caam_remove(struct platform_device *pdev)
 	struct device *ctrldev;
 	struct caam_drv_private *ctrlpriv;
 	struct caam_ctrl __iomem *ctrl;
-	int ring;
 
 	ctrldev = &pdev->dev;
 	ctrlpriv = dev_get_drvdata(ctrldev);
 	ctrl = (struct caam_ctrl __iomem *)ctrlpriv->ctrl;
 
-	/* Remove platform devices for JobRs */
-	for (ring = 0; ring < ctrlpriv->total_jobrs; ring++)
-		of_device_unregister(ctrlpriv->jrpdev[ring]);
+	/* Remove platform devices under the crypto node */
+	of_platform_depopulate(ctrldev);
 
 #ifdef CONFIG_CAAM_QI
 	if (ctrlpriv->qidev)
@@ -410,10 +408,21 @@ int caam_get_era(void)
 }
 EXPORT_SYMBOL(caam_get_era);
 
+static const struct of_device_id caam_match[] = {
+	{
+		.compatible = "fsl,sec-v4.0",
+	},
+	{
+		.compatible = "fsl,sec4.0",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, caam_match);
+
 /* Probe routine for CAAM top (controller) level */
 static int caam_probe(struct platform_device *pdev)
 {
-	int ret, ring, ridx, rspec, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
+	int ret, ring, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
 	u64 caam_id;
 	struct device *dev;
 	struct device_node *nprop, *np;
@@ -589,21 +598,9 @@ static int caam_probe(struct platform_device *pdev)
 		goto iounmap_ctrl;
 	}
 
-	/*
-	 * Detect and enable JobRs
-	 * First, find out how many ring spec'ed, allocate references
-	 * for all, then go probe each one.
-	 */
-	rspec = 0;
-	for_each_available_child_of_node(nprop, np)
-		if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
-		    of_device_is_compatible(np, "fsl,sec4.0-job-ring"))
-			rspec++;
-
-	ctrlpriv->jrpdev = devm_kcalloc(&pdev->dev, rspec,
-					sizeof(*ctrlpriv->jrpdev), GFP_KERNEL);
-	if (ctrlpriv->jrpdev == NULL) {
-		ret = -ENOMEM;
+	ret = of_platform_populate(nprop, caam_match, NULL, dev);
+	if (ret) {
+		dev_err(dev, "JR platform devices creation error\n");
 		goto iounmap_ctrl;
 	}
 
@@ -618,29 +615,19 @@ static int caam_probe(struct platform_device *pdev)
 	ctrlpriv->dfs_root = debugfs_create_dir(dev_name(dev), NULL);
 	ctrlpriv->ctl = debugfs_create_dir("ctl", ctrlpriv->dfs_root);
 #endif
+
 	ring = 0;
-	ridx = 0;
-	ctrlpriv->total_jobrs = 0;
 	for_each_available_child_of_node(nprop, np)
 		if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
 		    of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
-			ctrlpriv->jrpdev[ring] =
-				of_platform_device_create(np, NULL, dev);
-			if (!ctrlpriv->jrpdev[ring]) {
-				pr_warn("JR physical index %d: Platform device creation error\n",
-					ridx);
-				ridx++;
-				continue;
-			}
 			ctrlpriv->jr[ring] = (struct caam_job_ring __iomem __force *)
 					     ((__force uint8_t *)ctrl +
-					     (ridx + JR_BLOCK_NUMBER) *
+					     (ring + JR_BLOCK_NUMBER) *
 					      BLOCK_OFFSET
 					     );
 			ctrlpriv->total_jobrs++;
 			ring++;
-			ridx++;
-	}
+		}
 
 	/* Check to see if QI present. If so, enable */
 	ctrlpriv->qi_present =
@@ -849,17 +836,6 @@ static int caam_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static struct of_device_id caam_match[] = {
-	{
-		.compatible = "fsl,sec-v4.0",
-	},
-	{
-		.compatible = "fsl,sec4.0",
-	},
-	{},
-};
-MODULE_DEVICE_TABLE(of, caam_match);
-
 static struct platform_driver caam_driver = {
 	.driver = {
 		.name = "caam",
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index c334df638ff6..85b6c5835b8f 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -66,7 +66,6 @@ struct caam_drv_private_jr {
 struct caam_drv_private {
 
 	struct device *dev;
-	struct platform_device **jrpdev; /* Alloc'ed array per sub-device */
 #ifdef CONFIG_CAAM_QI
 	struct device *qidev;
 #endif
-- 
2.12.0.264.gd6db3f216544

^ permalink raw reply related

* [PATCH] crypto: caam - fix invalid dereference in caam_rsa_init_tfm()
From: Horia Geantă @ 2017-04-03 15:30 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, linux-crypto, Dan Douglass

In case caam_jr_alloc() fails, ctx->dev carries the error code,
thus accessing it with dev_err() is incorrect.

Cc: <stable@vger.kernel.org> # 4.8+
Fixes: 8c419778ab57e ("crypto: caam - add support for RSA algorithm")
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
 drivers/crypto/caam/caampkc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index 32100c4851dd..49cbdcba7883 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -506,7 +506,7 @@ static int caam_rsa_init_tfm(struct crypto_akcipher *tfm)
 	ctx->dev = caam_jr_alloc();
 
 	if (IS_ERR(ctx->dev)) {
-		dev_err(ctx->dev, "Job Ring Device allocation for transform failed\n");
+		pr_err("Job Ring Device allocation for transform failed\n");
 		return PTR_ERR(ctx->dev);
 	}
 
-- 
2.12.0.264.gd6db3f216544

^ permalink raw reply related

* Re: [PATCH] crypto: caam - fix JR platform device subsequent (re)creations
From: Rob Herring @ 2017-04-03 15:52 UTC (permalink / raw)
  To: Horia Geantă
  Cc: Herbert Xu, Russell King, David S. Miller, linux-crypto,
	Dan Douglass, Ruchika Gupta
In-Reply-To: <20170403151204.27488-1-horia.geanta@nxp.com>

On Mon, Apr 3, 2017 at 10:12 AM, Horia Geantă <horia.geanta@nxp.com> wrote:
> The way Job Ring platform devices are created and released does not
> allow for multiple create-release cycles.
>
> JR0 Platform device creation error
> JR0 Platform device creation error
> caam 2100000.caam: no queues configured, terminating
> caam: probe of 2100000.caam failed with error -12
>
> The reason is that platform devices are created for each job ring:
>
>         for_each_available_child_of_node(nprop, np)
>                 if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
>                     of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
>                         ctrlpriv->jrpdev[ring] =
>                                 of_platform_device_create(np, NULL, dev);
>
> which sets OF_POPULATED on the device node, but then it cleans these up:
>
>         /* Remove platform devices for JobRs */
>         for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
>                 if (ctrlpriv->jrpdev[ring])
>                         of_device_unregister(ctrlpriv->jrpdev[ring]);
>         }
>
> which leaves OF_POPULATED set.
>
> Use of_platform_populate / of_platform_depopulate instead.
> This allows for a bit of driver clean-up, jrpdev is no longer needed.
>
> Logic changes a bit too:
> -exit in case of_platform_populate fails, since currently even QI backend
> depends on JR; true, we no longer support the case when "some" of the JR
> DT nodes are incorrect
> -when cleaning up, caam_remove() would also depopulate RTIC in case
> it would have been populated somewhere else - not the case for now
>
> Fixes: 313ea293e9c4d ("crypto: caam - Add Platform driver for Job Ring")
> Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
> Suggested-by: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v5] DH support: add KDF handling support
From: David Howells @ 2017-04-03 22:22 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: dhowells, mathew.j.martineau, herbert, linux-crypto, keyrings
In-Reply-To: <7815450.KhQaSIWsFB@positron.chronox.de>

Stephan Mueller <smueller@chronox.de> wrote:

> +	struct keyctl_dh_params params = { .private = private,

That doesn't compile.  I think you meant ".priv".

David

^ permalink raw reply

* Re: [PATCH v5] DH support: add KDF handling support
From: David Howells @ 2017-04-03 22:28 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: dhowells, mathew.j.martineau, herbert, linux-crypto, keyrings
In-Reply-To: <7815450.KhQaSIWsFB@positron.chronox.de>

Stephan Mueller <smueller@chronox.de> wrote:

> this patch changes the documentation, the naming of the variables
> and the test case to refer to the variable name of a hashname
> instead of kdfname to match the current kernel implementation.

It's also needs an update to man1/keyctl.1.

David

^ permalink raw reply

* Re: [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
From: Matthias Kaehlcke @ 2017-04-03 22:49 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, x86, linux-kbuild, linux-kernel, linux-crypto,
	linux-raid
In-Reply-To: <20170317001520.85223-2-md@google.com>

El Thu, Mar 16, 2017 at 05:15:14PM -0700 Michael Davidson ha dit:

> Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS
> for clang.
> 
> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index b841fb36beb2..b21fd0ca2946 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
>  # See modpost pattern 2
>  KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
>  KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
> +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as)
> +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as)
>  else
>  
>  # These warnings generated too much noise in a regular build.

Ping, any feedback on this patch?

Cheers

Matthias

^ permalink raw reply

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
From: Matthias Kaehlcke @ 2017-04-03 23:01 UTC (permalink / raw)
  To: hpa
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, x86, linux-kbuild, linux-kernel, linux-crypto,
	linux-raid
In-Reply-To: <FB827022-C732-432A-8F31-734C88EBF7A4@zytor.com>

El Fri, Mar 17, 2017 at 04:50:19PM -0700 hpa@zytor.com ha dit:

> On March 16, 2017 5:15:16 PM PDT, Michael Davidson <md@google.com> wrote:
> >Suppress clang warnings about potential unaliged accesses
> >to members in packed structs. This gets rid of almost 10,000
> >warnings about accesses to the ring 0 stack pointer in the TSS.
> >
> >Signed-off-by: Michael Davidson <md@google.com>
> >---
> > arch/x86/Makefile | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> >diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >index 894a8d18bf97..7f21703c475d 100644
> >--- a/arch/x86/Makefile
> >+++ b/arch/x86/Makefile
> >@@ -128,6 +128,11 @@ endif
> >         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> > endif
> > 
> >+ifeq ($(cc-name),clang)
> >+# Suppress clang warnings about potential unaligned accesses.
> >+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> >+endif
> >+
> > ifdef CONFIG_X86_X32
> > 	x32_ld_ok := $(call try-run,\
> > 			/bin/echo -e '1: .quad 1b' | \
> 
> Why conditional on clang?

My understanding is that this warning is clang specific, it is not
listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

Cheers

Matthias

^ permalink raw reply

* Re: [PATCH v5] KEYS: add SP800-56A KDF support for DH
From: David Howells @ 2017-04-03 23:04 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: dhowells, herbert, mathew.j.martineau, linux-crypto, keyrings
In-Reply-To: <1571629.ErTDR5PMQO@positron.chronox.de>

Pulled.

^ permalink raw reply

* Re: [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting
From: Matthias Kaehlcke @ 2017-04-03 23:14 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Herbert Xu,
	David S. Miller, Alexander Potapenko, Dmitry Vyukov, x86,
	linux-kernel, linux-crypto
In-Reply-To: <20170317001520.85223-8-md@google.com>

El Thu, Mar 16, 2017 at 05:15:20PM -0700 Michael Davidson ha dit:

> aes_ctrby8_avx-x86_64.S uses the C preprocessor for token pasting
> of character sequences that are not valid preprocessor tokens.
> While this is allowed when preprocessing assembler files it exposes
> an incompatibilty between the clang and gcc preprocessors where
> clang does not strip leading white space from macro parameters,
> leading to the CONCAT(%xmm, i) macro expansion on line 96 resulting
> in a token with a space character embedded in it.
> 
> While this could be fixed by deleting the offending space character,
> the assembler is perfectly capable of handling the token pasting
> correctly for itself so it seems preferable to let it do so and to
> get rid or the CONCAT(), DDQ() and XMM() preprocessor macros.
> 
> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> index a916c4a61165..5f6a5af9c489 100644
> --- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> +++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> @@ -65,7 +65,6 @@
>  #include <linux/linkage.h>
>  #include <asm/inst.h>
>  
> -#define CONCAT(a,b)	a##b
>  #define VMOVDQ		vmovdqu
>  
>  #define xdata0		%xmm0
> @@ -92,8 +91,6 @@
>  #define num_bytes	%r8
>  
>  #define tmp		%r10
> -#define	DDQ(i)		CONCAT(ddq_add_,i)
> -#define	XMM(i)		CONCAT(%xmm, i)
>  #define	DDQ_DATA	0
>  #define	XDATA		1
>  #define KEY_128		1
> @@ -131,12 +128,12 @@ ddq_add_8:
>  /* generate a unique variable for ddq_add_x */
>  
>  .macro setddq n
> -	var_ddq_add = DDQ(\n)
> +	var_ddq_add = ddq_add_\n
>  .endm
>  
>  /* generate a unique variable for xmm register */
>  .macro setxdata n
> -	var_xdata = XMM(\n)
> +	var_xdata = %xmm\n
>  .endm
>  
>  /* club the numeric 'id' to the symbol 'name' */

Any feedback on this patch?

Thanks

Matthias

^ permalink raw reply

* Re: [PATCH v5] DH support: add KDF handling support
From: Stephan Müller @ 2017-04-03 23:18 UTC (permalink / raw)
  To: David Howells; +Cc: mathew.j.martineau, herbert, linux-crypto, keyrings
In-Reply-To: <6447.1491258148@warthog.procyon.org.uk>

Am Dienstag, 4. April 2017, 00:22:28 CEST schrieb David Howells:

Hi David,

> Stephan Mueller <smueller@chronox.de> wrote:
> > +	struct keyctl_dh_params params = { .private = private,
> 
> That doesn't compile.  I think you meant ".priv".

I think some code has been changed since I prepared the patch. Allow me to 
port the patch to the current code tree.

I will also update the keyctl.1 man page.


Ciao
Stephan

^ permalink raw reply

* [PATCH v6] DH support: add KDF handling support
From: Stephan Müller @ 2017-04-04  0:04 UTC (permalink / raw)
  To: David Howells; +Cc: mathew.j.martineau, herbert, linux-crypto, keyrings

Hi David,

Changes v6:

* addition of man/keyctl.1 documentation for dh_compute_kdf
  and dh_compute_kdf_oi

* change use of .priv instead of .private to compile the code

Thanks
Stephan

---8<---

Add the interface logic to support DH with KDF handling support.

The dh_compute code now allows the following options:

- no KDF support / output of raw DH shared secret:
  dh_compute <private> <prime> <base>

- KDF support without "other information" string:
  dh_compute_kdf <private> <prime> <base> <output length> <hash_type>

- KDF support with "other information string:
  dh_compute_kdf_oi <private> <prime> <base> <output length> <hash_type>
  where the OI string is provided on STDIN.

The test to verify the code is based on a test vector used for the CAVS
testing of SP800-56A.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 Makefile                                 |   1 +
 keyctl.c                                 | 133 ++++++++++++++++++++++++
 keyutils.c                               |  14 +++
 keyutils.h                               |  11 ++
 man/keyctl.1                             |  26 +++++
 man/keyctl_dh_compute.3                  |  57 +++++++++++
 tests/keyctl/dh_compute/valid/runtest.sh | 168 +++++++++++++++++++++++++++++++
 tests/toolbox.inc.sh                     |  44 ++++++++
 version.lds                              |   2 +
 9 files changed, 456 insertions(+)

diff --git a/Makefile b/Makefile
index 824bbbf..90fc33f 100644
--- a/Makefile
+++ b/Makefile
@@ -195,6 +195,7 @@ endif
 	$(LNS) keyctl_read.3 $(DESTDIR)$(MAN3)/keyctl_read_alloc.3
 	$(LNS) recursive_key_scan.3 $(DESTDIR)$(MAN3)/recursive_session_key_scan.3
 	$(LNS) keyctl_dh_compute.3 $(DESTDIR)$(MAN3)/keyctl_dh_compute_alloc.3
+	$(LNS) keyctl_dh_compute.3 $(DESTDIR)$(MAN3)/keyctl_dh_compute_kdf.3
 	$(INSTALL) -D -m 0644 keyutils.h $(DESTDIR)$(INCLUDEDIR)/keyutils.h
 
 ###############################################################################
diff --git a/keyctl.c b/keyctl.c
index 801a864..0d1fac8 100644
--- a/keyctl.c
+++ b/keyctl.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 #include <asm/unistd.h>
 #include "keyutils.h"
+#include <limits.h>
 
 struct command {
 	void (*action)(int argc, char *argv[]) __attribute__((noreturn));
@@ -67,6 +68,8 @@ static nr void act_keyctl_purge(int argc, char *argv[]);
 static nr void act_keyctl_invalidate(int argc, char *argv[]);
 static nr void act_keyctl_get_persistent(int argc, char *argv[]);
 static nr void act_keyctl_dh_compute(int argc, char *argv[]);
+static nr void act_keyctl_dh_compute_kdf(int argc, char *argv[]);
+static nr void act_keyctl_dh_compute_kdf_oi(int argc, char *argv[]);
 
 const struct command commands[] = {
 	{ act_keyctl___version,	"--version",	"" },
@@ -76,6 +79,8 @@ const struct command commands[] = {
 	{ act_keyctl_clear,	"clear",	"<keyring>" },
 	{ act_keyctl_describe,	"describe",	"<keyring>" },
 	{ act_keyctl_dh_compute, "dh_compute",	"<private> <prime> <base>" },
+	{ act_keyctl_dh_compute_kdf, "dh_compute_kdf", "<private> <prime> <base> <len> <hash_name>" },
+	{ act_keyctl_dh_compute_kdf_oi, "dh_compute_kdf_oi", "<private> <prime> <base> <len> <hash_name>" },
 	{ act_keyctl_instantiate, "instantiate","<key> <data> <keyring>" },
 	{ act_keyctl_invalidate,"invalidate",	"<key>" },
 	{ act_keyctl_get_persistent, "get_persistent", "<keyring> [<uid>]" },
@@ -1663,6 +1668,7 @@ static void act_keyctl_dh_compute(int argc, char *argv[])
 		}
 
 		printf("%02hhx", *p);
+		*p = 0x00;	/* zeroize buffer */
 		p++;
 
 		col++;
@@ -1674,6 +1680,133 @@ static void act_keyctl_dh_compute(int argc, char *argv[])
 	} while (--ret > 0);
 
 	printf("\n");
+
+	free(buffer);
+
+	exit(0);
+}
+
+static void act_keyctl_dh_compute_kdf(int argc, char *argv[])
+{
+	key_serial_t private, prime, base;
+	char *buffer;
+	char *p;
+	int ret, sep, col;
+	unsigned long buflen = 0;
+
+	if (argc != 6)
+		format();
+
+	private = get_key_id(argv[1]);
+	prime = get_key_id(argv[2]);
+	base = get_key_id(argv[3]);
+
+	buflen = strtoul(argv[4], NULL, 10);
+	if (buflen == ULONG_MAX)
+		error("dh_compute: cannot convert generated length value");
+
+	buffer = malloc(buflen);
+	if (!buffer)
+		error("dh_compute: cannot allocate memory");
+
+	ret = keyctl_dh_compute_kdf(private, prime, base, argv[5], NULL,  0,
+				    buffer, buflen);
+	if (ret < 0)
+		error("keyctl_dh_compute_alloc");
+
+	/* hexdump the contents */
+	printf("%u bytes of data in result:\n", ret);
+
+	sep = 0;
+	col = 0;
+	p = buffer;
+
+	do {
+		if (sep) {
+			putchar(sep);
+			sep = 0;
+		}
+
+		printf("%02hhx", *p);
+		*p = 0x00;	/* zeroize buffer */
+		p++;
+
+		col++;
+		if (col % 32 == 0)
+			sep = '\n';
+		else if (col % 4 == 0)
+			sep = ' ';
+
+	} while (--ret > 0);
+
+	printf("\n");
+
+	free(buffer);
+
+	exit(0);
+}
+
+static void act_keyctl_dh_compute_kdf_oi(int argc, char *argv[])
+{
+	key_serial_t private, prime, base;
+	char *buffer;
+	char *p;
+	int ret, sep, col;
+	unsigned long buflen = 0;
+	size_t oilen;
+	void *oi;
+
+	if (argc != 6)
+		format();
+
+	private = get_key_id(argv[1]);
+	prime = get_key_id(argv[2]);
+	base = get_key_id(argv[3]);
+
+	buflen = strtoul(argv[4], NULL, 10);
+	if (buflen == ULONG_MAX)
+		error("dh_compute: cannot convert generated length value");
+
+	buffer = malloc(buflen);
+	if (!buffer)
+		error("dh_compute: cannot allocate memory");
+
+	oi = grab_stdin(&oilen);
+
+	ret = keyctl_dh_compute_kdf(private, prime, base, argv[5], oi,  oilen,
+				    buffer, buflen);
+	if (ret < 0)
+		error("keyctl_dh_compute_alloc");
+
+	/* hexdump the contents */
+	printf("%u bytes of data in result:\n", ret);
+
+	sep = 0;
+	col = 0;
+	p = buffer;
+
+	do {
+		if (sep) {
+			putchar(sep);
+			sep = 0;
+		}
+
+		printf("%02hhx", *p);
+		*p = 0x00;	/* zeroize buffer */
+		p++;
+
+		col++;
+		if (col % 32 == 0)
+			sep = '\n';
+		else if (col % 4 == 0)
+			sep = ' ';
+
+	} while (--ret > 0);
+
+	printf("\n");
+
+	free(buffer);
+
 	exit(0);
 }
 
diff --git a/keyutils.c b/keyutils.c
index a6325d0..a84ce46 100644
--- a/keyutils.c
+++ b/keyutils.c
@@ -244,6 +244,20 @@ long keyctl_dh_compute(key_serial_t priv, key_serial_t prime,
 	return keyctl(KEYCTL_DH_COMPUTE, &params, buffer, buflen, 0);
 }
 
+long keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
+			   key_serial_t base, char *hashname, char *otherinfo,
+			   size_t otherinfolen, char *buffer, size_t buflen)
+{
+	struct keyctl_dh_params params = { .priv = private,
+					   .prime = prime,
+					   .base = base };
+	struct keyctl_kdf_params kdfparams = { .hashname = hashname,
+					       .otherinfo = otherinfo,
+					       .otherinfolen = otherinfolen };
+
+	return keyctl(KEYCTL_DH_COMPUTE, &params, buffer, buflen, &kdfparams);
+}
+
 /*****************************************************************************/
 /*
  * fetch key description into an allocated buffer
diff --git a/keyutils.h b/keyutils.h
index a69fa7a..48a517c 100644
--- a/keyutils.h
+++ b/keyutils.h
@@ -108,6 +108,13 @@ struct keyctl_dh_params {
 	key_serial_t base;
 };
 
+struct keyctl_kdf_params {
+	char *hashname;
+	char *otherinfo;
+	uint32_t otherinfolen;
+	uint32_t __spare[8];
+};
+
 /*
  * syscall wrappers
  */
@@ -163,6 +170,10 @@ extern long keyctl_invalidate(key_serial_t id);
 extern long keyctl_get_persistent(uid_t uid, key_serial_t id);
 extern long keyctl_dh_compute(key_serial_t priv, key_serial_t prime,
 			      key_serial_t base, char *buffer, size_t buflen);
+extern long keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
+				  key_serial_t base, char *hashname,
+				  char *otherinfo, size_t otherinfolen,
+				  char *buffer, size_t buflen);
 
 /*
  * utilities
diff --git a/man/keyctl.1 b/man/keyctl.1
index 7060506..396d902 100644
--- a/man/keyctl.1
+++ b/man/keyctl.1
@@ -92,6 +92,10 @@ keyctl \- key management facility control
 \fBkeyctl\fR get_persistent <keyring> [<uid>]
 .br
 \fBkeyctl\fR dh_compute <private> <prime> <base>
+.br
+\fBkeyctl\fR dh_compute_kdf <private> <prime> <base> <output_length> <hash_type>
+.br
+\fBkeyctl\fR dh_compute_kdf_oi <private> <prime> <base> <output_length> <hash_type>
 .SH DESCRIPTION
 This program is used to control the key management facility in various ways
 using a variety of subcommands.
@@ -702,6 +706,28 @@ The result is printed to stdout as a hex dump.
 $ keyctl dh_compute $1 $2 $3
 8 bytes of data in result:
 00010203 04050607
+.SS Compute a Diffie-Hellman shared secret and derive key material
+\fBkeyctl\fR dh_compute_kdf <private> <prime> <base> <output_length> <hash_type>
+
+This command computes a Diffie-Hellman shared secret and derives
+key material from the shared secret using a key derivation function (KDF).
+The shared secret is derived as outlined above and is input to the KDF
+using the specified hash type. The hash type must point to a hash name
+known to the kernel crypto API.
+
+The operation derves key material of the length specified by the caller.
+
+The operation is compliant to the specification of SP800-56A.
+
+The result is printed to stdout as hex dump.
+.SS Compute a Diffie-Hellman shared secret and apply KDF with other input
+\fBkeyctl\fR dh_compute_kdf_oi <private> <prime> <base> <output_length> <hash_type>
+
+This command is identical to the command
+.IR dh_compute_kdf
+to generate a Diffie-Hellman shared secret followed by a key derivation
+operation. This command allows the caller to provide the other input data
+(OI data) compliant to SP800-56A via stdin.
 .fi
 .RE
 .SH ERRORS
diff --git a/man/keyctl_dh_compute.3 b/man/keyctl_dh_compute.3
index 3b04dba..837c481 100644
--- a/man/keyctl_dh_compute.3
+++ b/man/keyctl_dh_compute.3
@@ -11,6 +11,8 @@
 .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .SH NAME
 keyctl_dh_compute \- Compute a Diffie-Hellman shared secret or public key
+.br
+keyctl_dh_compute_kdf \- Derive key from a Diffie-Hellman shared secret
 .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .SH SYNOPSIS
 .nf
@@ -21,6 +23,10 @@ keyctl_dh_compute \- Compute a Diffie-Hellman shared secret or public key
 .sp
 .BI "long keyctl_dh_compute_alloc(key_serial_t " private,
 .BI "key_serial_t " prime ", key_serial_t " base ", void **" _buffer ");"
+.sp
+.BI "long keyctl_dh_compute_kdf(key_serial_t " private ", key_serial_t " prime ,
+.BI "key_serial_t " base ", char *" hashname ", char *" otherinfo ",
+.BI "size_t " otherinfolen ", char *" buffer ", size_t " buflen ");"
 .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .SH DESCRIPTION
 .BR keyctl_dh_compute ()
@@ -64,6 +70,48 @@ places the data in it.  If successful, a pointer to the buffer is placed in
 .IR *_buffer .
 The caller must free the buffer.
 .P
+.BR keyctl_dh_compute_kdf ()
+derives a key from a Diffie-Hellman shared secret according to the protocol
+specified in SP800-56A. The Diffie-Hellman computation is based on the same
+primitives as discussed
+for
+.BR keyctl_dh_compute ().
+.P
+To implement the protocol of SP800-56A
+.I base
+is a key containing the remote public key to compute the Diffie-Hellman
+shared secret. That shared secret is post-processed with a key derivation
+function.
+.P
+The
+.I hashname
+specifies the Linux kernel crypto API name for a hash that shall be used
+for the key derivation function, such as sha256.
+The
+.I hashname
+must be a NULL terminated string.
+.P
+Following the specification of SP800-56A section 5.8.1.2 the
+.I otherinfo
+parameter may be provided. The format of the OtherInfo field is defined
+by the caller. The caller may also specify NULL as a valid argument when
+no OtherInfo data shall be processed. The length of the
+.I otherinfo
+parameter is specified with
+.I otherinfolen
+and is restricted to a maximum length by the kernel.
+.P
+The KDF returns the requested number of bytes specified with the
+.I genlen
+or the
+.I buflen
+parameter depending on the invoked function.
+.P
+.I buffer
+and
+.I buflen
+specify the buffer into which the computed result will be placed.
+.P
 .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .SH RETURN VALUE
 On success
@@ -91,6 +139,15 @@ The buffer pointer is invalid or buflen is too small.
 .TP
 .B EOPNOTSUPP
 One of the keys was not a valid user key.
+.TP
+.B EMSGSIZE
+When using
+.BR keyctl_dh_compute_kdf (),
+the size of either
+.I otherinfolen
+or
+.I buflen
+is too big.
 .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .SH LINKING
 This is a library function that can be found in
diff --git a/tests/keyctl/dh_compute/valid/runtest.sh b/tests/keyctl/dh_compute/valid/runtest.sh
index f2aace6..6498eef 100644
--- a/tests/keyctl/dh_compute/valid/runtest.sh
+++ b/tests/keyctl/dh_compute/valid/runtest.sh
@@ -84,5 +84,173 @@ expect_multiline payload "$public"
 
 echo "++++ FINISHED TEST: $result" >>$OUTPUTFILE
 
+
+################################################################
+# Testing DH compute with KDF according to SP800-56A
+#
+# test vectors from http://csrc.nist.gov/groups/STM/cavp/documents/keymgmt/KASTestVectorsFFC2014.zip
+################################################################
+
+# SHA-256
+
+# XephemCAVS
+private="\x81\xb2\xc6\x5f\x5c\xba\xc0\x0b\x13\x53\xac\x38\xbd\x77\xa2\x5a"
+private+="\x86\x50\xed\x48\x5e\x41\x3e\xac\x1d\x6c\x48\x85"
+
+# P
+prime="\xa3\xcc\x62\x23\xe5\x0c\x6e\x3f\x7b\xb0\x58\x1d\xcb\x9e\x9f\xf0"
+prime+="\x2c\x58\x07\x68\x32\x8a\x15\x20\x7b\x1c\x32\x31\x7f\xb7\x84\x96"
+prime+="\x81\x5e\x3c\xf7\xf9\xd0\x9c\xcb\x9f\xa8\x40\xff\x47\x98\x51\x1a"
+prime+="\x17\xb5\x59\x28\x72\x1e\x5d\xfb\xcc\xc5\x41\x47\xe0\xf0\x5f\x85"
+prime+="\xb3\xac\x41\x0b\x6a\xe3\xf5\x9b\x79\x6f\x3f\xea\xc7\xfc\x52\x49"
+prime+="\x21\x7e\xb2\xa0\x45\x88\x29\x3a\x5a\xde\x22\x78\x79\xf4\x6c\xeb"
+prime+="\x56\x45\x7b\x5c\x43\x12\x93\xe5\xe1\x04\xd1\xb9\x64\xbd\x2c\xdf"
+prime+="\xde\xff\xa0\x40\x49\xa9\x1e\x67\xee\x8c\x86\xe9\x44\xf0\x4f\x94"
+prime+="\x4a\x30\xe3\x61\xf8\xd1\x5d\x17\xe5\x01\x0c\xab\xb4\xef\x40\xc0"
+prime+="\xeb\xa5\xf4\xa2\x52\xd4\xfd\x6c\xf9\xda\xe6\x0e\x86\xe4\xb3\x00"
+prime+="\x9b\x1d\xfc\x92\x66\x70\x35\x72\x61\x58\x7a\xd0\x5c\x00\xa6\xc6"
+prime+="\xf0\x10\x6c\xec\x8f\xc5\x91\x31\x51\x50\x84\xa8\x70\x59\x41\x65"
+prime+="\xb4\x93\x90\xdb\x2d\x00\xe7\x53\x8f\x23\x0d\x53\x2f\x4a\x4e\xca"
+prime+="\x83\x09\xd7\x07\xc0\xb3\x83\x5c\xee\x04\xf3\xca\x55\x8a\x22\xc6"
+prime+="\xb5\x20\xfe\x25\xde\x6f\xfa\x90\xef\xda\x49\x27\xd0\x18\x59\x4c"
+prime+="\x0c\x0b\x77\x06\x73\x93\xb7\xf1\xe0\xfc\x7c\xf2\x16\xaf\xf3\x9f"
+
+# YephemIUT
+xa="\x9a\x70\x82\x2d\x3f\x06\x12\x3d\x0e\x51\x8e\xe1\x16\x51\xe5\xf6"
+xa+="\xb1\x19\xdc\x3b\x97\xd5\xb1\xc0\xa2\xa6\xf6\xde\x94\x25\x64\xba"
+xa+="\x10\x06\x1e\xec\xde\xb7\x36\x9c\xa5\x37\x49\x9e\x04\xb0\x36\xe9"
+xa+="\x7f\x44\x5a\x95\x6f\x63\x69\xae\x6e\x63\xfd\x27\xea\xe3\xe3\x47"
+xa+="\x85\x54\x47\xd3\xba\xc1\xc6\x0c\x10\xe7\x35\x07\x72\xc6\xc0\xc6"
+xa+="\xfb\xf9\xca\x3e\x38\xf0\xe8\x65\x88\x25\xd3\xb2\x0f\x1f\x02\x8f"
+xa+="\x35\xe3\x4d\x12\x35\x10\x3d\xf2\x33\x9b\x5b\x09\x9d\x3f\xe3\xe5"
+xa+="\x34\x6a\x69\x16\x42\xba\xc5\xb0\xbb\x03\xcd\x5d\x04\xd7\x56\x26"
+xa+="\x21\x49\x3f\xf1\xc4\x27\x3b\x6a\x45\xc5\xec\xb0\xb5\xe9\x08\xa0"
+xa+="\xf9\xf5\x62\x28\x2e\x85\x3e\xfc\x9a\x7e\xa1\x12\xe9\x47\x4f\xf6"
+xa+="\x94\x18\xf7\xc4\x7a\xe9\x66\xd4\x52\x4c\xa1\x70\x1b\x60\xa4\xbe"
+xa+="\x15\xc7\x5e\x27\xb4\x05\x80\x64\x68\x15\x6e\x02\xcb\xc5\x8f\xf4"
+xa+="\x66\x3c\x96\xac\x0c\x87\x36\x81\x35\xfa\x9b\x0b\xb6\x33\x7a\xe2"
+xa+="\x58\x52\x1d\x7d\x60\xc2\xa9\x1b\x4e\xd7\x72\xad\x65\x03\x40\x49"
+xa+="\x97\xf6\x79\x9d\xf6\x63\xa8\x99\x9c\xfd\x74\x7f\xa0\x67\xb9\x05"
+xa+="\x8a\xb3\x3b\xc1\x45\x94\x36\x6f\x28\xf5\xa2\xd9\x00\xb6\x46\x7a"
+
+# Z
+read -d '' shared <<"EOF"
+0fdbd9a2 ebf50cba 489b4e4d 7cd6924a 42ee6324 a26988b2 22bc38e6 9cc445f1
+eb47c1a4 62eca39f 39bcd7b8 19dede51 30bc38da ec99c16f 40a4e5c1 9c97b796
+8b41823d a0650e37 13c73e6f 5f2a9dff 2e67dbf5 40ee66f4 e694c28f ba1d604b
+71b57b8a eeb67a35 ba425a38 490b6fb9 f713db22 6f893b7a 8962f426 ba3046fb
+cff8538c 16f583e8 ae947672 0ba55ff9 75b440d0 c4565cc7 5837d23a fea61a39
+e0b7f6c4 e24c2154 7eb19fce f8dbed10 b06a9cce 971c0f0f ba7c1d5c b5035eaa
+4fddd3ba fe757339 e3321e3e 4ebfe9e7 9c6c0401 4df63cf9 28d0a2c0 5b2d5521
+030c35f1 c84c97fe 64cad509 8012a003 d52d24c4 1a1f9348 b7575251 3facb02f
+EOF
+
+# OI
+otherinfo="\xa1\xb2\xc3\xd4\xe5\x43\x41\x56\x53\x69\x64\x0d\x64\xc1\xb2"
+otherinfo+="\x33\x61\xb2\x61\xde\x78\x68\x8e\xa8\x65\xfc\xff\x11\x3c\x84"
+
+# DKM
+read -d '' derived <<"EOF"
+8284e313 02c8a26b 393ec52d 9f9e0882
+EOF
+
+pcreate_key "-e $prime" user dh:prime @s
+expect_keyid primeid
+
+pcreate_key "-e $xa" user dh:xa @s
+expect_keyid xaid
+
+pcreate_key "-e $private" user dh:private @s
+expect_keyid privateid
+
+marker "COMPUTE DH SHARED SECRET"
+dh_compute $privateid $primeid $xaid
+expect_multiline payload "$shared"
+
+marker "COMPUTE DERIVED KEY FROM DH SHARED SECRET (SHA-256)"
+echo -e -n $otherinfo | dh_compute_kdf_oi $privateid $primeid $xaid 16 "sha256"
+expect_multiline payload "$derived"
+
+
+# SHA-224
+
+# XephemCAVS
+private="\x86\x1b\xa2\x59\xab\xa6\xaa\x57\x7d\xe2\x2f\x50\x8e\xcb\xbc\x26"
+private+="\xc5\xac\xfc\xcb\x9e\xa2\x3b\x43\x4d\x6d\x2b\x79"
+
+# P
+prime="\xa5\xb1\x76\x4e\x13\xc8\x16\x99\xab\xa3\x8f\x0d\xc0\xd1\x5e\x15"
+prime+="\xf5\x0f\xcd\x5c\xf7\xc2\x23\x72\xca\xfc\x5e\xd7\x62\x94\x1b\xd9"
+prime+="\xe0\xfb\x9a\xab\xee\x74\x66\xd2\xc8\x29\xaa\xb0\x31\xdb\x7b\x1b"
+prime+="\x5a\x64\xe6\x8e\xd5\x3b\xaf\xb2\x83\xba\x0f\x01\x8b\xeb\x3e\xdc"
+prime+="\x95\x7f\xe4\x53\xbe\x0d\xaa\xb6\x1b\x32\x28\x76\x3e\x80\x75\x8c"
+prime+="\x6d\x8c\x28\x3c\xf6\x30\xed\xd9\xd7\x0a\x8a\xf3\x30\xdd\x0a\xf6"
+prime+="\xa8\xd5\x94\xc2\x3c\xdd\x24\xc8\xad\x3f\xcf\xea\x41\x75\x77\x72"
+prime+="\xce\xed\x92\x1e\x63\x86\x2f\x24\x6e\x6f\x49\xd8\x74\x7e\x44\xae"
+prime+="\xf0\x1e\x30\x9b\x6d\xcc\x80\xd4\x50\x38\x3b\xb1\xf9\x4d\xd5\x90"
+prime+="\x84\xf8\xe9\x6f\x85\x6e\xc7\xc8\x33\x5e\xdb\x05\x5f\x8e\xc6\xc4"
+prime+="\x81\x52\x0b\x3f\x28\xe8\x0b\x62\x09\xb8\xae\x61\xcc\x86\x0e\x24"
+prime+="\xc8\x22\xb6\x6c\x4f\x97\x80\x49\x93\xbc\xd0\xa9\x72\xb3\x53\x54"
+prime+="\x01\x33\x0e\xbe\x4b\x2e\x92\x3f\x18\x9b\x63\x35\x62\xe4\x68\xeb"
+prime+="\x99\xa4\xbc\x88\xcc\xbf\xf8\xdf\x0f\xd5\xaf\xcf\xe6\xae\x19\x18"
+prime+="\x42\x14\xab\x3f\xef\xb7\xf0\x66\x8b\x8b\x26\x83\xbe\xbd\x56\x51"
+prime+="\xa4\xc6\x38\x43\xb9\xb1\x4b\xc7\x38\xd5\x20\xb1\xb7\x21\x2c\x69"
+
+# YephemIUT
+xa="\x17\xd7\x1a\xf4\x35\x3c\x22\x12\x2a\xeb\x2a\x06\x19\xcc\x2c\xf7"
+xa+="\x35\x53\xf2\x8e\x9f\xb1\x91\xfd\xb2\x86\xb1\x15\xb9\xfd\xa8\x66"
+xa+="\x2d\xe5\x17\x3b\x1a\xff\x70\x48\x8d\x9b\xc8\x48\xe5\x37\xd7\xe5"
+xa+="\x02\x16\x49\xd3\x7d\xc7\x8c\x94\x36\x9d\xb9\x0c\x27\x84\xc9\x4d"
+xa+="\x97\x0a\xc9\xb5\xe3\x5e\xfd\x22\xd4\x18\xd3\x1b\x68\xd9\x55\x0b"
+xa+="\xaa\x77\x16\xe9\x8e\xa6\x78\x3b\xb3\xa8\x45\x05\x9f\xba\xa4\xa6"
+xa+="\x72\x0a\x6a\x23\xc5\x6b\xa5\x2b\x4d\x9b\x72\x6e\x00\x68\xe9\xeb"
+xa+="\x4d\x17\x5b\xff\x43\x69\xf3\xd2\xa4\xaf\x66\xee\xcd\x62\xef\x7b"
+xa+="\x23\xc3\x37\xd4\x70\x95\x2b\x17\x67\xc8\xbf\x78\x2f\x0b\x58\xb4"
+xa+="\xfc\x82\x45\xf8\x40\x78\x71\x70\xf4\xb0\xa5\x1b\x5e\xb4\x60\x75"
+xa+="\x8a\xdd\xc9\xf4\x4a\x73\xa3\xf6\x07\x60\x3b\xd3\x50\x73\xd1\xa6"
+xa+="\x9a\x20\x3a\x04\x94\xa8\xc2\x02\x1b\xa0\xda\x1f\x04\x95\xf5\x60"
+xa+="\xc0\xba\x81\x79\x4e\xee\xeb\x82\x5d\x1b\xd3\x43\x16\xa5\x2a\xe1"
+xa+="\xc9\x00\x10\x0c\x0d\x6f\xa0\x25\x46\xed\x7a\x9c\x38\xa6\xa3\x43"
+xa+="\xd6\x86\x59\xee\xb5\x9c\xf3\x81\x04\xa9\x6b\xb2\x5a\x6d\xbb\xf0"
+xa+="\xcb\xc0\xed\xe7\x3a\x7b\xba\x67\x51\x81\xe0\xcd\x2e\x7b\x9f\x89"
+
+# Z
+read -d '' shared <<"EOF"
+057c22b8 c5872fef 08ebe852 fafab4b7 c2c2ffbb 376d71bd a941b16e 32614adf
+ebb82aeb d50f29d3 cec63d10 77f50e21 cf381b87 a818c614 52c5cce2 af85f40c
+06615b97 fe8c3a80 68990ac5 83957b52 8dd6d52d a3b51e84 aec355fd 4a3fe5ce
+faa3b17c 9e71cb4d 28ecab6d 21297280 e52397b7 ccb1b62d 8d5d3ce4 1d26b2a3
+bdbf880b b39e8b02 8a745ff2 9f0984da efe97084 5d850884 525403ca d2a52956
+f55b9a89 b2d801f1 710333c0 479c5955 b54c8163 83c65ad9 c78b8c67 cc1b211b
+208b9fab b9c99a68 18293e6a 8da069e6 75eb4317 668a7d4b 6f235533 f3ff4ed0
+4f8ad579 f9ad14e7 f68ae183 41d603d9 d6297123 00716c98 bbbf16eb 2a2cc92f
+EOF
+
+# OI
+otherinfo="\xa1\xb2\xc3\xd4\xe5\x43\x41\x56\x53\x69\x64\xaa\x27\xe2\x49"
+otherinfo+="\xbf\x0a\x12\x76\x46\x8d\x80\x82\x59\xf3\xb8\xe2\x68\x78\x51"
+
+# DKM
+read -d '' derived <<"EOF"
+88bf39c0 08eec33a dc3b4430 054ba262
+EOF
+
+pcreate_key "-e $prime" user dh:prime @s
+expect_keyid primeid
+
+pcreate_key "-e $xa" user dh:xa @s
+expect_keyid xaid
+
+pcreate_key "-e $private" user dh:private @s
+expect_keyid privateid
+
+marker "COMPUTE DH SHARED SECRET"
+dh_compute $privateid $primeid $xaid
+expect_multiline payload "$shared"
+
+marker "COMPUTE DERIVED KEY FROM DH SHARED SECRET (SHA-224)"
+echo -e -n $otherinfo | dh_compute_kdf_oi $privateid $primeid $xaid 16 "sha224"
+expect_multiline payload "$derived"
+
 # --- then report the results in the database ---
 toolbox_report_result $TEST $result
diff --git a/tests/toolbox.inc.sh b/tests/toolbox.inc.sh
index 7f19a02..27b253f 100644
--- a/tests/toolbox.inc.sh
+++ b/tests/toolbox.inc.sh
@@ -1106,6 +1106,50 @@ function dh_compute ()
 
 ###############################################################################
 #
+# Do a DH computation post-processed by a KDF
+#
+###############################################################################
+function dh_compute_kdf ()
+{
+    my_exitval=0
+    if [ "x$1" = "x--fail" ]
+    then
+	my_exitval=1
+	shift
+    fi
+
+    echo keyctl dh_compute_kdf $@ >>$OUTPUTFILE
+    keyctl dh_compute_kdf $@ >>$OUTPUTFILE 2>&1
+    if [ $? != $my_exitval ]
+    then
+	failed
+    fi
+}
+
+###############################################################################
+#
+# Do a DH computation post-processed by a KDF with other information
+#
+###############################################################################
+function dh_compute_kdf_oi ()
+{
+    my_exitval=0
+    if [ "x$1" = "x--fail" ]
+    then
+	my_exitval=1
+	shift
+    fi
+
+    echo keyctl dh_compute_kdf_oi $@ >>$OUTPUTFILE
+    keyctl dh_compute_kdf_oi $@ >>$OUTPUTFILE 2>&1
+    if [ $? != $my_exitval ]
+    then
+	failed
+    fi
+}
+
+###############################################################################
+#
 # Make sure we sleep at least N seconds
 #
 ###############################################################################
diff --git a/version.lds b/version.lds
index 2bfed13..b8eebfb 100644
--- a/version.lds
+++ b/version.lds
@@ -66,5 +66,7 @@ KEYUTILS_1.6 {
 	/* management functions */
 	keyctl_dh_compute;
 	keyctl_dh_compute_alloc;
+	keyctl_dh_compute_kdf;
+	keyctl_dh_compute_kdf_alloc;
 
 } KEYUTILS_1.5;
-- 
2.9.3

^ permalink raw reply related

* Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms
From: Michael Ellerman @ 2017-04-04  5:03 UTC (permalink / raw)
  To: Horia Geantă, Herbert Xu, Scott Wood, Roy Pledge
  Cc: Claudiu Manoil, Cristian Stoica, Dan Douglass, linux-arm-kernel,
	Vakul Garg, linuxppc-dev, David S. Miller, Alexandru Porosanu,
	linux-crypto
In-Reply-To: <20170317100602.2837-8-horia.geanta@nxp.com>

Horia Geantă <horia.geanta@nxp.com> writes:

> Add support to submit ablkcipher and authenc algorithms
> via the QI backend:
> -ablkcipher:
> cbc({aes,des,des3_ede})
> ctr(aes), rfc3686(ctr(aes))
> xts(aes)
> -authenc:
> authenc(hmac(md5),cbc({aes,des,des3_ede}))
> authenc(hmac(sha*),cbc({aes,des,des3_ede}))
>
> caam/qi being a new driver, let's wait some time to settle down without
> interfering with existing caam/jr driver.
> Accordingly, for now all caam/qi algorithms (caamalg_qi module) are
> marked to be of lower priority than caam/jr ones (caamalg module).
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> Signed-off-by: Alex Porosanu <alexandru.porosanu@nxp.com>
> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
> ---
>  drivers/crypto/caam/Kconfig        |   20 +-
>  drivers/crypto/caam/Makefile       |    1 +
>  drivers/crypto/caam/caamalg.c      |    9 +-
>  drivers/crypto/caam/caamalg_desc.c |   77 +-
>  drivers/crypto/caam/caamalg_desc.h |   15 +-
>  drivers/crypto/caam/caamalg_qi.c   | 2387 ++++++++++++++++++++++++++++++++++++
>  drivers/crypto/caam/sg_sw_qm.h     |  108 ++
>  7 files changed, 2601 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/crypto/caam/caamalg_qi.c
>  create mode 100644 drivers/crypto/caam/sg_sw_qm.h


This appears to be blowing up my Freescale (NXP) P5020DS board:

  Unable to handle kernel paging request for data at address 0x00000020
  Faulting instruction address: 0xc0000000004393e4
  Oops: Kernel access of bad area, sig: 11 [#1]
  SMP NR_CPUS=24 
  CoreNet Generic
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789 #5
  task: c0000000f70c0000 task.stack: c0000000f70c8000
  NIP: c0000000004393e4 LR: c0000000004aeba0 CTR: c0000000004fa7d8
  REGS: c0000000f70cb160 TRAP: 0300   Not tainted  (4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789)
  MSR: 0000000080029000 <CE,EE,ME>
    CR: 24adbe48  XER: 20000000
  DEAR: 0000000000000020 ESR: 0000000000000000 SOFTE: 1 
  GPR00: c0000000006feba0 c0000000f70cb3e0 c000000000e60000 0000000000000000 
  GPR04: 0000000000000001 0000000000000000 c000000000e0b290 0000000000000003 
  GPR08: 0000000000000004 c000000000ea5280 0000000000000004 0000000000000004 
  GPR12: 0000000088adbe22 c00000003fff5000 c000000000ba3518 8000080088090fa8 
  GPR16: 0000000000001000 c000000000ba3500 c0000000f72c68d8 0000000000000004 
  GPR20: c000000000ea5280 c000000000ba34e8 0000000000000020 0000000000000004 
  GPR24: c000000000eab7c0 0000000000000000 c0000000f7fc8818 c000000000eb0000 
  GPR28: c0000000f786cc00 c000000000eab780 fffffffff786cc00 c000000000eab7c0 
  NIP [c0000000004393e4] .gen_pool_alloc+0x0/0xc
  LR [c0000000004aeba0] .qman_alloc_cgrid_range+0x24/0x54
  Call Trace:
  [c0000000f70cb3e0] [c000000000504054] .platform_device_register_full+0x12c/0x150 (unreliable)
  [c0000000f70cb460] [c0000000006feba0] .caam_qi_init+0x158/0x63c
  [c0000000f70cb5f0] [c0000000006fc64c] .caam_probe+0x8b8/0x1830
  [c0000000f70cb740] [c000000000503288] .platform_drv_probe+0x60/0xac
  [c0000000f70cb7c0] [c000000000501194] .driver_probe_device+0x248/0x344
  [c0000000f70cb870] [c0000000005013b4] .__driver_attach+0x124/0x128
  [c0000000f70cb900] [c0000000004fed90] .bus_for_each_dev+0x80/0xcc
  [c0000000f70cb9a0] [c000000000500858] .driver_attach+0x24/0x38
  [c0000000f70cba10] [c00000000050043c] .bus_add_driver+0x14c/0x29c
  [c0000000f70cbab0] [c000000000501d64] .driver_register+0x8c/0x154
  [c0000000f70cbb30] [c000000000503000] .__platform_driver_register+0x48/0x5c
  [c0000000f70cbba0] [c000000000c7f798] .caam_driver_init+0x1c/0x30
  [c0000000f70cbc10] [c000000000001904] .do_one_initcall+0x60/0x1a8
  [c0000000f70cbcf0] [c000000000c35f30] .kernel_init_freeable+0x248/0x334
  [c0000000f70cbdb0] [c0000000000020fc] .kernel_init+0x1c/0xf20
  [c0000000f70cbe30] [c0000000000009bc] .ret_from_kernel_thread+0x58/0x9c
  Instruction dump:
  eb61ffd8 eb81ffe0 eba1ffe8 ebc1fff0 ebe1fff8 4e800020 38600000 4bffffb0 
  7ce53b78 4bffff0c 7f67db78 4bffff24 <e8a30020> e8c30028 4bfffd30 fbe1fff8 
  ---[ end trace 9f61087068991b02 ]---


home:linux-next(4)(I)> git bisect log
...
git bisect bad b189817cf7894e03fd3700acd923221d3007259e
# first bad commit: [b189817cf7894e03fd3700acd923221d3007259e] crypto: caam/qi - add ablkcipher and authenc algorithms


The oops is saying gen_pool_alloc() was called with a NULL pointer, so
it seems qm_cgralloc is NULL:

static int qman_alloc_range(struct gen_pool *p, u32 *result, u32 cnt)
{
	unsigned long addr;

	addr = gen_pool_alloc(p, cnt);
	...

int qman_alloc_cgrid_range(u32 *result, u32 count)
{
	return qman_alloc_range(qm_cgralloc, result, count);
}


I didn't pull the thread any further than that.

cheers

^ permalink raw reply

* Re: [PATCH v6] DH support: add KDF handling support
From: David Howells @ 2017-04-04  7:45 UTC (permalink / raw)
  To: Stephan Müller
  Cc: dhowells, mathew.j.martineau, herbert, linux-crypto, keyrings
In-Reply-To: <1600325.7sEBTCiKyY@positron.chronox.de>

Pulled.

^ permalink raw reply

* What should be the algo priority
From: Harsh Jain @ 2017-04-04  7:53 UTC (permalink / raw)
  To: linux-crypto

Hi,

Do we have any guidelines documented to decide what should be the
algorithm priority. Specially for authenc implementation.Most of the
drivers have fixed priority for all algos. Problem comes in when we
have cbc(aes), hmac(sha1) and authenc(cbc(aes),hmac(sha1))
implementation in driver. Base authenc driver gets more precedence
because of higher priority(enc->base.cra_priority * 10 +
auth_base->cra_priority;)

What should be the priority of
cbc(aes),
hmac(sha1)
authenc(cbc(aes),hmac(sha1))


Regards
Harsh Jain

^ permalink raw reply

* Re: [PATCH 4/5] crypto/nx: Add P9 NX support for 842 compression engine.
From: Michael Ellerman @ 2017-04-04 10:55 UTC (permalink / raw)
  To: Haren Myneni, herbert, ddstreet
  Cc: linuxppc-dev, linux-crypto, benh, mikey, suka, hbabu
In-Reply-To: <1491066107.29552.29.camel@hbabu-laptop>

Hi Haren,

A few comments ...

Haren Myneni <haren@linux.vnet.ibm.com> writes:

> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 4e5a470..7315621 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -19,6 +19,8 @@
>  #define VAS_RX_FIFO_SIZE_MIN	(1 << 10)	/* 1KB */
>  #define VAS_RX_FIFO_SIZE_MAX	(8 << 20)	/* 8MB */
>  
> +#define is_vas_available()	(cpu_has_feature(CPU_FTR_ARCH_300))

You shouldn't need that, it should all come from the device tree.

> diff --git a/drivers/crypto/nx/Kconfig b/drivers/crypto/nx/Kconfig
> index ad7552a..4ad7fdb 100644
> --- a/drivers/crypto/nx/Kconfig
> +++ b/drivers/crypto/nx/Kconfig
> @@ -38,6 +38,7 @@ config CRYPTO_DEV_NX_COMPRESS_PSERIES
>  config CRYPTO_DEV_NX_COMPRESS_POWERNV
>  	tristate "Compression acceleration support on PowerNV platform"
>  	depends on PPC_POWERNV
> +	select VAS

Don't select symbols that are user visible. 

I'm not sure we actually want CONFIG_VAS to be user visible, but
currently it is so this should be 'depends on VAS'.

> diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
> index 8737e90..66efd39 100644
> --- a/drivers/crypto/nx/nx-842-powernv.c
> +++ b/drivers/crypto/nx/nx-842-powernv.c
> @@ -554,6 +662,164 @@ static int nx842_powernv_decompress(const unsigned char *in, unsigned int inlen,
>  				      wmem, CCW_FC_842_DECOMP_CRC);
>  }
>  
> +
> +static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
> +					int vasid, int ct)
> +{
> +	struct vas_window *rxwin, *txwin = NULL;
> +	struct vas_rx_win_attr rxattr;
> +	struct vas_tx_win_attr txattr;
> +	struct nx842_coproc *coproc;
> +	u32 lpid, pid, tid;
> +	u64 rx_fifo;
> +	int ret;
> +#define RX_FIFO_SIZE 0x8000

Where's that come from?

> +	if (of_property_read_u64(dn, "rx-fifo-address", (void *)&rx_fifo)) {
> +		pr_err("ibm,nx-842: Missing rx-fifo-address property\n");

The driver already declares pr_fmt(), so do you need the prefixes on
these pr_err()s ?

> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(dn, "lpid", &lpid)) {
> +		pr_err("ibm,nx-842: Missing lpid property\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(dn, "pid", &pid)) {
> +		pr_err("ibm,nx-842: Missing pid property\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(dn, "tid", &tid)) {
> +		pr_err("ibm,nx-842: Missing tid property\n");
> +		return -EINVAL;
> +	}
> +
> +	vas_init_rx_win_attr(&rxattr, ct);
> +	rxattr.rx_fifo = (void *)rx_fifo;
> +	rxattr.rx_fifo_size = RX_FIFO_SIZE;
> +	rxattr.lnotify_lpid = lpid;
> +	rxattr.lnotify_pid = pid;
> +	rxattr.lnotify_tid = tid;
> +	rxattr.wcreds_max = 64;
> +
> +	/*
> +	 * Open a VAS receice window which is used to configure RxFIFO
> +	 * for NX.
> +	 */
> +	rxwin = vas_rx_win_open(vasid, ct, &rxattr);
> +	if (IS_ERR(rxwin)) {
> +		pr_err("ibm,nx-842: setting RxFIFO with VAS failed: %ld\n",
> +			PTR_ERR(rxwin));
> +		return PTR_ERR(rxwin);
> +	}
> +
> +	/*
> +	 * Kernel requests will be high priority. So open send
> +	 * windows only for high priority RxFIFO entries.
> +	 */
> +	if (ct == VAS_COP_TYPE_842_HIPRI) {

This if body looks like it should be a separate function to me.

> +		vas_init_tx_win_attr(&txattr, ct);
> +		txattr.lpid = 0;	/* lpid is 0 for kernel requests */
> +		txattr.pid = mfspr(SPRN_PID);
> +
> +		/*
> +		 * Open a VAS send window which is used to send request to NX.
> +		 */
> +		txwin = vas_tx_win_open(vasid, ct, &txattr);
> +		if (IS_ERR(txwin)) {
> +			pr_err("ibm,nx-842: Can not open TX window: %ld\n",
> +				PTR_ERR(txwin));
> +			ret = PTR_ERR(txwin);
> +			goto err_out;
> +		}
> +	}
> +
> +	coproc = kmalloc(sizeof(*coproc), GFP_KERNEL);
> +	if (!coproc) {
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}

The error handling would be simpler if you did that earlier, before you
open the RX/TX windows.

> +	coproc->chip_id = chip_id;
> +	coproc->vas.rxwin = rxwin;
> +	coproc->vas.txwin = txwin;
> +
> +	INIT_LIST_HEAD(&coproc->list);
> +	list_add(&coproc->list, &nx842_coprocs);

That duplicates logic in the non-vas probe, so ideally would be shared
or in a helper.

> +
> +	return 0;
> +
> +err_out:
> +	if (txwin)
> +		vas_win_close(txwin);
> +
> +	vas_win_close(rxwin);
> +
> +	return ret;
> +}
> +
> +
> +static int __init nx842_powernv_probe_vas(struct device_node *dn)
> +{
> +	struct device_node *nxdn, *np;

There's too many device nodes in this function.

> +	int chip_id, vasid, rc;
> +
> +	chip_id = of_get_ibm_chip_id(dn);
> +	if (chip_id < 0) {
> +		pr_err("ibm,chip-id missing\n");
> +		return -EINVAL;
> +	}
> +
> +	np = of_find_node_by_name(dn, "vas");

You should always search by compatible when possible. I don't see why
you wouldn't here.


> +	if (!np) {
> +		pr_err("ibm,xscom: Missing VAS device node\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(np, "vas-id", &vasid)) {
> +		pr_err("ibm,vas: Missing vas-id device property\n");
> +		of_node_put(np);
> +		return -EINVAL;
> +	}
> +
> +	of_node_put(np);
> +
> +	nxdn = of_find_compatible_node(dn, NULL, "ibm,power-nx");

What are you trying to do here?

This will find any node in the device tree that is compatible with
"ibm,power-nx". It will start searching after dn in the device tree. But
it doesn't search the children of dn necessarily, is that what you're
trying to do?

> +	if (!nxdn) {
> +		pr_err("ibm,xscom: Missing NX device node\n");
> +		return -EINVAL;
> +	}
> +
> +	np = of_find_node_by_name(nxdn, "ibm,nx-842-high");

Search by name again.

> +	if (!np) {
> +		pr_err("ibm,nx-842-high device node is missing\n");
> +		rc = -EINVAL;
> +		goto out_nd_put;
> +	}
> +
> +	rc = vas_cfg_coproc_info(np, chip_id, vasid, VAS_COP_TYPE_842_HIPRI);
> +	of_node_put(np);
> +	if (rc)
> +		goto out_nd_put;
> +
> +	np = of_find_node_by_name(nxdn, "ibm,nx-842-normal");

Search by name again.

Normal vs high should not be encoded in the name, it should be a
property of the node.

> +	if (!np) {
> +		pr_err("ibm,nx-842-normal device node is missing\n");
> +		rc = -EINVAL;
> +		goto out_nd_put;
> +	}
> +
> +	rc = vas_cfg_coproc_info(np, chip_id, vasid, VAS_COP_TYPE_842);
> +	of_node_put(np);
> +	if (!rc)
> +		return 0;
> +
> +out_nd_put:
> +	of_node_put(nxdn);
> +	return rc;
> +}
> +
>  static int __init nx842_powernv_probe(struct device_node *dn)
>  {
>  	struct nx842_coproc *coproc;
> @@ -602,11 +868,42 @@ static void nx842_delete_coproc(void)
>  	struct nx842_coproc *coproc, *n;
>  
>  	list_for_each_entry_safe(coproc, n, &nx842_coprocs, list) {
> +		if (is_vas_available()) {

That should just be a check of coproc->vas.rxwin != NULL or similar.

> +			vas_win_close(coproc->vas.rxwin);
> +			/*
> +			 * txwin opened only for high priority RxFIFOs
> +			 */
> +			if (coproc->vas.txwin)
> +				vas_win_close(coproc->vas.txwin);
> +		}

That should be pulled out into a helper, not in the middle of the loop
here.

>  		list_del(&coproc->list);
>  		kfree(coproc);
>  	}
>  }


cheers

^ permalink raw reply

* Re: [PATCH 1/5] crypto/nx: Rename nx842_powernv_function as icswx function
From: Michael Ellerman @ 2017-04-04 11:11 UTC (permalink / raw)
  To: Haren Myneni, herbert, ddstreet; +Cc: mikey, suka, linux-crypto, linuxppc-dev
In-Reply-To: <1491065941.29552.26.camel@hbabu-laptop>

Haren Myneni <haren@linux.vnet.ibm.com> writes:

> [PATCH 1/5] crypto/nx: Rename nx842_powernv_function as icswx function
>
> nx842_powernv_function is points to nx842_icswx_function and
> will be point to VAS function which will be added later for
> P9 NX support.

I know it's nit-picking but can you give it a better name while you're
there.

I was thinking it should be called "send" or something, but it actually
synchronously sends and waits for a request.

So perhaps just nx842_exec(), for "execute a request", and then you can
have nx842_exec_icswx() and nx842_exec_vas().

cheers

^ permalink raw reply

* Re: [PATCH 2/5] crypto/nx: Create nx842_cfg_crb function
From: Michael Ellerman @ 2017-04-04 11:12 UTC (permalink / raw)
  To: Haren Myneni, herbert, ddstreet
  Cc: linuxppc-dev, linux-crypto, benh, mikey, suka, hbabu
In-Reply-To: <1491066000.29552.27.camel@hbabu-laptop>

Haren Myneni <haren@linux.vnet.ibm.com> writes:

> [PATCH 2/5] crypto/nx: Create nx842_cfg_crb function
>
> Configure CRB is moved to nx842_cfg_crb() so that it can be
> used for icswx function and VAS function which will be added
> later.

Buy a vowel! :)

nx842_configure_crb() is fine.

cheers

^ permalink raw reply

* Re: [PATCH 3/5] crypto/nx: Create nx842_delete_coproc function
From: Michael Ellerman @ 2017-04-04 11:13 UTC (permalink / raw)
  To: Haren Myneni, herbert, ddstreet; +Cc: mikey, suka, linux-crypto, linuxppc-dev
In-Reply-To: <1491066046.29552.28.camel@hbabu-laptop>

Haren Myneni <haren@linux.vnet.ibm.com> writes:

> [PATCH 3/5] crypto/nx: Create nx842_delete_coproc function
>
> Move deleting coprocessor info upon exit or failure to
> nx842_delete_coproc().

Naming again, this deletes *all* the coprocs, so the name should be
plural.

cheers

^ permalink raw reply

* Re: [PATCH 5/5] crypto/nx: Add P9 NX specific error codes for 842 engine
From: Michael Ellerman @ 2017-04-04 11:15 UTC (permalink / raw)
  To: Haren Myneni, herbert, ddstreet; +Cc: mikey, suka, linux-crypto, linuxppc-dev
In-Reply-To: <1491066154.29552.30.camel@hbabu-laptop>

Haren Myneni <haren@linux.vnet.ibm.com> writes:

> [PATCH 5/5] crypto/nx: Add P9 NX specific error codes for 842 engine
>
> This patch adds changes for checking P9 specific 842 engine
> error codes. These errros are reported in co-processor status
> block (CSB) for failures.

But you just enabled support on P9 in patch 4.

So you should reorder patch 4 and 5. ie. add the P9 error handling
before you enable P9 support.

cheers

^ permalink raw reply

* Re: [PATCH] padata: avoid race in reordering
From: Jason A. Donenfeld @ 2017-04-04 11:53 UTC (permalink / raw)
  To: stable; +Cc: Steffen Klassert, Linux Crypto Mailing List, LKML, Herbert Xu
In-Reply-To: <20170324141608.GA15370@gondor.apana.org.au>

Herbert applied this to his tree. It's probably a good stable
candidate, since it's a two line change to fix a race condition.

On Fri, Mar 24, 2017 at 3:16 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> Under extremely heavy uses of padata, crashes occur, and with list
>> debugging turned on, this happens instead:
>>
>> [87487.298728] WARNING: CPU: 1 PID: 882 at lib/list_debug.c:33
>> __list_add+0xae/0x130
>> [87487.301868] list_add corruption. prev->next should be next
>> (ffffb17abfc043d0), but was ffff8dba70872c80. (prev=ffff8dba70872b00).
>> [87487.339011]  [<ffffffff9a53d075>] dump_stack+0x68/0xa3
>> [87487.342198]  [<ffffffff99e119a1>] ? console_unlock+0x281/0x6d0
>> [87487.345364]  [<ffffffff99d6b91f>] __warn+0xff/0x140
>> [87487.348513]  [<ffffffff99d6b9aa>] warn_slowpath_fmt+0x4a/0x50
>> [87487.351659]  [<ffffffff9a58b5de>] __list_add+0xae/0x130
>> [87487.354772]  [<ffffffff9add5094>] ? _raw_spin_lock+0x64/0x70
>> [87487.357915]  [<ffffffff99eefd66>] padata_reorder+0x1e6/0x420
>> [87487.361084]  [<ffffffff99ef0055>] padata_do_serial+0xa5/0x120
>>
>> padata_reorder calls list_add_tail with the list to which its adding
>> locked, which seems correct:
>>
>> spin_lock(&squeue->serial.lock);
>> list_add_tail(&padata->list, &squeue->serial.list);
>> spin_unlock(&squeue->serial.lock);
>>
>> This therefore leaves only place where such inconsistency could occur:
>> if padata->list is added at the same time on two different threads.
>> This pdata pointer comes from the function call to
>> padata_get_next(pd), which has in it the following block:
>>
>> next_queue = per_cpu_ptr(pd->pqueue, cpu);
>> padata = NULL;
>> reorder = &next_queue->reorder;
>> if (!list_empty(&reorder->list)) {
>>       padata = list_entry(reorder->list.next,
>>                           struct padata_priv, list);
>>       spin_lock(&reorder->lock);
>>       list_del_init(&padata->list);
>>       atomic_dec(&pd->reorder_objects);
>>       spin_unlock(&reorder->lock);
>>
>>       pd->processed++;
>>
>>       goto out;
>> }
>> out:
>> return padata;
>>
>> I strongly suspect that the problem here is that two threads can race
>> on reorder list. Even though the deletion is locked, call to
>> list_entry is not locked, which means it's feasible that two threads
>> pick up the same padata object and subsequently call list_add_tail on
>> them at the same time. The fix is thus be hoist that lock outside of
>> that block.
>>
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
> Patch applied.  Thanks.
> --
> 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: What should be the algo priority
From: Stephan Müller @ 2017-04-04 12:37 UTC (permalink / raw)
  To: Harsh Jain; +Cc: linux-crypto
In-Reply-To: <CAFXBA=kTywuuZOKw6a7mj+aXtn0YcOYF=jmesuBE2L3zZmojMw@mail.gmail.com>

Am Dienstag, 4. April 2017, 09:53:17 CEST schrieb Harsh Jain:

Hi Harsh,

> Hi,
> 
> Do we have any guidelines documented to decide what should be the
> algorithm priority. Specially for authenc implementation.Most of the
> drivers have fixed priority for all algos. Problem comes in when we
> have cbc(aes), hmac(sha1) and authenc(cbc(aes),hmac(sha1))
> implementation in driver. Base authenc driver gets more precedence
> because of higher priority(enc->base.cra_priority * 10 +
> auth_base->cra_priority;)
> 
> What should be the priority of
> cbc(aes),
> hmac(sha1)
> authenc(cbc(aes),hmac(sha1))

There is no general rule about the actual numbers. But commonly, the prios are 
set such that the prios of C implementations < ASM implementations < hardware 
accelerators. The idea is to give users the fastest implementation there is 
for his particular system.

Ciao
Stephan

^ permalink raw reply

* Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms
From: Horia Geantă @ 2017-04-04 12:42 UTC (permalink / raw)
  To: Michael Ellerman, Herbert Xu, Scott Wood, Roy Pledge,
	Madalin-Cristian Bucur
  Cc: David S. Miller, linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Dan Douglass,
	Alexandru Porosanu, Vakul Garg, Cristian Stoica, Claudiu Manoil,
	linuxppc-dev@
In-Reply-To: <87vaqkvk6c.fsf@concordia.ellerman.id.au>

On 4/4/2017 8:03 AM, Michael Ellerman wrote:
> Horia Geantă <horia.geanta@nxp.com> writes:
> 
>> Add support to submit ablkcipher and authenc algorithms
>> via the QI backend:
>> -ablkcipher:
>> cbc({aes,des,des3_ede})
>> ctr(aes), rfc3686(ctr(aes))
>> xts(aes)
>> -authenc:
>> authenc(hmac(md5),cbc({aes,des,des3_ede}))
>> authenc(hmac(sha*),cbc({aes,des,des3_ede}))
>>
>> caam/qi being a new driver, let's wait some time to settle down without
>> interfering with existing caam/jr driver.
>> Accordingly, for now all caam/qi algorithms (caamalg_qi module) are
>> marked to be of lower priority than caam/jr ones (caamalg module).
>>
>> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
>> Signed-off-by: Alex Porosanu <alexandru.porosanu@nxp.com>
>> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
>> ---
>>  drivers/crypto/caam/Kconfig        |   20 +-
>>  drivers/crypto/caam/Makefile       |    1 +
>>  drivers/crypto/caam/caamalg.c      |    9 +-
>>  drivers/crypto/caam/caamalg_desc.c |   77 +-
>>  drivers/crypto/caam/caamalg_desc.h |   15 +-
>>  drivers/crypto/caam/caamalg_qi.c   | 2387 ++++++++++++++++++++++++++++++++++++
>>  drivers/crypto/caam/sg_sw_qm.h     |  108 ++
>>  7 files changed, 2601 insertions(+), 16 deletions(-)
>>  create mode 100644 drivers/crypto/caam/caamalg_qi.c
>>  create mode 100644 drivers/crypto/caam/sg_sw_qm.h
> 
> 
> This appears to be blowing up my Freescale (NXP) P5020DS board:
> 

Michael,

Unfortunately I was not able to reproduce the issue on my P5020DS using
the same SHA1 (b189817cf789). Assuming you've used
corenet64_smp_defconfig and the p5020ds dts file in this tree.

Roy, Madalin,

This might be a probing order issue b/w CAAM/QI and QMan
(drivers/soc/fsl/qbman/qman_ccsr.c) platform drivers.
AFAICT the same dependency exists b/w QMan and DPAA Ethernet, thus is
should be fixed there too.

CAAM and QMan are sibling devices, having "soc" DT node as parent.
Should device link mechanism (Documentation/driver-api/device_link.rst)
be used to register the dependency b/w QMan driver (supplier) and
CAAM/QI driver (consumer)?

Thanks,
Horia

>   Unable to handle kernel paging request for data at address 0x00000020
>   Faulting instruction address: 0xc0000000004393e4
>   Oops: Kernel access of bad area, sig: 11 [#1]
>   SMP NR_CPUS=24 
>   CoreNet Generic
>   Modules linked in:
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789 #5
>   task: c0000000f70c0000 task.stack: c0000000f70c8000
>   NIP: c0000000004393e4 LR: c0000000004aeba0 CTR: c0000000004fa7d8
>   REGS: c0000000f70cb160 TRAP: 0300   Not tainted  (4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789)
>   MSR: 0000000080029000 <CE,EE,ME>
>     CR: 24adbe48  XER: 20000000
>   DEAR: 0000000000000020 ESR: 0000000000000000 SOFTE: 1 
>   GPR00: c0000000006feba0 c0000000f70cb3e0 c000000000e60000 0000000000000000 
>   GPR04: 0000000000000001 0000000000000000 c000000000e0b290 0000000000000003 
>   GPR08: 0000000000000004 c000000000ea5280 0000000000000004 0000000000000004 
>   GPR12: 0000000088adbe22 c00000003fff5000 c000000000ba3518 8000080088090fa8 
>   GPR16: 0000000000001000 c000000000ba3500 c0000000f72c68d8 0000000000000004 
>   GPR20: c000000000ea5280 c000000000ba34e8 0000000000000020 0000000000000004 
>   GPR24: c000000000eab7c0 0000000000000000 c0000000f7fc8818 c000000000eb0000 
>   GPR28: c0000000f786cc00 c000000000eab780 fffffffff786cc00 c000000000eab7c0 
>   NIP [c0000000004393e4] .gen_pool_alloc+0x0/0xc
>   LR [c0000000004aeba0] .qman_alloc_cgrid_range+0x24/0x54
>   Call Trace:
>   [c0000000f70cb3e0] [c000000000504054] .platform_device_register_full+0x12c/0x150 (unreliable)
>   [c0000000f70cb460] [c0000000006feba0] .caam_qi_init+0x158/0x63c
>   [c0000000f70cb5f0] [c0000000006fc64c] .caam_probe+0x8b8/0x1830
>   [c0000000f70cb740] [c000000000503288] .platform_drv_probe+0x60/0xac
>   [c0000000f70cb7c0] [c000000000501194] .driver_probe_device+0x248/0x344
>   [c0000000f70cb870] [c0000000005013b4] .__driver_attach+0x124/0x128
>   [c0000000f70cb900] [c0000000004fed90] .bus_for_each_dev+0x80/0xcc
>   [c0000000f70cb9a0] [c000000000500858] .driver_attach+0x24/0x38
>   [c0000000f70cba10] [c00000000050043c] .bus_add_driver+0x14c/0x29c
>   [c0000000f70cbab0] [c000000000501d64] .driver_register+0x8c/0x154
>   [c0000000f70cbb30] [c000000000503000] .__platform_driver_register+0x48/0x5c
>   [c0000000f70cbba0] [c000000000c7f798] .caam_driver_init+0x1c/0x30
>   [c0000000f70cbc10] [c000000000001904] .do_one_initcall+0x60/0x1a8
>   [c0000000f70cbcf0] [c000000000c35f30] .kernel_init_freeable+0x248/0x334
>   [c0000000f70cbdb0] [c0000000000020fc] .kernel_init+0x1c/0xf20
>   [c0000000f70cbe30] [c0000000000009bc] .ret_from_kernel_thread+0x58/0x9c
>   Instruction dump:
>   eb61ffd8 eb81ffe0 eba1ffe8 ebc1fff0 ebe1fff8 4e800020 38600000 4bffffb0 
>   7ce53b78 4bffff0c 7f67db78 4bffff24 <e8a30020> e8c30028 4bfffd30 fbe1fff8 
>   ---[ end trace 9f61087068991b02 ]---
> 
> 
> home:linux-next(4)(I)> git bisect log
> ...
> git bisect bad b189817cf7894e03fd3700acd923221d3007259e
> # first bad commit: [b189817cf7894e03fd3700acd923221d3007259e] crypto: caam/qi - add ablkcipher and authenc algorithms
> 
> 
> The oops is saying gen_pool_alloc() was called with a NULL pointer, so
> it seems qm_cgralloc is NULL:
> 
> static int qman_alloc_range(struct gen_pool *p, u32 *result, u32 cnt)
> {
> 	unsigned long addr;
> 
> 	addr = gen_pool_alloc(p, cnt);
> 	...
> 
> int qman_alloc_cgrid_range(u32 *result, u32 count)
> {
> 	return qman_alloc_range(qm_cgralloc, result, count);
> }
> 
> 
> I didn't pull the thread any further than that.
> 
> cheers
> 


^ permalink raw reply

* Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms
From: Laurentiu Tudor @ 2017-04-04 13:28 UTC (permalink / raw)
  To: Michael Ellerman, Horia Geantă, Herbert Xu, Scott Wood,
	Roy Pledge
  Cc: Claudiu Manoil, Cristian Stoica, Dan Douglass,
	linux-arm-kernel@lists.infradead.org, Vakul Garg,
	linuxppc-dev@lists.ozlabs.org, David S. Miller,
	Alexandru Porosanu, linux-crypto@vger.kernel.org
In-Reply-To: <87vaqkvk6c.fsf@concordia.ellerman.id.au>

Hi Michael,

Just a couple of basic things to check:
  - was the dtb updated to the newest?
  - is the qman node present? This should be easily visible in 
/proc/device-tree/soc@ffe000000/qman@318000.

---
Best Regards, Laurentiu

On 04/04/2017 08:03 AM, Michael Ellerman wrote:
> Horia Geantă <horia.geanta@nxp.com> writes:
>
>> Add support to submit ablkcipher and authenc algorithms
>> via the QI backend:
>> -ablkcipher:
>> cbc({aes,des,des3_ede})
>> ctr(aes), rfc3686(ctr(aes))
>> xts(aes)
>> -authenc:
>> authenc(hmac(md5),cbc({aes,des,des3_ede}))
>> authenc(hmac(sha*),cbc({aes,des,des3_ede}))
>>
>> caam/qi being a new driver, let's wait some time to settle down without
>> interfering with existing caam/jr driver.
>> Accordingly, for now all caam/qi algorithms (caamalg_qi module) are
>> marked to be of lower priority than caam/jr ones (caamalg module).
>>
>> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
>> Signed-off-by: Alex Porosanu <alexandru.porosanu@nxp.com>
>> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
>> ---
>>   drivers/crypto/caam/Kconfig        |   20 +-
>>   drivers/crypto/caam/Makefile       |    1 +
>>   drivers/crypto/caam/caamalg.c      |    9 +-
>>   drivers/crypto/caam/caamalg_desc.c |   77 +-
>>   drivers/crypto/caam/caamalg_desc.h |   15 +-
>>   drivers/crypto/caam/caamalg_qi.c   | 2387 ++++++++++++++++++++++++++++++++++++
>>   drivers/crypto/caam/sg_sw_qm.h     |  108 ++
>>   7 files changed, 2601 insertions(+), 16 deletions(-)
>>   create mode 100644 drivers/crypto/caam/caamalg_qi.c
>>   create mode 100644 drivers/crypto/caam/sg_sw_qm.h
>
>
> This appears to be blowing up my Freescale (NXP) P5020DS board:
>
>    Unable to handle kernel paging request for data at address 0x00000020
>    Faulting instruction address: 0xc0000000004393e4
>    Oops: Kernel access of bad area, sig: 11 [#1]
>    SMP NR_CPUS=24
>    CoreNet Generic
>    Modules linked in:
>    CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789 #5
>    task: c0000000f70c0000 task.stack: c0000000f70c8000
>    NIP: c0000000004393e4 LR: c0000000004aeba0 CTR: c0000000004fa7d8
>    REGS: c0000000f70cb160 TRAP: 0300   Not tainted  (4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789)
>    MSR: 0000000080029000 <CE,EE,ME>
>      CR: 24adbe48  XER: 20000000
>    DEAR: 0000000000000020 ESR: 0000000000000000 SOFTE: 1
>    GPR00: c0000000006feba0 c0000000f70cb3e0 c000000000e60000 0000000000000000
>    GPR04: 0000000000000001 0000000000000000 c000000000e0b290 0000000000000003
>    GPR08: 0000000000000004 c000000000ea5280 0000000000000004 0000000000000004
>    GPR12: 0000000088adbe22 c00000003fff5000 c000000000ba3518 8000080088090fa8
>    GPR16: 0000000000001000 c000000000ba3500 c0000000f72c68d8 0000000000000004
>    GPR20: c000000000ea5280 c000000000ba34e8 0000000000000020 0000000000000004
>    GPR24: c000000000eab7c0 0000000000000000 c0000000f7fc8818 c000000000eb0000
>    GPR28: c0000000f786cc00 c000000000eab780 fffffffff786cc00 c000000000eab7c0
>    NIP [c0000000004393e4] .gen_pool_alloc+0x0/0xc
>    LR [c0000000004aeba0] .qman_alloc_cgrid_range+0x24/0x54
>    Call Trace:
>    [c0000000f70cb3e0] [c000000000504054] .platform_device_register_full+0x12c/0x150 (unreliable)
>    [c0000000f70cb460] [c0000000006feba0] .caam_qi_init+0x158/0x63c
>    [c0000000f70cb5f0] [c0000000006fc64c] .caam_probe+0x8b8/0x1830
>    [c0000000f70cb740] [c000000000503288] .platform_drv_probe+0x60/0xac
>    [c0000000f70cb7c0] [c000000000501194] .driver_probe_device+0x248/0x344
>    [c0000000f70cb870] [c0000000005013b4] .__driver_attach+0x124/0x128
>    [c0000000f70cb900] [c0000000004fed90] .bus_for_each_dev+0x80/0xcc
>    [c0000000f70cb9a0] [c000000000500858] .driver_attach+0x24/0x38
>    [c0000000f70cba10] [c00000000050043c] .bus_add_driver+0x14c/0x29c
>    [c0000000f70cbab0] [c000000000501d64] .driver_register+0x8c/0x154
>    [c0000000f70cbb30] [c000000000503000] .__platform_driver_register+0x48/0x5c
>    [c0000000f70cbba0] [c000000000c7f798] .caam_driver_init+0x1c/0x30
>    [c0000000f70cbc10] [c000000000001904] .do_one_initcall+0x60/0x1a8
>    [c0000000f70cbcf0] [c000000000c35f30] .kernel_init_freeable+0x248/0x334
>    [c0000000f70cbdb0] [c0000000000020fc] .kernel_init+0x1c/0xf20
>    [c0000000f70cbe30] [c0000000000009bc] .ret_from_kernel_thread+0x58/0x9c
>    Instruction dump:
>    eb61ffd8 eb81ffe0 eba1ffe8 ebc1fff0 ebe1fff8 4e800020 38600000 4bffffb0
>    7ce53b78 4bffff0c 7f67db78 4bffff24 <e8a30020> e8c30028 4bfffd30 fbe1fff8
>    ---[ end trace 9f61087068991b02 ]---
>
>
> home:linux-next(4)(I)> git bisect log
> ...
> git bisect bad b189817cf7894e03fd3700acd923221d3007259e
> # first bad commit: [b189817cf7894e03fd3700acd923221d3007259e] crypto: caam/qi - add ablkcipher and authenc algorithms
>
>
> The oops is saying gen_pool_alloc() was called with a NULL pointer, so
> it seems qm_cgralloc is NULL:
>
> static int qman_alloc_range(struct gen_pool *p, u32 *result, u32 cnt)
> {
> 	unsigned long addr;
>
> 	addr = gen_pool_alloc(p, cnt);
> 	...
>
> int qman_alloc_cgrid_range(u32 *result, u32 count)
> {
> 	return qman_alloc_range(qm_cgralloc, result, count);
> }
>
>
> I didn't pull the thread any further than that.
>
> cheers
>

^ 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