Re: [PATCH 1/2] hwrng: msm: add a spinlock and support for blocking reads

[Timur Tabi]

On 6/22/18 12:36 AM, Stephen Boyd wrote:

Quoting Timur Tabi (2018-06-21 08:17:55)

@@ -96,11 +110,39 @@ static int msm_rng_read(struct hwrng *hwrng, void *data, 
size_t max, bool wait)
  
 /* read random data from hardware */

 do {
-   val = readl_relaxed(rng->base + PRNG_STATUS);
-   if (!(val & PRNG_STATUS_DATA_AVAIL))
-   break;
+   spin_lock(&rng->lock);
+
+   /*
+* First check the status bit.  If 'wait' is true, then wait
+* up to TIMEOUT microseconds for data to be available.
+*/
+   if (wait) {
+   int ret;

Please don't do local variables like this. Put them at the top of the
function.


Ok.




+
+   ret = readl_poll_timeout_atomic(rng->base + PRNG_STATUS,
+   val, val & PRNG_STATUS_DATA_AVAIL, 0, TIMEOUT);
+   if (ret) {
+   /* Timed out */
+   spin_unlock(&rng->lock);
+   break;
+   }
+   } else {
+   val = readl_relaxed(rng->base + PRNG_STATUS);
+   if (!(val & PRNG_STATUS_DATA_AVAIL)) {
+   spin_unlock(&rng->lock);
+   break;
+   }
+   }
  
 val = readl_relaxed(rng->base + PRNG_DATA_OUT);

+   spin_unlock(&rng->lock);

Maybe this should be written as:

spin_lock()
if (wait) {
has_data = readl_poll_timeout_atomic(...) == 0;
} else {
val = readl_relaxed(rng->base + PRNG_STATUS);
has_data = val & PRNG_STATUS_DATA_AVAIL;
}

if (has_data)
val = readl_relaxed(rng->base + PRNG_DATA_OUT);
spin_unlock();

if (!has_data)
break;


That would work, but I don't really see this as better, just different.


+   /*
+* Zero is technically a valid random number, but it's also
+* the value returned if the PRNG is not enabled properly.
+* To avoid accidentally returning all zeros, treat it as
+* invalid and just return what we've already read.
+*/
 if (!val)
 break;

Is this a related change? Looks like a nice comment that isn't related
to locking.


It's slightly related in that the locking is needed to reduce the number 
of times we read zero from the DATA_OUT register.


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 0/5] Improve crypto compression tests

[Jan Glauber]

Hi Herbert,

I've been trying to address your comment from:
https://marc.info/?l=linux-crypto-vger&m=152410933702305&w=2
with patches 1-4.

I've only found deflate and LZS to have a non-generic implementation,
but LZS does not have a generic one so for now deflate seems to be
the only algorithm that could use the mixed test.

Jan Glauber (5):
  crypto: deflate - Rename to generic
  crypto: thunderx_zip - Add driver names and module aliases
  crypto: testmgr - Improve compression/decompression test
  crypto: testmgr - Add test vectors for LZS compression
  crypto: thunderx_zip - Make functions static

 crypto/Makefile |  2 +-
 crypto/{deflate.c => deflate_generic.c} |  3 +
 crypto/testmgr.c| 32 +-
 crypto/testmgr.h| 77 +
 drivers/crypto/cavium/zip/zip_crypto.c  | 16 ++---
 drivers/crypto/cavium/zip/zip_main.c|  8 ++-
 6 files changed, 126 insertions(+), 12 deletions(-)
 rename crypto/{deflate.c => deflate_generic.c} (98%)

-- 
2.17.1

[PATCH 2/5] crypto: thunderx_zip - Add driver names and module aliases

[Jan Glauber]

Add missing cra_driver_name's and module aliases for deflate and lzs.

Signed-off-by: Jan Glauber 
---
 drivers/crypto/cavium/zip/zip_main.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/cavium/zip/zip_main.c 
b/drivers/crypto/cavium/zip/zip_main.c
index be055b9547f6..68e67be954ad 100644
--- a/drivers/crypto/cavium/zip/zip_main.c
+++ b/drivers/crypto/cavium/zip/zip_main.c
@@ -351,6 +351,7 @@ static struct pci_driver zip_driver = {
 
 static struct crypto_alg zip_comp_deflate = {
.cra_name   = "deflate",
+   .cra_driver_name= "deflate-thunderx",
.cra_flags  = CRYPTO_ALG_TYPE_COMPRESS,
.cra_ctxsize= sizeof(struct zip_kernel_ctx),
.cra_priority   = 300,
@@ -365,6 +366,7 @@ static struct crypto_alg zip_comp_deflate = {
 
 static struct crypto_alg zip_comp_lzs = {
.cra_name   = "lzs",
+   .cra_driver_name= "lzs-thunderx",
.cra_flags  = CRYPTO_ALG_TYPE_COMPRESS,
.cra_ctxsize= sizeof(struct zip_kernel_ctx),
.cra_priority   = 300,
@@ -384,7 +386,7 @@ static struct scomp_alg zip_scomp_deflate = {
.decompress = zip_scomp_decompress,
.base   = {
.cra_name   = "deflate",
-   .cra_driver_name= "deflate-scomp",
+   .cra_driver_name= "deflate-scomp-thunderx",
.cra_module = THIS_MODULE,
.cra_priority   = 300,
}
@@ -397,7 +399,7 @@ static struct scomp_alg zip_scomp_lzs = {
.decompress = zip_scomp_decompress,
.base   = {
.cra_name   = "lzs",
-   .cra_driver_name= "lzs-scomp",
+   .cra_driver_name= "lzs-scomp-thunderx",
.cra_module = THIS_MODULE,
.cra_priority   = 300,
}
@@ -725,3 +727,5 @@ MODULE_AUTHOR("Cavium Inc");
 MODULE_DESCRIPTION("Cavium Inc ThunderX ZIP Driver");
 MODULE_LICENSE("GPL v2");
 MODULE_DEVICE_TABLE(pci, zip_id_table);
+MODULE_ALIAS_CRYPTO("deflate");
+MODULE_ALIAS_CRYPTO("lzs");
-- 
2.17.1

[PATCH 3/5] crypto: testmgr - Improve compression/decompression test

[Jan Glauber]

While commit 336073840a87 ("crypto: testmgr - Allow different compression 
results")
allowed to test non-generic compression algorithms there are some corner
cases that would not be detected in test_comp().

For example if input -> compression -> decompression would all yield
the same bytes the test would still pass.

Improve the compression test by using the generic variant (if available)
to decompress the compressed test vector from the non-generic
algorithm.

Suggested-by: Herbert Xu 
Signed-off-by: Jan Glauber 
---
 crypto/testmgr.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index d1d99843cce4..cfb5fe4c5ccf 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1346,6 +1346,7 @@ static int test_comp(struct crypto_comp *tfm,
 int ctcount, int dtcount)
 {
const char *algo = crypto_tfm_alg_driver_name(crypto_comp_tfm(tfm));
+   const char *name = crypto_tfm_alg_name(crypto_comp_tfm(tfm));
char *output, *decomp_output;
unsigned int i;
int ret;
@@ -1363,6 +1364,8 @@ static int test_comp(struct crypto_comp *tfm,
for (i = 0; i < ctcount; i++) {
int ilen;
unsigned int dlen = COMP_BUF_SIZE;
+   struct crypto_comp *tfm_decomp = NULL;
+   char *gname;
 
memset(output, 0, sizeof(COMP_BUF_SIZE));
memset(decomp_output, 0, sizeof(COMP_BUF_SIZE));
@@ -1377,9 +1380,27 @@ static int test_comp(struct crypto_comp *tfm,
goto out;
}
 
+   /*
+* If compression of a non-generic algorithm was tested try to
+* decompress using the generic variant.
+*/
+   if (!strstr(algo, "generic")) {
+   /* Construct name from cra_name + "-generic" */
+   gname = kmalloc(strlen(name) + 9, GFP_KERNEL);
+   strncpy(gname, name, strlen(name));
+   strncpy(gname + strlen(name), "-generic", 9);
+
+   tfm_decomp = crypto_alloc_comp(gname, 0, 0);
+   kfree(gname);
+   }
+
+   /* If there is no generic variant use the same tfm as before. */
+   if (!tfm_decomp || IS_ERR(tfm_decomp))
+   tfm_decomp = tfm;
+
ilen = dlen;
dlen = COMP_BUF_SIZE;
-   ret = crypto_comp_decompress(tfm, output,
+   ret = crypto_comp_decompress(tfm_decomp, output,
 ilen, decomp_output, &dlen);
if (ret) {
pr_err("alg: comp: compression failed: decompress: on 
test %d for %s failed: ret=%d\n",
-- 
2.17.1

[PATCH 4/5] crypto: testmgr - Add test vectors for LZS compression

[Jan Glauber]

The test vectors were generated using the ThunderX ZIP coprocessor.

Signed-off-by: Jan Glauber 
---
 crypto/testmgr.c |  9 ++
 crypto/testmgr.h | 77 
 2 files changed, 86 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index cfb5fe4c5ccf..8e9ff1229e93 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -3238,6 +3238,15 @@ static const struct alg_test_desc alg_test_descs[] = {
.decomp = __VECS(lzo_decomp_tv_template)
}
}
+   }, {
+   .alg = "lzs",
+   .test = alg_test_comp,
+   .suite = {
+   .comp = {
+   .comp = __VECS(lzs_comp_tv_template),
+   .decomp = __VECS(lzs_decomp_tv_template)
+   }
+   }
}, {
.alg = "md4",
.test = alg_test_hash,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index b950aa234e43..ae7fecadcade 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -31699,6 +31699,83 @@ static const struct comp_testvec 
lzo_decomp_tv_template[] = {
},
 };
 
+/*
+ * LZS test vectors (null-terminated strings).
+ */
+static const struct comp_testvec lzs_comp_tv_template[] = {
+   {
+   .inlen  = 70,
+   .outlen = 40,
+   .input  = "Join us now and share the software "
+   "Join us now and share the software ",
+   .output = "\x25\x1b\xcd\x26\xe1\x01\xd4\xe6"
+ "\x20\x37\x1b\xce\xe2\x03\x09\xb8"
+ "\xc8\x20\x39\x9a\x0c\x27\x23\x28"
+ "\x80\xe8\x68\xc2\x07\x33\x79\x98"
+ "\xe8\x77\xc6\xda\x3f\xfc\xc0\x00",
+   }, {
+   .inlen  = 184,
+   .outlen = 130,
+   .input  = "This document describes a compression method based 
on the LZS "
+   "compression algorithm.  This document defines the 
application of "
+   "the LZS algorithm to the IP Payload Compression 
Protocol.",
+   .output = "\x2a\x1a\x0d\x27\x31\x01\x90\xde"
+ "\x63\x3a\x9b\x4c\xa6\xe3\xa6\x24"
+ "\x32\x9c\xcc\x67\x23\x49\x8b\x0c"
+ "\x08\x0c\x22\x03\x19\xbc\xda\x70"
+ "\x39\x62\x83\x99\xa4\xde\x6e\x10"
+ "\x67\x43\xa1\xa0\xde\x64\x10\x18"
+ "\x8c\x27\x33\x2e\x18\xc8\x38\xe0"
+ "\xca\x20\x26\x16\x8a\x7a\x4f\x53"
+ "\x09\xb0\xce\x6f\xde\x19\xa0\xda"
+ "\x2e\x10\x08\x3a\xdf\x06\x63\x49"
+ "\xb8\xca\x73\xdb\x61\x84\xe0\x70"
+ "\x36\x1a\x4c\x66\x13\xa6\xca\x37"
+ "\x99\xb9\x3e\x3b\xdf\x1c\x90\x6f"
+ "\xca\xe1\x24\xa0\x20\x28\x18\x4f"
+ "\x26\xc3\x79\x87\xe0\x10\xfc\xbe"
+ "\x65\x03\x91\xbf\x42\x7d\x83\x60"
+ "\xbb\x00",
+   },
+};
+
+static const struct comp_testvec lzs_decomp_tv_template[] = {
+   {
+   .inlen  = 130,
+   .outlen = 184,
+   .input  = "\x2a\x1a\x0d\x27\x31\x01\x90\xde"
+ "\x63\x3a\x9b\x4c\xa6\xe3\xa6\x24"
+ "\x32\x9c\xcc\x67\x23\x49\x8b\x0c"
+ "\x08\x0c\x22\x03\x19\xbc\xda\x70"
+ "\x39\x62\x83\x99\xa4\xde\x6e\x10"
+ "\x67\x43\xa1\xa0\xde\x64\x10\x18"
+ "\x8c\x27\x33\x2e\x18\xc8\x38\xe0"
+ "\xca\x20\x26\x16\x8a\x7a\x4f\x53"
+ "\x09\xb0\xce\x6f\xde\x19\xa0\xda"
+ "\x2e\x10\x08\x3a\xdf\x06\x63\x49"
+ "\xb8\xca\x73\xdb\x61\x84\xe0\x70"
+ "\x36\x1a\x4c\x66\x13\xa6\xca\x37"
+ "\x99\xb9\x3e\x3b\xdf\x1c\x90\x6f"
+ "\xca\xe1\x24\xa0\x20\x28\x18\x4f"
+ "\x26\xc3\x79\x87\xe0\x10\xfc\xbe"
+ "\x65\x03\x91\xbf\x42\x7d\x83\x60"
+ "\xbb\x00",
+   .output = "This document describes a compression method based 
on the LZS "
+   "compression algorithm.  This document defines the 
application of "
+   "the LZS algorithm to the IP Payload Compression 
Protocol.",
+   }, {
+   .inlen  = 40,
+   .outlen = 70,
+   .input  = "\x25\x1b\xcd\x26\xe1\x01\xd4\xe6"
+ "\x20\x37\x1b\xce\xe2\x03\x09\xb8"
+ "\xc8\x20\x39\x9a\x0c\x27\x23\x28"
+ "\x80\xe8\x68\xc2\x07\x33\x79\x98"
+ "\xe8\x77\xc6\xda

[PATCH 5/5] crypto: thunderx_zip - Make functions static

[Jan Glauber]

Make functions static where possible, found by sparse.

Signed-off-by: Jan Glauber 
---
 drivers/crypto/cavium/zip/zip_crypto.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/cavium/zip/zip_crypto.c 
b/drivers/crypto/cavium/zip/zip_crypto.c
index b92b6e7e100f..f9e99a15d2a0 100644
--- a/drivers/crypto/cavium/zip/zip_crypto.c
+++ b/drivers/crypto/cavium/zip/zip_crypto.c
@@ -69,7 +69,7 @@ static void zip_static_init_zip_ops(struct zip_operation 
*zip_ops,
zip_ops->csum = 1; /* Adler checksum desired */
 }
 
-int zip_ctx_init(struct zip_kernel_ctx *zip_ctx, int lzs_flag)
+static int zip_ctx_init(struct zip_kernel_ctx *zip_ctx, int lzs_flag)
 {
struct zip_operation  *comp_ctx   = &zip_ctx->zip_comp;
struct zip_operation  *decomp_ctx = &zip_ctx->zip_decomp;
@@ -107,7 +107,7 @@ int zip_ctx_init(struct zip_kernel_ctx *zip_ctx, int 
lzs_flag)
return -ENOMEM;
 }
 
-void zip_ctx_exit(struct zip_kernel_ctx *zip_ctx)
+static void zip_ctx_exit(struct zip_kernel_ctx *zip_ctx)
 {
struct zip_operation  *comp_ctx   = &zip_ctx->zip_comp;
struct zip_operation  *dec_ctx = &zip_ctx->zip_decomp;
@@ -119,9 +119,9 @@ void zip_ctx_exit(struct zip_kernel_ctx *zip_ctx)
zip_data_buf_free(dec_ctx->output, MAX_OUTPUT_BUFFER_SIZE);
 }
 
-int zip_compress(const u8 *src, unsigned int slen,
-u8 *dst, unsigned int *dlen,
-struct zip_kernel_ctx *zip_ctx)
+static int zip_compress(const u8 *src, unsigned int slen,
+   u8 *dst, unsigned int *dlen,
+   struct zip_kernel_ctx *zip_ctx)
 {
struct zip_operation  *zip_ops   = NULL;
struct zip_state  *zip_state;
@@ -155,9 +155,9 @@ int zip_compress(const u8 *src, unsigned int slen,
return ret;
 }
 
-int zip_decompress(const u8 *src, unsigned int slen,
-  u8 *dst, unsigned int *dlen,
-  struct zip_kernel_ctx *zip_ctx)
+static int zip_decompress(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen,
+ struct zip_kernel_ctx *zip_ctx)
 {
struct zip_operation  *zip_ops   = NULL;
struct zip_state  *zip_state;
-- 
2.17.1

[PATCH 1/5] crypto: deflate - Rename to generic

[Jan Glauber]

Rename deflate -> deflate_generic and add the default priority of 100.

Signed-off-by: Jan Glauber 
---
 crypto/Makefile | 2 +-
 crypto/{deflate.c => deflate_generic.c} | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)
 rename crypto/{deflate.c => deflate_generic.c} (98%)

diff --git a/crypto/Makefile b/crypto/Makefile
index 6d1d40eeb964..351c7212aff8 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -119,7 +119,7 @@ obj-$(CONFIG_CRYPTO_SPECK) += speck.o
 obj-$(CONFIG_CRYPTO_SALSA20) += salsa20_generic.o
 obj-$(CONFIG_CRYPTO_CHACHA20) += chacha20_generic.o
 obj-$(CONFIG_CRYPTO_POLY1305) += poly1305_generic.o
-obj-$(CONFIG_CRYPTO_DEFLATE) += deflate.o
+obj-$(CONFIG_CRYPTO_DEFLATE) += deflate_generic.o
 obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
 obj-$(CONFIG_CRYPTO_CRC32C) += crc32c_generic.o
 obj-$(CONFIG_CRYPTO_CRC32) += crc32_generic.o
diff --git a/crypto/deflate.c b/crypto/deflate_generic.c
similarity index 98%
rename from crypto/deflate.c
rename to crypto/deflate_generic.c
index 94ec3b36a8e8..66bff5614195 100644
--- a/crypto/deflate.c
+++ b/crypto/deflate_generic.c
@@ -279,6 +279,8 @@ static int deflate_sdecompress(struct crypto_scomp *tfm, 
const u8 *src,
 
 static struct crypto_alg alg = {
.cra_name   = "deflate",
+   .cra_driver_name= "deflate-generic",
+   .cra_priority   = 100,
.cra_flags  = CRYPTO_ALG_TYPE_COMPRESS,
.cra_ctxsize= sizeof(struct deflate_ctx),
.cra_module = THIS_MODULE,
@@ -341,3 +343,4 @@ MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Deflate Compression Algorithm for IPCOMP");
 MODULE_AUTHOR("James Morris ");
 MODULE_ALIAS_CRYPTO("deflate");
+MODULE_ALIAS_CRYPTO("deflate-generic");
-- 
2.17.1

Re: [PATCH] crypto: atmel-ecc - fix to allow multi segment scatterlists

[Herbert Xu]

On Wed, Jun 13, 2018 at 04:29:58PM +0300, Tudor Ambarus wrote:
> Remove the limitation of single element scatterlists. ECDH with
> multi-element scatterlists is needed by TPM.
> 
> Similar to 'commit 95ec01ba1ef0 ("crypto: ecdh - fix to allow multi
> segment scatterlists")'.
> 
> Signed-off-by: Tudor Ambarus 

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 00/10] crypto: inside-secure - sha512/384 support

[Herbert Xu]

On Tue, May 29, 2018 at 02:13:42PM +0200, Antoine Tenart wrote:
> Hello Herbert,
> 
> This series adds support for the SHA512 and SHA384 algorithms in the
> Inside Secure SafeXcel driver. Variants of those two algorithms are also
> added as well (hmac, and AEAD).
> 
> Before doing so a few patches rework the driver to prepare for the new
> algorithms additions.
> 
> Thanks!
> Antoine
> 
> Antoine Tenart (10):
>   crypto: inside-secure - use the error handler for invalidation
> requests
>   crypto: inside-secure - improve the counter computation
>   crypto: sha512_generic - add a sha512 0-length pre-computed hash
>   crypto: inside-secure - sha512 support
>   crypto: inside-secure - hmac(sha512) support
>   crypto: inside-secure - authenc(hmac(sha512),cbc(aes)) support
>   crypto: sha512_generic - add a sha384 0-length pre-computed hash
>   crypto: inside-secure - sha384 support
>   crypto: inside-secure - hmac(sha384) support
>   crypto: inside-secure - authenc(hmac(sha384),cbc(aes)) support
> 
>  crypto/sha512_generic.c   |  22 +
>  drivers/crypto/inside-secure/safexcel.c   |   6 +
>  drivers/crypto/inside-secure/safexcel.h   |  23 +-
>  .../crypto/inside-secure/safexcel_cipher.c|  89 +++-
>  drivers/crypto/inside-secure/safexcel_hash.c  | 381 --
>  include/crypto/sha.h  |   4 +
>  6 files changed, 469 insertions(+), 56 deletions(-)

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

Re: [PATCH] crypto: cavium: make structure algs static

[Herbert Xu]

On Fri, Jun 01, 2018 at 02:12:27PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> The structure algs is local to the source and does not need to be in
> global scope, so make it static.
> 
> Cleans up sparse warning:
> drivers/crypto/cavium/cpt/cptvf_algs.c:354:19: warning: symbol 'algs'
> was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King 

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

Re: [PATCH] crypto: atmel-ecc - remove overly verbose dev_info

[Herbert Xu]

On Wed, Jun 13, 2018 at 04:29:59PM +0300, Tudor Ambarus wrote:
> Remove it because when using a slow console, it can affect
> the speed of crypto operations.
> 
> Similar to 'commit 730f23b66095 ("crypto: vmx - Remove overly
> verbose printk from AES XTS init")'.
> 
> Signed-off-by: Tudor Ambarus 

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] hwrng: msm: add a spinlock and support for blocking reads

[Stanimir Varbanov]

Hi,

On 06/21/2018 06:17 PM, Timur Tabi wrote:
> The hwrng.read callback includes a boolean parameter called 'wait'
> which indicates whether the function should block and wait for
> more data.
> 
> When 'wait' is true, the driver spins on the DATA_AVAIL bit or until
> a reasonable timeout.  The timeout can occur if there is a heavy load
> on reading the PRNG.
> 
> The same code also needs a spinlock to protect against race conditions.
> 
> If multiple cores hammer on the PRNG, it's possible for a race
> condition to occur between reading the status register and
> reading the data register.  Add a spinlock to protect against
> that.

Before entering into the read function we already hold a mutex which
serializes data reading so I cannot imagine how below sequence could
happen. Can you explain how to reproduce this race?

> 
> 1. Core 1 reads status register, shows data is available.
> 2. Core 2 also reads status register, same result
> 3. Core 2 reads data register, depleting all entropy
> 4. Core 1 reads data register, which returns 0
> 
> Signed-off-by: Timur Tabi 
> ---
>  drivers/char/hw_random/msm-rng.c | 57 
> +++-
>  1 file changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/hw_random/msm-rng.c 
> b/drivers/char/hw_random/msm-rng.c
> index 841fee845ec9..44580588b938 100644
> --- a/drivers/char/hw_random/msm-rng.c
> +++ b/drivers/char/hw_random/msm-rng.c
> @@ -15,9 +15,11 @@

-- 
regards,
Stan

Re: [PATCH 1/2] hwrng: msm: add a spinlock and support for blocking reads

[Timur Tabi]

On 6/22/18 10:38 AM, Stanimir Varbanov wrote:

Before entering into the read function we already hold a mutex which
serializes data reading so I cannot imagine how below sequence could
happen. Can you explain how to reproduce this race?


1. Core 1 reads status register, shows data is available.
2. Core 2 also reads status register, same result
3. Core 2 reads data register, depleting all entropy
4. Core 1 reads data register, which returns 0


I have a test which spawns 100 copies of rngtest on a 48-core machine. 
Without the spinlock, the driver returns no data much more often.


If there really is a mutex that serializes data reads across all cores, 
then I don't have an explanation.


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] dt-bindings: fsl-imx-sahara: Add i.MX51 as a supported SoC

[Fabio Estevam]

From: Fabio Estevam 

i.MX51 and i.MX51 share the same sahara IP block version, so add
i.MX51 in the list of supported SoCs.

Signed-off-by: Fabio Estevam 
---
 Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt 
b/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
index e8a35c7..5343a48 100644
--- a/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
+++ b/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
@@ -1,8 +1,9 @@
 Freescale SAHARA Cryptographic Accelerator included in some i.MX chips.
-Currently only i.MX27 and i.MX53 are supported.
+Currently i.MX27, i.MX51 and i.MX53 are supported.
 
 Required properties:
-- compatible : Should be "fsl,-sahara"
+- compatible : Should be "fsl,imx27-sahara", "fsl,imx51-sahara"
+or "fsl,imx53-sahara"
 - reg : Should contain SAHARA registers location and length
 - interrupts : Should contain SAHARA interrupt number
 
-- 
2.7.4

[PATCH 2/2] crypto: sahara - Add i.MX51 entry

[Fabio Estevam]

From: Fabio Estevam 

i.MX51 and i.MX51 share the same sahara IP block version, so add
i.MX51 in the compatible list.

Signed-off-by: Fabio Estevam 
---
 drivers/crypto/sahara.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 0f2245e..ce1a42c 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -1382,6 +1382,7 @@ static const struct platform_device_id 
sahara_platform_ids[] = {
 MODULE_DEVICE_TABLE(platform, sahara_platform_ids);
 
 static const struct of_device_id sahara_dt_ids[] = {
+   { .compatible = "fsl,imx51-sahara" },
{ .compatible = "fsl,imx53-sahara" },
{ .compatible = "fsl,imx27-sahara" },
{ /* sentinel */ }
@@ -1507,7 +1508,9 @@ static int sahara_probe(struct platform_device *pdev)
if (version != SAHARA_VERSION_3)
err = -ENODEV;
} else if (of_device_is_compatible(pdev->dev.of_node,
-   "fsl,imx53-sahara")) {
+  "fsl,imx53-sahara") ||
+  of_device_is_compatible(pdev->dev.of_node,
+  "fsl,imx51-sahara")) {
if (((version >> 8) & 0xff) != SAHARA_VERSION_4)
err = -ENODEV;
version = (version >> 8) & 0xff;
-- 
2.7.4

Re: [PATCH 1/2] hwrng: msm: add a spinlock and support for blocking reads

[Stephen Boyd]

Quoting Timur Tabi (2018-06-22 08:41:11)
> On 6/22/18 10:38 AM, Stanimir Varbanov wrote:
> > Before entering into the read function we already hold a mutex which
> > serializes data reading so I cannot imagine how below sequence could
> > happen. Can you explain how to reproduce this race?
> > 
> >> 1. Core 1 reads status register, shows data is available.
> >> 2. Core 2 also reads status register, same result
> >> 3. Core 2 reads data register, depleting all entropy
> >> 4. Core 1 reads data register, which returns 0
> 
> I have a test which spawns 100 copies of rngtest on a 48-core machine. 
> Without the spinlock, the driver returns no data much more often.
> 
> If there really is a mutex that serializes data reads across all cores, 
> then I don't have an explanation.
> 

Perhaps it's because you implemented the 'wait' functionality in this
driver? Before the patch there wasn't any sort of wait check so we would
bail out if there wasn't any data even if the caller requested that we
wait for randomness to be available.

Re: [PATCH 1/2] hwrng: msm: add a spinlock and support for blocking reads

[Timur Tabi]

On 6/22/18 12:51 PM, Stephen Boyd wrote:

Perhaps it's because you implemented the 'wait' functionality in this
driver? Before the patch there wasn't any sort of wait check so we would
bail out if there wasn't any data even if the caller requested that we
wait for randomness to be available.


No, my tests showed the race condition (or at least something that looks 
like a race condition) even without the 'wait' feature.  I added support 
for 'wait' only recently, but I've been working with the crypto people 
for a month on everything else.


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2 2/2] crypto: sahara - Add i.MX51 entry

[Fabio Estevam]

From: Fabio Estevam 

i.MX51 and i.MX53 share the same sahara IP block version, so add
i.MX51 in the compatible list.

Signed-off-by: Fabio Estevam 
---
Changes since v1:
- Fix typo in commit log "i.MX51 and i.MX53"

 drivers/crypto/sahara.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 0f2245e..ce1a42c 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -1382,6 +1382,7 @@ static const struct platform_device_id 
sahara_platform_ids[] = {
 MODULE_DEVICE_TABLE(platform, sahara_platform_ids);
 
 static const struct of_device_id sahara_dt_ids[] = {
+   { .compatible = "fsl,imx51-sahara" },
{ .compatible = "fsl,imx53-sahara" },
{ .compatible = "fsl,imx27-sahara" },
{ /* sentinel */ }
@@ -1507,7 +1508,9 @@ static int sahara_probe(struct platform_device *pdev)
if (version != SAHARA_VERSION_3)
err = -ENODEV;
} else if (of_device_is_compatible(pdev->dev.of_node,
-   "fsl,imx53-sahara")) {
+  "fsl,imx53-sahara") ||
+  of_device_is_compatible(pdev->dev.of_node,
+  "fsl,imx51-sahara")) {
if (((version >> 8) & 0xff) != SAHARA_VERSION_4)
err = -ENODEV;
version = (version >> 8) & 0xff;
-- 
2.7.4

[PATCH v2 1/2] dt-bindings: fsl-imx-sahara: Add i.MX51 as a supported SoC

[Fabio Estevam]

From: Fabio Estevam 

i.MX51 and i.MX53 share the same sahara IP block version, so add
i.MX51 in the list of supported SoCs.

Signed-off-by: Fabio Estevam 
---
Changes since v1:
- Fix typo in commit log "i.MX51 and i.MX53"

 Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt 
b/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
index e8a35c7..5343a48 100644
--- a/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
+++ b/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
@@ -1,8 +1,9 @@
 Freescale SAHARA Cryptographic Accelerator included in some i.MX chips.
-Currently only i.MX27 and i.MX53 are supported.
+Currently i.MX27, i.MX51 and i.MX53 are supported.
 
 Required properties:
-- compatible : Should be "fsl,-sahara"
+- compatible : Should be "fsl,imx27-sahara", "fsl,imx51-sahara"
+or "fsl,imx53-sahara"
 - reg : Should contain SAHARA registers location and length
 - interrupts : Should contain SAHARA interrupt number
 
-- 
2.7.4

[cryptodev:master 5/15] drivers/crypto/inside-secure/safexcel_hash.c:1299:25: sparse: cast truncates bits from constant value (6a09e667f3bcc908 becomes f3bcc908)

[kbuild test robot]

tree:   
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
head:   38641b83ebc54635151810eeef00b61da3097952
commit: b460edb6230ac2877b0d176b9122736fed6f3c6e [5/15] crypto: inside-secure - 
sha512 support
reproduce:
# apt-get install sparse
git checkout b460edb6230ac2877b0d176b9122736fed6f3c6e
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/slab.h:631:13: sparse: undefined identifier 
'__builtin_mul_overflow'
   drivers/crypto/inside-secure/safexcel_hash.c:1005:45: sparse: cast to 
restricted __le32
   drivers/crypto/inside-secure/safexcel_hash.c:1006:45: sparse: cast to 
restricted __le32
   include/linux/slab.h:631:13: sparse: call with no type!
>> drivers/crypto/inside-secure/safexcel_hash.c:1299:25: sparse: cast truncates 
>> bits from constant value (6a09e667f3bcc908 becomes f3bcc908)
>> drivers/crypto/inside-secure/safexcel_hash.c:1301:25: sparse: cast truncates 
>> bits from constant value (bb67ae8584caa73b becomes 84caa73b)
>> drivers/crypto/inside-secure/safexcel_hash.c:1303:25: sparse: cast truncates 
>> bits from constant value (3c6ef372fe94f82b becomes fe94f82b)
>> drivers/crypto/inside-secure/safexcel_hash.c:1305:25: sparse: cast truncates 
>> bits from constant value (a54ff53a5f1d36f1 becomes 5f1d36f1)
>> drivers/crypto/inside-secure/safexcel_hash.c:1307:25: sparse: cast truncates 
>> bits from constant value (510e527fade682d1 becomes ade682d1)
>> drivers/crypto/inside-secure/safexcel_hash.c:1309:26: sparse: cast truncates 
>> bits from constant value (9b05688c2b3e6c1f becomes 2b3e6c1f)
>> drivers/crypto/inside-secure/safexcel_hash.c:1311:26: sparse: cast truncates 
>> bits from constant value (1f83d9abfb41bd6b becomes fb41bd6b)
>> drivers/crypto/inside-secure/safexcel_hash.c:1313:26: sparse: cast truncates 
>> bits from constant value (5be0cd19137e2179 becomes 137e2179)

vim +1299 drivers/crypto/inside-secure/safexcel_hash.c

  1291  
  1292  static int safexcel_sha512_init(struct ahash_request *areq)
  1293  {
  1294  struct safexcel_ahash_ctx *ctx = 
crypto_ahash_ctx(crypto_ahash_reqtfm(areq));
  1295  struct safexcel_ahash_req *req = ahash_request_ctx(areq);
  1296  
  1297  memset(req, 0, sizeof(*req));
  1298  
> 1299  req->state[0] = lower_32_bits(SHA512_H0);
  1300  req->state[1] = upper_32_bits(SHA512_H0);
> 1301  req->state[2] = lower_32_bits(SHA512_H1);
  1302  req->state[3] = upper_32_bits(SHA512_H1);
> 1303  req->state[4] = lower_32_bits(SHA512_H2);
  1304  req->state[5] = upper_32_bits(SHA512_H2);
> 1305  req->state[6] = lower_32_bits(SHA512_H3);
  1306  req->state[7] = upper_32_bits(SHA512_H3);
> 1307  req->state[8] = lower_32_bits(SHA512_H4);
  1308  req->state[9] = upper_32_bits(SHA512_H4);
> 1309  req->state[10] = lower_32_bits(SHA512_H5);
  1310  req->state[11] = upper_32_bits(SHA512_H5);
> 1311  req->state[12] = lower_32_bits(SHA512_H6);
  1312  req->state[13] = upper_32_bits(SHA512_H6);
> 1313  req->state[14] = lower_32_bits(SHA512_H7);
  1314  req->state[15] = upper_32_bits(SHA512_H7);
  1315  
  1316  ctx->alg = CONTEXT_CONTROL_CRYPTO_ALG_SHA512;
  1317  req->digest = CONTEXT_CONTROL_DIGEST_PRECOMPUTED;
  1318  req->state_sz = SHA512_DIGEST_SIZE;
  1319  
  1320  return 0;
  1321  }
  1322  

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

Re: [PATCH 1/2] hwrng: msm: add a spinlock and support for blocking reads

[Timur Tabi]

On 06/22/2018 01:03 PM, Timur Tabi wrote:



Perhaps it's because you implemented the 'wait' functionality in this
driver? Before the patch there wasn't any sort of wait check so we would
bail out if there wasn't any data even if the caller requested that we
wait for randomness to be available.


No, my tests showed the race condition (or at least something that looks 
like a race condition) even without the 'wait' feature.  I added support 
for 'wait' only recently, but I've been working with the crypto people 
for a month on everything else.


Looks like I was wrong.

I created some new tests to reproduce the problem, but I can't reproduce 
it any more.  I can only assume that what I saw as a race condition back 
then was something else entirely.


So all of the spinlock code needs to go.  It looks like at this point, 
if Vinod can add support for QCOM8160 to his patches, the only thing I 
can contribute is support for 'wait'.


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[cryptodev:master 9/15] drivers/crypto/inside-secure/safexcel_hash.c:1373:25: sparse: cast truncates bits from constant value (cbbb9d5dc1059ed8 becomes c1059ed8)

[kbuild test robot]

tree:   
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
head:   38641b83ebc54635151810eeef00b61da3097952
commit: 9e46eafdf82a67dd069eef27c48898b79379c9f2 [9/15] crypto: inside-secure - 
sha384 support
reproduce:
# apt-get install sparse
git checkout 9e46eafdf82a67dd069eef27c48898b79379c9f2
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/slab.h:631:13: sparse: undefined identifier 
'__builtin_mul_overflow'
   drivers/crypto/inside-secure/safexcel_hash.c:1009:45: sparse: cast to 
restricted __le32
   drivers/crypto/inside-secure/safexcel_hash.c:1010:45: sparse: cast to 
restricted __le32
   include/linux/slab.h:631:13: sparse: call with no type!
   drivers/crypto/inside-secure/safexcel_hash.c:1303:25: sparse: cast truncates 
bits from constant value (6a09e667f3bcc908 becomes f3bcc908)
   drivers/crypto/inside-secure/safexcel_hash.c:1305:25: sparse: cast truncates 
bits from constant value (bb67ae8584caa73b becomes 84caa73b)
   drivers/crypto/inside-secure/safexcel_hash.c:1307:25: sparse: cast truncates 
bits from constant value (3c6ef372fe94f82b becomes fe94f82b)
   drivers/crypto/inside-secure/safexcel_hash.c:1309:25: sparse: cast truncates 
bits from constant value (a54ff53a5f1d36f1 becomes 5f1d36f1)
   drivers/crypto/inside-secure/safexcel_hash.c:1311:25: sparse: cast truncates 
bits from constant value (510e527fade682d1 becomes ade682d1)
   drivers/crypto/inside-secure/safexcel_hash.c:1313:26: sparse: cast truncates 
bits from constant value (9b05688c2b3e6c1f becomes 2b3e6c1f)
   drivers/crypto/inside-secure/safexcel_hash.c:1315:26: sparse: cast truncates 
bits from constant value (1f83d9abfb41bd6b becomes fb41bd6b)
   drivers/crypto/inside-secure/safexcel_hash.c:1317:26: sparse: cast truncates 
bits from constant value (5be0cd19137e2179 becomes 137e2179)
>> drivers/crypto/inside-secure/safexcel_hash.c:1373:25: sparse: cast truncates 
>> bits from constant value (cbbb9d5dc1059ed8 becomes c1059ed8)
>> drivers/crypto/inside-secure/safexcel_hash.c:1375:25: sparse: cast truncates 
>> bits from constant value (629a292a367cd507 becomes 367cd507)
>> drivers/crypto/inside-secure/safexcel_hash.c:1377:25: sparse: cast truncates 
>> bits from constant value (9159015a3070dd17 becomes 3070dd17)
>> drivers/crypto/inside-secure/safexcel_hash.c:1379:25: sparse: cast truncates 
>> bits from constant value (152fecd8f70e5939 becomes f70e5939)
>> drivers/crypto/inside-secure/safexcel_hash.c:1381:25: sparse: cast truncates 
>> bits from constant value (67332667ffc00b31 becomes ffc00b31)
>> drivers/crypto/inside-secure/safexcel_hash.c:1383:26: sparse: cast truncates 
>> bits from constant value (8eb44a8768581511 becomes 68581511)
>> drivers/crypto/inside-secure/safexcel_hash.c:1385:26: sparse: cast truncates 
>> bits from constant value (db0c2e0d64f98fa7 becomes 64f98fa7)
>> drivers/crypto/inside-secure/safexcel_hash.c:1387:26: sparse: cast truncates 
>> bits from constant value (47b5481dbefa4fa4 becomes befa4fa4)

vim +1373 drivers/crypto/inside-secure/safexcel_hash.c

  1295  
  1296  static int safexcel_sha512_init(struct ahash_request *areq)
  1297  {
  1298  struct safexcel_ahash_ctx *ctx = 
crypto_ahash_ctx(crypto_ahash_reqtfm(areq));
  1299  struct safexcel_ahash_req *req = ahash_request_ctx(areq);
  1300  
  1301  memset(req, 0, sizeof(*req));
  1302  
> 1303  req->state[0] = lower_32_bits(SHA512_H0);
  1304  req->state[1] = upper_32_bits(SHA512_H0);
> 1305  req->state[2] = lower_32_bits(SHA512_H1);
  1306  req->state[3] = upper_32_bits(SHA512_H1);
> 1307  req->state[4] = lower_32_bits(SHA512_H2);
  1308  req->state[5] = upper_32_bits(SHA512_H2);
> 1309  req->state[6] = lower_32_bits(SHA512_H3);
  1310  req->state[7] = upper_32_bits(SHA512_H3);
> 1311  req->state[8] = lower_32_bits(SHA512_H4);
  1312  req->state[9] = upper_32_bits(SHA512_H4);
> 1313  req->state[10] = lower_32_bits(SHA512_H5);
  1314  req->state[11] = upper_32_bits(SHA512_H5);
> 1315  req->state[12] = lower_32_bits(SHA512_H6);
  1316  req->state[13] = upper_32_bits(SHA512_H6);
  1317  req->state[14] = lower_32_bits(SHA512_H7);
  1318  req->state[15] = upper_32_bits(SHA512_H7);
  1319  
  1320  ctx->alg = CONTEXT_CONTROL_CRYPTO_ALG_SHA512;
  1321  req->digest = CONTEXT_CONTROL_DIGEST_PRECOMPUTED;
  1322  req->state_sz = SHA512_DIGEST_SIZE;
  1323  
  1324  return 0;
  1325  }
  1326  
  1327  static int safexcel_sha512_digest(struct ahash_request *areq)
  1328  {
  1329  int ret = safexcel_sha512_init(areq);
  1330  
  1331  if (ret)
  1332  return ret;
  1333  
  1334  return safexcel_ahash_finup(areq);
  1335  }
  1336  
  1337  struct safexcel_alg_template safexcel_alg_sha512 = {
  1338 

Re: [PATCH 4/5] crypto: testmgr - Add test vectors for LZS compression

[Eric Biggers]

Hi Jan,

On Fri, Jun 22, 2018 at 04:37:21PM +0200, Jan Glauber wrote:
> The test vectors were generated using the ThunderX ZIP coprocessor.
> 
> Signed-off-by: Jan Glauber 
> ---
>  crypto/testmgr.c |  9 ++
>  crypto/testmgr.h | 77 
>  2 files changed, 86 insertions(+)
> 
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index cfb5fe4c5ccf..8e9ff1229e93 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -3238,6 +3238,15 @@ static const struct alg_test_desc alg_test_descs[] = {
>   .decomp = __VECS(lzo_decomp_tv_template)
>   }
>   }
> + }, {
> + .alg = "lzs",
> + .test = alg_test_comp,
> + .suite = {
> + .comp = {
> + .comp = __VECS(lzs_comp_tv_template),
> + .decomp = __VECS(lzs_decomp_tv_template)
> + }
> + }
>   }, {
>   .alg = "md4",
>   .test = alg_test_hash,
> diff --git a/crypto/testmgr.h b/crypto/testmgr.h
> index b950aa234e43..ae7fecadcade 100644
> --- a/crypto/testmgr.h
> +++ b/crypto/testmgr.h
> @@ -31699,6 +31699,83 @@ static const struct comp_testvec 
> lzo_decomp_tv_template[] = {
>   },
>  };
>  
> +/*
> + * LZS test vectors (null-terminated strings).
> + */
> +static const struct comp_testvec lzs_comp_tv_template[] = {
> + {
> + .inlen  = 70,
> + .outlen = 40,
> + .input  = "Join us now and share the software "
> + "Join us now and share the software ",
> + .output = "\x25\x1b\xcd\x26\xe1\x01\xd4\xe6"
> +   "\x20\x37\x1b\xce\xe2\x03\x09\xb8"
> +   "\xc8\x20\x39\x9a\x0c\x27\x23\x28"
> +   "\x80\xe8\x68\xc2\x07\x33\x79\x98"
> +   "\xe8\x77\xc6\xda\x3f\xfc\xc0\x00",
> + }, {
> + .inlen  = 184,
> + .outlen = 130,
> + .input  = "This document describes a compression method based 
> on the LZS "
> + "compression algorithm.  This document defines the 
> application of "
> + "the LZS algorithm to the IP Payload Compression 
> Protocol.",

Your comment claims that the test vectors (presumably the inputs) are
null-terminated strings, but the lengths of the inputs actually don't include
the null terminator.  The length of the first one, for example, would have to be
71 to include the null terminator, not 70.

Eric

Re: [PATCH 3/5] crypto: testmgr - Improve compression/decompression test

[Eric Biggers]

Hi Jan,

On Fri, Jun 22, 2018 at 04:37:20PM +0200, Jan Glauber wrote:
> While commit 336073840a87 ("crypto: testmgr - Allow different compression 
> results")
> allowed to test non-generic compression algorithms there are some corner
> cases that would not be detected in test_comp().
> 
> For example if input -> compression -> decompression would all yield
> the same bytes the test would still pass.
> 
> Improve the compression test by using the generic variant (if available)
> to decompress the compressed test vector from the non-generic
> algorithm.
> 
> Suggested-by: Herbert Xu 
> Signed-off-by: Jan Glauber 
> ---
>  crypto/testmgr.c | 23 ++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index d1d99843cce4..cfb5fe4c5ccf 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -1346,6 +1346,7 @@ static int test_comp(struct crypto_comp *tfm,
>int ctcount, int dtcount)
>  {

Any particular reason for not updating test_acomp() too?

>   const char *algo = crypto_tfm_alg_driver_name(crypto_comp_tfm(tfm));
> + const char *name = crypto_tfm_alg_name(crypto_comp_tfm(tfm));
>   char *output, *decomp_output;
>   unsigned int i;
>   int ret;
> @@ -1363,6 +1364,8 @@ static int test_comp(struct crypto_comp *tfm,
>   for (i = 0; i < ctcount; i++) {
>   int ilen;
>   unsigned int dlen = COMP_BUF_SIZE;
> + struct crypto_comp *tfm_decomp = NULL;
> + char *gname;
>  
>   memset(output, 0, sizeof(COMP_BUF_SIZE));
>   memset(decomp_output, 0, sizeof(COMP_BUF_SIZE));
> @@ -1377,9 +1380,27 @@ static int test_comp(struct crypto_comp *tfm,
>   goto out;
>   }
>  
> + /*
> +  * If compression of a non-generic algorithm was tested try to
> +  * decompress using the generic variant.
> +  */
> + if (!strstr(algo, "generic")) {

That's a pretty sloppy string comparison.  It matches "generic" anywhere in the
string, like "foogenericbar".  It should just be "-generic" at the end, right?
Like:

static bool is_generic_driver(const char *driver_name)
{
size_t len = strlen(driver_name);

return len >= 8 && !strcmp(&driver_name[len - 8], "-generic");
}

> + /* Construct name from cra_name + "-generic" */
> + gname = kmalloc(strlen(name) + 9, GFP_KERNEL);
> + strncpy(gname, name, strlen(name));
> + strncpy(gname + strlen(name), "-generic", 9);
> +
> + tfm_decomp = crypto_alloc_comp(gname, 0, 0);
> + kfree(gname);

If you're going to allocate memory here you need to check for error (note:
kasprintf() would make building the string a bit cleaner).  But algorithm names
are limited anyway, so a better way may be:

char generic_name[CRYPTO_MAX_ALG_NAME];

if (snprintf(generic_name, "%s-generic", name) <
sizeof(generic_name))
tfm_decomp = crypto_alloc_comp(gname, 0, 0);

> + }
> +
> + /* If there is no generic variant use the same tfm as before. */
> + if (!tfm_decomp || IS_ERR(tfm_decomp))
> + tfm_decomp = tfm;
> +

if (!IS_ERR_OR_NULL(tfm_decomp))

>   ilen = dlen;
>   dlen = COMP_BUF_SIZE;
> - ret = crypto_comp_decompress(tfm, output,
> + ret = crypto_comp_decompress(tfm_decomp, output,
>ilen, decomp_output, &dlen);

Shouldn't you decompress with both tfms, not just the generic one?

It's also weird that each 'struct comp_testvec' in 'ctemplate[]' has an
'output', but it's never used.  The issue seems to be that there are separate
test vectors for compression and decompression, but you really only need one
set.  It would have the '.uncompressed' and '.compressed' data.  From that, you
could compress the '.uncompressed' data with the tfm under test, and decompress
that result with both the tfm under test and the generic tfm.  Then, you could
decompress the '.compressed' data with the tfm under test and verify it matches
the '.uncompressed' data.  (I did something similar for symmetric ciphers in
commit 92a4c9fef34c.)

Thanks,

- Eric

Re: [PATCH 3/3] hwrng: msm - Add support for prng v2

[Stanimir Varbanov]

Hi Herbert,

On 06/21/2018 02:53 PM, Herbert Xu wrote:
> On Thu, Jun 21, 2018 at 02:27:10PM +0300, Stanimir Varbanov wrote:
>>
>> OK, I just wanted to say that it is _not_ PRNG and the register names
>> gives us wrong impression.
> 
> So does it generate one bit of output for each bit of hardware-
> generated entropy like /dev/random? Or does it use a hardware-
> generated seed to power a PRNG?

I believe it is the second one. Isn't the second one SP 800-90A?

-- 
regards,
Stan

Re: [PATCH 3/3] hwrng: msm - Add support for prng v2

[Herbert Xu]

On Fri, Jun 22, 2018 at 11:27:59AM +0300, Stanimir Varbanov wrote:
> Hi Herbert,
> 
> On 06/21/2018 02:53 PM, Herbert Xu wrote:
> > On Thu, Jun 21, 2018 at 02:27:10PM +0300, Stanimir Varbanov wrote:
> >>
> >> OK, I just wanted to say that it is _not_ PRNG and the register names
> >> gives us wrong impression.
> > 
> > So does it generate one bit of output for each bit of hardware-
> > generated entropy like /dev/random? Or does it use a hardware-
> > generated seed to power a PRNG?
> 
> I believe it is the second one. Isn't the second one SP 800-90A?

In that case it should switch over to algif_rng.

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

Re: [PATCH 3/3] hwrng: msm - Add support for prng v2

[Vinod]

On 22-06-18, 22:38, Herbert Xu wrote:
> On Fri, Jun 22, 2018 at 11:27:59AM +0300, Stanimir Varbanov wrote:
> > Hi Herbert,
> > 
> > On 06/21/2018 02:53 PM, Herbert Xu wrote:
> > > On Thu, Jun 21, 2018 at 02:27:10PM +0300, Stanimir Varbanov wrote:
> > >>
> > >> OK, I just wanted to say that it is _not_ PRNG and the register names
> > >> gives us wrong impression.
> > > 
> > > So does it generate one bit of output for each bit of hardware-
> > > generated entropy like /dev/random? Or does it use a hardware-
> > > generated seed to power a PRNG?
> > 
> > I believe it is the second one. Isn't the second one SP 800-90A?
> 
> In that case it should switch over to algif_rng.

Okay I am doing the port taking the exynos-rng as a ref.
Question is how to test it, how is one supposed to exercise the rng, any
test utils/apps for that? Sorry for noob question, new to crypto
interfaces.

-- 
~Vinod

Re: [PATCH 3/3] hwrng: msm - Add support for prng v2

[Herbert Xu]

On Fri, Jun 22, 2018 at 08:18:09PM +0530, Vinod wrote:
>
> Okay I am doing the port taking the exynos-rng as a ref.
> Question is how to test it, how is one supposed to exercise the rng, any
> test utils/apps for that? Sorry for noob question, new to crypto
> interfaces.

algif_rng is available through the af_alg socket interface.

Ccing Stephan as he has a library that may help you.

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

Crypto Fixes for 4.18

[Herbert Xu]

Hi Linus: 

This push fixes the following issues:

- Fix use after free in chtls.
- Fix RBP breakage in sha3.
- Fix use after free in hwrng_unregister.
- Fix overread in morus640.
- Move sleep out of kernel_neon in arm64/aes-blk.


Please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus


Dan Carpenter (1):
  crypto: chtls - use after free in chtls_pt_recvmsg()

Dmitry Vyukov (1):
  crypto: don't optimize keccakf()

Jia He (1):
  crypto: arm64/aes-blk - fix and move skcipher_walk_done out of 
kernel_neon_begin, _end

Michael Büsch (1):
  hwrng: core - Always drop the RNG in hwrng_unregister()

Ondrej Mosnáček (1):
  crypto: morus640 - Fix out-of-bounds access

 arch/arm64/crypto/aes-glue.c|2 +-
 crypto/morus640.c   |3 ++-
 crypto/sha3_generic.c   |2 +-
 drivers/char/hw_random/core.c   |   11 +--
 drivers/crypto/chelsio/chtls/chtls_io.c |5 ++---
 5 files changed, 15 insertions(+), 8 deletions(-)

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: inside-secure - increase minimum transfer size

[Herbert Xu]

On Mon, May 28, 2018 at 11:03:27AM +0200, Antoine Tenart wrote:
> From: Ofer Heifetz 
> 
> The token size was increased for AEAD support. Occasional authentication
> fails arise since the result descriptor overflows. This is because the
> token size and the engine minimal thresholds must be in sync.
> 
> Signed-off-by: Ofer Heifetz 
> 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][next] crypto: aegis: fix indentation of a statement

[Herbert Xu]

On Wed, May 30, 2018 at 11:50:05AM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Trival fix to correct the indentation of a single statement
> 
> Signed-off-by: Colin Ian King 

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

Re: [PATCH 3/3] hwrng: msm - Add support for prng v2

[Stanimir Varbanov]

Hi,

On 06/22/2018 05:38 PM, Herbert Xu wrote:
> On Fri, Jun 22, 2018 at 11:27:59AM +0300, Stanimir Varbanov wrote:
>> Hi Herbert,
>>
>> On 06/21/2018 02:53 PM, Herbert Xu wrote:
>>> On Thu, Jun 21, 2018 at 02:27:10PM +0300, Stanimir Varbanov wrote:

 OK, I just wanted to say that it is _not_ PRNG and the register names
 gives us wrong impression.
>>>
>>> So does it generate one bit of output for each bit of hardware-
>>> generated entropy like /dev/random? Or does it use a hardware-
>>> generated seed to power a PRNG?
>>
>> I believe it is the second one. Isn't the second one SP 800-90A?
> 
> In that case it should switch over to algif_rng.

OK, what about the rest drivers in drivers/char/hw_random? Are they all
true RNG?

-- 
regards,
Stan

[PATCH v18 2/7] parisc: iomap: introduce io{read|write}64

[Logan Gunthorpe]

Add support for io{read|write}64() functions in parisc architecture.
These are pretty straightforward copies of similar functions which
make use of readq and writeq.

Also, indicate that the lo_hi and hi_lo variants of these functions
are not provided by this architecture.

Signed-off-by: Logan Gunthorpe 
Reviewed-by: Andy Shevchenko 
Acked-by: Helge Deller 
Cc: "James E.J. Bottomley" 
Cc: Greg Kroah-Hartman 
Cc: Philippe Ombredanne 
Cc: Kate Stewart 
Cc: Thomas Gleixner 
---
 arch/parisc/include/asm/io.h |  9 +++
 arch/parisc/lib/iomap.c  | 64 
 2 files changed, 73 insertions(+)

diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index afe493b23d04..30a8315d5c07 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -311,6 +311,15 @@ extern void outsl (unsigned long port, const void *src, 
unsigned long count);
  * value for either 32 or 64 bit mode */
 #define F_EXTEND(x) ((unsigned long)((x) | (0xULL)))
 
+#define ioread64 ioread64
+#define ioread64be ioread64be
+#define iowrite64 iowrite64
+#define iowrite64be iowrite64be
+extern u64 ioread64(void __iomem *addr);
+extern u64 ioread64be(void __iomem *addr);
+extern void iowrite64(u64 val, void __iomem *addr);
+extern void iowrite64be(u64 val, void __iomem *addr);
+
 #include 
 
 /*
diff --git a/arch/parisc/lib/iomap.c b/arch/parisc/lib/iomap.c
index 4b19e6e64fb7..0195aec657e2 100644
--- a/arch/parisc/lib/iomap.c
+++ b/arch/parisc/lib/iomap.c
@@ -48,11 +48,15 @@ struct iomap_ops {
unsigned int (*read16be)(void __iomem *);
unsigned int (*read32)(void __iomem *);
unsigned int (*read32be)(void __iomem *);
+   u64 (*read64)(void __iomem *);
+   u64 (*read64be)(void __iomem *);
void (*write8)(u8, void __iomem *);
void (*write16)(u16, void __iomem *);
void (*write16be)(u16, void __iomem *);
void (*write32)(u32, void __iomem *);
void (*write32be)(u32, void __iomem *);
+   void (*write64)(u64, void __iomem *);
+   void (*write64be)(u64, void __iomem *);
void (*read8r)(void __iomem *, void *, unsigned long);
void (*read16r)(void __iomem *, void *, unsigned long);
void (*read32r)(void __iomem *, void *, unsigned long);
@@ -171,6 +175,16 @@ static unsigned int iomem_read32be(void __iomem *addr)
return __raw_readl(addr);
 }
 
+static u64 iomem_read64(void __iomem *addr)
+{
+   return readq(addr);
+}
+
+static u64 iomem_read64be(void __iomem *addr)
+{
+   return __raw_readq(addr);
+}
+
 static void iomem_write8(u8 datum, void __iomem *addr)
 {
writeb(datum, addr);
@@ -196,6 +210,16 @@ static void iomem_write32be(u32 datum, void __iomem *addr)
__raw_writel(datum, addr);
 }
 
+static void iomem_write64(u64 datum, void __iomem *addr)
+{
+   writel(datum, addr);
+}
+
+static void iomem_write64be(u64 datum, void __iomem *addr)
+{
+   __raw_writel(datum, addr);
+}
+
 static void iomem_read8r(void __iomem *addr, void *dst, unsigned long count)
 {
while (count--) {
@@ -250,11 +274,15 @@ static const struct iomap_ops iomem_ops = {
.read16be = iomem_read16be,
.read32 = iomem_read32,
.read32be = iomem_read32be,
+   .read64 = iomem_read64,
+   .read64be = iomem_read64be,
.write8 = iomem_write8,
.write16 = iomem_write16,
.write16be = iomem_write16be,
.write32 = iomem_write32,
.write32be = iomem_write32be,
+   .write64 = iomem_write64,
+   .write64be = iomem_write64be,
.read8r = iomem_read8r,
.read16r = iomem_read16r,
.read32r = iomem_read32r,
@@ -304,6 +332,20 @@ unsigned int ioread32be(void __iomem *addr)
return *((u32 *)addr);
 }
 
+u64 ioread64(void __iomem *addr)
+{
+   if (unlikely(INDIRECT_ADDR(addr)))
+   return iomap_ops[ADDR_TO_REGION(addr)]->read64(addr);
+   return le64_to_cpup((u64 *)addr);
+}
+
+u64 ioread64be(void __iomem *addr)
+{
+   if (unlikely(INDIRECT_ADDR(addr)))
+   return iomap_ops[ADDR_TO_REGION(addr)]->read64be(addr);
+   return *((u64 *)addr);
+}
+
 void iowrite8(u8 datum, void __iomem *addr)
 {
if (unlikely(INDIRECT_ADDR(addr))) {
@@ -349,6 +391,24 @@ void iowrite32be(u32 datum, void __iomem *addr)
}
 }
 
+void iowrite64(u64 datum, void __iomem *addr)
+{
+   if (unlikely(INDIRECT_ADDR(addr))) {
+   iomap_ops[ADDR_TO_REGION(addr)]->write64(datum, addr);
+   } else {
+   *((u64 *)addr) = cpu_to_le64(datum);
+   }
+}
+
+void iowrite64be(u64 datum, void __iomem *addr)
+{
+   if (unlikely(INDIRECT_ADDR(addr))) {
+   iomap_ops[ADDR_TO_REGION(addr)]->write64be(datum, addr);
+   } else {
+   *((u64 *)addr) = datum;
+   }
+}
+
 /* Repeating interfaces */
 
 void ioread8_rep(void __iomem *addr, void *dst, unsigned long count)
@@ -449,11 +509,15 @@ EXPORT_SYMBOL(iore

[PATCH v18 6/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Logan Gunthorpe]

Clean up the extra ifdefs which defined the wr_reg64 and rd_reg64
functions in non-64bit cases in favour of the new common
io-64-nonatomic-lo-hi header.

To be consistent with CAAM engine HW spec: in case of 64-bit registers,
irrespective of device endianness, the lower address should be read from
/ written to first, followed by the upper address. Indeed the I/O
accessors in CAAM driver currently don't follow the spec, however this
is a good opportunity to fix the code.

Signed-off-by: Logan Gunthorpe 
Reviewed-by: Horia Geantă 
Reviewed-by: Andy Shevchenko 
Cc: Dan Douglass 
Cc: Herbert Xu 
Cc: "David S. Miller" 
---
 drivers/crypto/caam/regs.h | 30 +++---
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 4fb91ba39c36..5826acd9194e 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -10,7 +10,7 @@
 
 #include 
 #include 
-#include 
+#include 
 
 /*
  * Architecture-specific register access methods
@@ -136,10 +136,9 @@ static inline void clrsetbits_32(void __iomem *reg, u32 
clear, u32 set)
  *base + 0x : least-significant 32 bits
  *base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
-   if (caam_little_end)
+   if (!caam_imx && caam_little_end)
iowrite64(data, reg);
else
iowrite64be(data, reg);
@@ -147,35 +146,12 @@ static inline void wr_reg64(void __iomem *reg, u64 data)
 
 static inline u64 rd_reg64(void __iomem *reg)
 {
-   if (caam_little_end)
+   if (!caam_imx && caam_little_end)
return ioread64(reg);
else
return ioread64be(reg);
 }
 
-#else /* CONFIG_64BIT */
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-   if (!caam_imx && caam_little_end) {
-   wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-   wr_reg32((u32 __iomem *)(reg), data);
-   } else {
-   wr_reg32((u32 __iomem *)(reg), data >> 32);
-   wr_reg32((u32 __iomem *)(reg) + 1, data);
-   }
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-   if (!caam_imx && caam_little_end)
-   return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-   (u64)rd_reg32((u32 __iomem *)(reg)));
-
-   return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-   (u64)rd_reg32((u32 __iomem *)(reg) + 1));
-}
-#endif /* CONFIG_64BIT  */
-
 static inline u64 cpu_to_caam_dma64(dma_addr_t value)
 {
if (caam_imx)
-- 
2.11.0

[PATCH v18 3/7] iomap: introduce io{read|write}64_{lo_hi|hi_lo}

[Logan Gunthorpe]

In order to provide non-atomic functions for io{read|write}64 that will
use readq and writeq when appropriate. We define a number of variants
of these functions in the generic iomap that will do non-atomic
operations on pio but atomic operations on mmio.

These functions are only defined if readq and writeq are defined. If
they are not, then the wrappers that always use non-atomic operations
from include/linux/io-64-nonatomic*.h will be used.

Signed-off-by: Logan Gunthorpe 
Reviewed-by: Andy Shevchenko 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Arnd Bergmann 
Cc: Suresh Warrier 
Cc: Nicholas Piggin 
---
 arch/powerpc/include/asm/io.h |   2 +
 include/asm-generic/iomap.h   |  26 +++--
 lib/iomap.c   | 132 ++
 3 files changed, 154 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index e0331e754568..20fe5d7515db 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -798,8 +798,10 @@ extern void __iounmap_at(void *ea, unsigned long size);
 
 #define mmio_read16be(addr)readw_be(addr)
 #define mmio_read32be(addr)readl_be(addr)
+#define mmio_read64be(addr)readq_be(addr)
 #define mmio_write16be(val, addr)  writew_be(val, addr)
 #define mmio_write32be(val, addr)  writel_be(val, addr)
+#define mmio_write64be(val, addr)  writeq_be(val, addr)
 #define mmio_insb(addr, dst, count)readsb(addr, dst, count)
 #define mmio_insw(addr, dst, count)readsw(addr, dst, count)
 #define mmio_insl(addr, dst, count)readsl(addr, dst, count)
diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index 5b63b94ef6b5..5a4af0199b32 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -31,9 +31,16 @@ extern unsigned int ioread16(void __iomem *);
 extern unsigned int ioread16be(void __iomem *);
 extern unsigned int ioread32(void __iomem *);
 extern unsigned int ioread32be(void __iomem *);
-#ifdef CONFIG_64BIT
-extern u64 ioread64(void __iomem *);
-extern u64 ioread64be(void __iomem *);
+
+#ifdef readq
+#define ioread64_lo_hi ioread64_lo_hi
+#define ioread64_hi_lo ioread64_hi_lo
+#define ioread64be_lo_hi ioread64be_lo_hi
+#define ioread64be_hi_lo ioread64be_hi_lo
+extern u64 ioread64_lo_hi(void __iomem *addr);
+extern u64 ioread64_hi_lo(void __iomem *addr);
+extern u64 ioread64be_lo_hi(void __iomem *addr);
+extern u64 ioread64be_hi_lo(void __iomem *addr);
 #endif
 
 extern void iowrite8(u8, void __iomem *);
@@ -41,9 +48,16 @@ extern void iowrite16(u16, void __iomem *);
 extern void iowrite16be(u16, void __iomem *);
 extern void iowrite32(u32, void __iomem *);
 extern void iowrite32be(u32, void __iomem *);
-#ifdef CONFIG_64BIT
-extern void iowrite64(u64, void __iomem *);
-extern void iowrite64be(u64, void __iomem *);
+
+#ifdef writeq
+#define iowrite64_lo_hi iowrite64_lo_hi
+#define iowrite64_hi_lo iowrite64_hi_lo
+#define iowrite64be_lo_hi iowrite64be_lo_hi
+#define iowrite64be_hi_lo iowrite64be_hi_lo
+extern void iowrite64_lo_hi(u64 val, void __iomem *addr);
+extern void iowrite64_hi_lo(u64 val, void __iomem *addr);
+extern void iowrite64be_lo_hi(u64 val, void __iomem *addr);
+extern void iowrite64be_hi_lo(u64 val, void __iomem *addr);
 #endif
 
 /*
diff --git a/lib/iomap.c b/lib/iomap.c
index 2c293b22569f..e909ab71e995 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -67,6 +67,7 @@ static void bad_io_access(unsigned long port, const char 
*access)
 #ifndef mmio_read16be
 #define mmio_read16be(addr) swab16(readw(addr))
 #define mmio_read32be(addr) swab32(readl(addr))
+#define mmio_read64be(addr) swab64(readq(addr))
 #endif
 
 unsigned int ioread8(void __iomem *addr)
@@ -100,6 +101,80 @@ EXPORT_SYMBOL(ioread16be);
 EXPORT_SYMBOL(ioread32);
 EXPORT_SYMBOL(ioread32be);
 
+#ifdef readq
+static u64 pio_read64_lo_hi(unsigned long port)
+{
+   u64 lo, hi;
+
+   lo = inl(port);
+   hi = inl(port + sizeof(u32));
+
+   return lo | (hi << 32);
+}
+
+static u64 pio_read64_hi_lo(unsigned long port)
+{
+   u64 lo, hi;
+
+   hi = inl(port + sizeof(u32));
+   lo = inl(port);
+
+   return lo | (hi << 32);
+}
+
+static u64 pio_read64be_lo_hi(unsigned long port)
+{
+   u64 lo, hi;
+
+   lo = pio_read32be(port + sizeof(u32));
+   hi = pio_read32be(port);
+
+   return lo | (hi << 32);
+}
+
+static u64 pio_read64be_hi_lo(unsigned long port)
+{
+   u64 lo, hi;
+
+   hi = pio_read32be(port);
+   lo = pio_read32be(port + sizeof(u32));
+
+   return lo | (hi << 32);
+}
+
+u64 ioread64_lo_hi(void __iomem *addr)
+{
+   IO_COND(addr, return pio_read64_lo_hi(port), return readq(addr));
+   return 0xULL;
+}
+
+u64 ioread64_hi_lo(void __iomem *addr)
+{
+   IO_COND(addr, return pio_read64_hi_lo(port), return readq(addr));
+   return 0xULL;
+}
+
+u64 ioread64be_lo_hi(void __iomem *addr)
+{
+  

[PATCH v18 4/7] io-64-nonatomic: add io{read|write}64[be]{_lo_hi|_hi_lo} macros

[Logan Gunthorpe]

This patch adds generic io{read|write}64[be]{_lo_hi|_hi_lo} macros if
they are not already defined by the architecture. (As they are provided
by the generic iomap library).

The patch also points io{read|write}64[be] to the variant specified by the
header name.

This is because new drivers are encouraged to use ioreadXX, et al instead
of readX[1], et al -- and mixing ioreadXX with readq is pretty ugly.

[1] LDD3: section 9.4.2

Signed-off-by: Logan Gunthorpe 
Reviewed-by: Andy Shevchenko 
Cc: Christoph Hellwig 
Cc: Arnd Bergmann 
Cc: Alan Cox 
Cc: Greg Kroah-Hartman 
---
 include/linux/io-64-nonatomic-hi-lo.h | 64 +++
 include/linux/io-64-nonatomic-lo-hi.h | 64 +++
 2 files changed, 128 insertions(+)

diff --git a/include/linux/io-64-nonatomic-hi-lo.h 
b/include/linux/io-64-nonatomic-hi-lo.h
index 862d786a904f..ae21b72cce85 100644
--- a/include/linux/io-64-nonatomic-hi-lo.h
+++ b/include/linux/io-64-nonatomic-hi-lo.h
@@ -55,4 +55,68 @@ static inline void hi_lo_writeq_relaxed(__u64 val, volatile 
void __iomem *addr)
 #define writeq_relaxed hi_lo_writeq_relaxed
 #endif
 
+#ifndef ioread64_hi_lo
+#define ioread64_hi_lo ioread64_hi_lo
+static inline u64 ioread64_hi_lo(void __iomem *addr)
+{
+   u32 low, high;
+
+   high = ioread32(addr + sizeof(u32));
+   low = ioread32(addr);
+
+   return low + ((u64)high << 32);
+}
+#endif
+
+#ifndef iowrite64_hi_lo
+#define iowrite64_hi_lo iowrite64_hi_lo
+static inline void iowrite64_hi_lo(u64 val, void __iomem *addr)
+{
+   iowrite32(val >> 32, addr + sizeof(u32));
+   iowrite32(val, addr);
+}
+#endif
+
+#ifndef ioread64be_hi_lo
+#define ioread64be_hi_lo ioread64be_hi_lo
+static inline u64 ioread64be_hi_lo(void __iomem *addr)
+{
+   u32 low, high;
+
+   high = ioread32be(addr);
+   low = ioread32be(addr + sizeof(u32));
+
+   return low + ((u64)high << 32);
+}
+#endif
+
+#ifndef iowrite64be_hi_lo
+#define iowrite64be_hi_lo iowrite64be_hi_lo
+static inline void iowrite64be_hi_lo(u64 val, void __iomem *addr)
+{
+   iowrite32be(val >> 32, addr);
+   iowrite32be(val, addr + sizeof(u32));
+}
+#endif
+
+#ifndef ioread64
+#define ioread64_is_nonatomic
+#define ioread64 ioread64_hi_lo
+#endif
+
+#ifndef iowrite64
+#define iowrite64_is_nonatomic
+#define iowrite64 iowrite64_hi_lo
+#endif
+
+#ifndef ioread64be
+#define ioread64be_is_nonatomic
+#define ioread64be ioread64be_hi_lo
+#endif
+
+#ifndef iowrite64be
+#define iowrite64be_is_nonatomic
+#define iowrite64be iowrite64be_hi_lo
+#endif
+
 #endif /* _LINUX_IO_64_NONATOMIC_HI_LO_H_ */
diff --git a/include/linux/io-64-nonatomic-lo-hi.h 
b/include/linux/io-64-nonatomic-lo-hi.h
index d042e7bb5adb..faaa842dbdb9 100644
--- a/include/linux/io-64-nonatomic-lo-hi.h
+++ b/include/linux/io-64-nonatomic-lo-hi.h
@@ -55,4 +55,68 @@ static inline void lo_hi_writeq_relaxed(__u64 val, volatile 
void __iomem *addr)
 #define writeq_relaxed lo_hi_writeq_relaxed
 #endif
 
+#ifndef ioread64_lo_hi
+#define ioread64_lo_hi ioread64_lo_hi
+static inline u64 ioread64_lo_hi(void __iomem *addr)
+{
+   u32 low, high;
+
+   low = ioread32(addr);
+   high = ioread32(addr + sizeof(u32));
+
+   return low + ((u64)high << 32);
+}
+#endif
+
+#ifndef iowrite64_lo_hi
+#define iowrite64_lo_hi iowrite64_lo_hi
+static inline void iowrite64_lo_hi(u64 val, void __iomem *addr)
+{
+   iowrite32(val, addr);
+   iowrite32(val >> 32, addr + sizeof(u32));
+}
+#endif
+
+#ifndef ioread64be_lo_hi
+#define ioread64be_lo_hi ioread64be_lo_hi
+static inline u64 ioread64be_lo_hi(void __iomem *addr)
+{
+   u32 low, high;
+
+   low = ioread32be(addr + sizeof(u32));
+   high = ioread32be(addr);
+
+   return low + ((u64)high << 32);
+}
+#endif
+
+#ifndef iowrite64be_lo_hi
+#define iowrite64be_lo_hi iowrite64be_lo_hi
+static inline void iowrite64be_lo_hi(u64 val, void __iomem *addr)
+{
+   iowrite32be(val, addr + sizeof(u32));
+   iowrite32be(val >> 32, addr);
+}
+#endif
+
+#ifndef ioread64
+#define ioread64_is_nonatomic
+#define ioread64 ioread64_lo_hi
+#endif
+
+#ifndef iowrite64
+#define iowrite64_is_nonatomic
+#define iowrite64 iowrite64_lo_hi
+#endif
+
+#ifndef ioread64be
+#define ioread64be_is_nonatomic
+#define ioread64be ioread64be_lo_hi
+#endif
+
+#ifndef iowrite64be
+#define iowrite64be_is_nonatomic
+#define iowrite64be iowrite64be_lo_hi
+#endif
+
 #endif /* _LINUX_IO_64_NONATOMIC_LO_HI_H_ */
-- 
2.11.0

[PATCH v18 0/7] Add io{read|write}64 to io-64-atomic headers

[Logan Gunthorpe]

This is a resend of my cleanup series to push a number of instances of
people defining their own io{read|write}64 functions into common
headers seing they don't exist in non-64bit systems. This series adds
inline functions to the io-64-nonatomic headers and then cleans up the
drivers that defined their own copies.

This cleanup was originally requested by Greg after he reviewed my
Switchtec NTB code.

@Andrew, can you please consider merging this series as it has a
number of cross-tree pieces? It has been around for a number of
cycles, has had some reviews, and a few small pieces of it have been
already accepted through various other trees.

This marks the one year aniversary since I started this series.

Thanks,

Logan

--

Changes since v17:
- Rebased onto v4.18-rc1 (No Changes)

Changes since v16:
- Rebased onto v4.17-rc4 (No Changes)

Changes since v15:
- Rebased onto v4.17-rc1, dropping the powerpc patches which were
  picked up by Michael

Changes since v14:
- Rebased onto v4.16-rc7
- Replace the first two patches so that instead of correcting the
  endianness annotations we change to using writeX() and readX() with
  swabX() calls. This makes the big-endian functions more symmetric
  with the little-endian versions (with respect to barriers that are
  not included in the raw functions). As a side effect, it also fixes
  the kbuild warnings that the first two patches tried to address.

Changes since v13:
- Changed the subject of patch 0001 to correct a nit pointed out by Luc

Changes since v12:
- Rebased onto v4.16-rc6
- Split patch 0001 into two and reworked the commit log as requested
  by Luc Van Oostenryck

Changes since v11:
- Rebased onto v4.16-rc5
- Added a patch (0001) to fix some old and new sparse warnings
  that the kbuild robot warned about this cycle. The latest version
  of sparse was required to reproduce these.
- Added a patch (0002) to add io{read|write}64 to parisc which the kbuild
  robot also found errors for this cycle

Changes since v10:
- Rebased onto v4.16-rc4, this droped the drm/tilcdc patch which was
  picked up by that tree and is already in 4.16.

Changes since v9:
- Rebased onto v4.15-rc6
- Fixed a couple of issues in the new version of the CAAM patch as
  pointed out by Horia

Changes since v8:
- Rebased onto v4.15-rc2, as a result rewrote patch 7 seeing someone did
  some similar cleanup in that area.
- Added a patch to clean up the Switchtec NTB driver which landed in
  v4.15-rc1

Changes since v7:
- Fix minor nits from Andy Shevchenko
- Rebased onto v4.14-rc1

Changes since v6:
 ** none **

Changes since v5:
- Added a fix to the tilcdc driver to ensure it doesn't use the
  non-atomic operation. (This includes adding io{read|write}64[be]_is_nonatomic
  defines).

Changes since v4:
- Add functions so the powerpc implementation of iomap.c compiles. (As
  noticed by Horia)

Changes since v3:

- I noticed powerpc didn't use the appropriate functions seeing
  readq/writeq were not defined when iomap.h was included. Thus I've
  included a patch to adjust this
- Fixed some mistakes with a couple of the defines in io-64-nonatomic*
  headers
- Fixed a typo noticed by Horia.

(earlier versions were drastically different)


Logan Gunthorpe (7):
  iomap: Use non-raw io functions for io{read|write}XXbe
  parisc: iomap: introduce io{read|write}64
  iomap: introduce io{read|write}64_{lo_hi|hi_lo}
  io-64-nonatomic: add io{read|write}64[be]{_lo_hi|_hi_lo} macros
  ntb: ntb_hw_intel: use io-64-nonatomic instead of in-driver hacks
  crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64
  ntb: ntb_hw_switchtec: Cleanup 64bit IO defines to use the common
header

 arch/parisc/include/asm/io.h   |   9 +++
 arch/parisc/lib/iomap.c|  64 +++
 arch/powerpc/include/asm/io.h  |   2 +
 drivers/crypto/caam/regs.h |  30 +--
 drivers/ntb/hw/intel/ntb_hw_intel.h|  30 +--
 drivers/ntb/hw/mscc/ntb_hw_switchtec.c |  36 +
 include/asm-generic/iomap.h|  26 --
 include/linux/io-64-nonatomic-hi-lo.h  |  64 +++
 include/linux/io-64-nonatomic-lo-hi.h  |  64 +++
 lib/iomap.c| 140 -
 10 files changed, 367 insertions(+), 98 deletions(-)

--
2.11.0

[PATCH v18 1/7] iomap: Use non-raw io functions for io{read|write}XXbe

[Logan Gunthorpe]

Fix an asymmetry in the io{read|write}XXbe functions in that the
big-endian variants make use of the raw io accessors while the
little-endian variants use the regular accessors. Some architectures
implement barriers to order against both spinlocks and DMA accesses
and for these case, the big-endian variant of the API would not be
protected.

Thus, change the mmio_be macros to use the appropriate swab() function
wrapping the regular accessor. This is similar to what was done for PIO.

When this code was originally written, barriers in the IO accessors were
not common and the accessors simply wrapped the raw functions in a
conversion to CPU endianness. Since then, barriers have been added in
some architectures and are now missing in the big endian variant of the
API.

This also manages to silence a few sparse warnings that check
for using the correct endian types which the original code did
not annotate correctly.

Signed-off-by: Logan Gunthorpe 
Cc: Thomas Gleixner 
Cc: Kate Stewart 
Cc: Philippe Ombredanne 
Cc: Greg Kroah-Hartman 
Cc: Arnd Bergmann 
Link: 
http://lkml.kernel.org/r/cak8p3a25zqdxyay3ivv+jmsszs7f6ssgc+hdbkgs54zfvix...@mail.gmail.com
---
 lib/iomap.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/iomap.c b/lib/iomap.c
index 541d926da95e..2c293b22569f 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -65,8 +65,8 @@ static void bad_io_access(unsigned long port, const char 
*access)
 #endif
 
 #ifndef mmio_read16be
-#define mmio_read16be(addr) be16_to_cpu(__raw_readw(addr))
-#define mmio_read32be(addr) be32_to_cpu(__raw_readl(addr))
+#define mmio_read16be(addr) swab16(readw(addr))
+#define mmio_read32be(addr) swab32(readl(addr))
 #endif
 
 unsigned int ioread8(void __iomem *addr)
@@ -106,8 +106,8 @@ EXPORT_SYMBOL(ioread32be);
 #endif
 
 #ifndef mmio_write16be
-#define mmio_write16be(val,port) __raw_writew(be16_to_cpu(val),port)
-#define mmio_write32be(val,port) __raw_writel(be32_to_cpu(val),port)
+#define mmio_write16be(val,port) writew(swab16(val),port)
+#define mmio_write32be(val,port) writel(swab32(val),port)
 #endif
 
 void iowrite8(u8 val, void __iomem *addr)
-- 
2.11.0

[PATCH v18 7/7] ntb: ntb_hw_switchtec: Cleanup 64bit IO defines to use the common header

[Logan Gunthorpe]

Clean up the ifdefs which conditionally defined the io{read|write}64
functions in favour of the new common io-64-nonatomic-lo-hi header.

Per a nit from Andy Shevchenko, the include list is also made
alphabetical.

Signed-off-by: Logan Gunthorpe 
Reviewed-by: Andy Shevchenko 
Cc: Jon Mason 
---
 drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 36 --
 1 file changed, 4 insertions(+), 32 deletions(-)

diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c 
b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
index f624ae27eabe..f403da24b833 100644
--- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
+++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
@@ -13,12 +13,13 @@
  *
  */
 
-#include 
-#include 
+#include 
+#include 
 #include 
 #include 
-#include 
+#include 
 #include 
+#include 
 
 MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
 MODULE_VERSION("0.1");
@@ -35,35 +36,6 @@ module_param(use_lut_mws, bool, 0644);
 MODULE_PARM_DESC(use_lut_mws,
 "Enable the use of the LUT based memory windows");
 
-#ifndef ioread64
-#ifdef readq
-#define ioread64 readq
-#else
-#define ioread64 _ioread64
-static inline u64 _ioread64(void __iomem *mmio)
-{
-   u64 low, high;
-
-   low = ioread32(mmio);
-   high = ioread32(mmio + sizeof(u32));
-   return low | (high << 32);
-}
-#endif
-#endif
-
-#ifndef iowrite64
-#ifdef writeq
-#define iowrite64 writeq
-#else
-#define iowrite64 _iowrite64
-static inline void _iowrite64(u64 val, void __iomem *mmio)
-{
-   iowrite32(val, mmio);
-   iowrite32(val >> 32, mmio + sizeof(u32));
-}
-#endif
-#endif
-
 #define SWITCHTEC_NTB_MAGIC 0x45CC0001
 #define MAX_MWS 128
 
-- 
2.11.0

[PATCH v18 5/7] ntb: ntb_hw_intel: use io-64-nonatomic instead of in-driver hacks

[Logan Gunthorpe]

Now that ioread64 and iowrite64 are available in io-64-nonatomic,
we can remove the hack at the top of ntb_hw_intel.c and replace it
with an include.

Signed-off-by: Logan Gunthorpe 
Reviewed-by: Andy Shevchenko 
Acked-by: Dave Jiang 
Acked-by: Allen Hubbe 
Acked-by: Jon Mason 
---
 drivers/ntb/hw/intel/ntb_hw_intel.h | 30 +-
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.h 
b/drivers/ntb/hw/intel/ntb_hw_intel.h
index c49ff8970ce3..e071e28bca3f 100644
--- a/drivers/ntb/hw/intel/ntb_hw_intel.h
+++ b/drivers/ntb/hw/intel/ntb_hw_intel.h
@@ -53,6 +53,7 @@
 
 #include 
 #include 
+#include 
 
 /* PCI device IDs */
 #define PCI_DEVICE_ID_INTEL_NTB_B2B_JSF0x3725
@@ -218,33 +219,4 @@ static inline int pdev_is_gen3(struct pci_dev *pdev)
return 0;
 }
 
-#ifndef ioread64
-#ifdef readq
-#define ioread64 readq
-#else
-#define ioread64 _ioread64
-static inline u64 _ioread64(void __iomem *mmio)
-{
-   u64 low, high;
-
-   low = ioread32(mmio);
-   high = ioread32(mmio + sizeof(u32));
-   return low | (high << 32);
-}
-#endif
-#endif
-
-#ifndef iowrite64
-#ifdef writeq
-#define iowrite64 writeq
-#else
-#define iowrite64 _iowrite64
-static inline void _iowrite64(u64 val, void __iomem *mmio)
-{
-   iowrite32(val, mmio);
-   iowrite32(val >> 32, mmio + sizeof(u32));
-}
-#endif
-#endif
-
 #endif
-- 
2.11.0

[RFC PATCH 0/2] Fix GCC -Wstringop-truncation warnings

[Stafford Horne]

Hello,

When compiling OpenRISC kernels with our new toolchain based on 9.0.0 I am
seeing various -Wstringop-truncation warnings.  There might be more as I am not
compiling all drivers/modules yet, if someone thinks thats helpful let me know.

I discussed this with Greg KH at the OSS Summit Japan yesterday and it seems no
one has pointed these out yet, so here are the patches.

Please let me know if I should keep these on the OpenRISC tree (which I can do
for 4.18) or if the maintainers will pick them up separately.

-Stafford

Stafford Horne (2):
  crypto: Fix -Wstringop-truncation warnings
  kobject: Fix -Wstringop-truncation warning

 crypto/ablkcipher.c | 4 ++--
 crypto/blkcipher.c  | 2 +-
 lib/kobject.c   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.17.0

[RFC PATCH 2/2] kobject: Fix -Wstringop-truncation warning

[Stafford Horne]

When compiling with GCC 9.0.0 I am seeing the following warning:

In function ‘fill_kobj_path’,
inlined from ‘kobject_get_path’ at lib/kobject.c:155:2:
lib/kobject.c:128:3: warning: ‘strncpy’ output truncated before terminating 
nul copying as many bytes from a string as its length [-Wstringop-truncation]
   strncpy(path + length, kobject_name(parent), cur);
   ^
lib/kobject.c: In function ‘kobject_get_path’:
lib/kobject.c:125:13: note: length computed here
   int cur = strlen(kobject_name(parent));
 ^~~~

This is pointing out a bug that the strncpy limit is the source string not the
destination buffer remaining length.  Fix it.

Cc: Greg Kroah-Hartman 
Cc: Arnd Bergmann 
Signed-off-by: Stafford Horne 
---
 lib/kobject.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index 18989b5b3b56..15338e5a96f2 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -125,7 +125,7 @@ static void fill_kobj_path(struct kobject *kobj, char 
*path, int length)
int cur = strlen(kobject_name(parent));
/* back up enough to print this name with '/' */
length -= cur;
-   strncpy(path + length, kobject_name(parent), cur);
+   strncpy(path + length, kobject_name(parent), length);
*(path + --length) = '/';
}
 
-- 
2.17.0

[RFC PATCH 1/2] crypto: Fix -Wstringop-truncation warnings

[Stafford Horne]

As of GCC 9.0.0 the build is reporting warnings like:

crypto/ablkcipher.c: In function ‘crypto_ablkcipher_report’:
crypto/ablkcipher.c:374:2: warning: ‘strncpy’ specified bound 64 equals 
destination size [-Wstringop-truncation]
  strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
  ^~~
   sizeof(rblkcipher.geniv));
   ~

This means the strnycpy might create a non null terminated string.  Fix this by
limiting the size of the string copy to include the null terminator.

Cc: Greg Kroah-Hartman 
Cc: Arnd Bergmann 
Signed-off-by: Stafford Horne 
---
 crypto/ablkcipher.c | 4 ++--
 crypto/blkcipher.c  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index d880a4897159..972cd7c879f6 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -372,7 +372,7 @@ static int crypto_ablkcipher_report(struct sk_buff *skb, 
struct crypto_alg *alg)
 
strncpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type));
strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
-   sizeof(rblkcipher.geniv));
+   sizeof(rblkcipher.geniv) - 1);
 
rblkcipher.blocksize = alg->cra_blocksize;
rblkcipher.min_keysize = alg->cra_ablkcipher.min_keysize;
@@ -446,7 +446,7 @@ static int crypto_givcipher_report(struct sk_buff *skb, 
struct crypto_alg *alg)
 
strncpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type));
strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
-   sizeof(rblkcipher.geniv));
+   sizeof(rblkcipher.geniv) - 1);
 
rblkcipher.blocksize = alg->cra_blocksize;
rblkcipher.min_keysize = alg->cra_ablkcipher.min_keysize;
diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
index 01c0d4aa2563..f1644b5cf68c 100644
--- a/crypto/blkcipher.c
+++ b/crypto/blkcipher.c
@@ -511,7 +511,7 @@ static int crypto_blkcipher_report(struct sk_buff *skb, 
struct crypto_alg *alg)
 
strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type));
strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "",
-   sizeof(rblkcipher.geniv));
+   sizeof(rblkcipher.geniv) - 1);
 
rblkcipher.blocksize = alg->cra_blocksize;
rblkcipher.min_keysize = alg->cra_blkcipher.min_keysize;
-- 
2.17.0

Re: [RFC PATCH 1/2] crypto: Fix -Wstringop-truncation warnings

[Max Filippov]

On Fri, Jun 22, 2018 at 7:07 PM, Stafford Horne  wrote:
> As of GCC 9.0.0 the build is reporting warnings like:
>
> crypto/ablkcipher.c: In function ‘crypto_ablkcipher_report’:
> crypto/ablkcipher.c:374:2: warning: ‘strncpy’ specified bound 64 equals 
> destination size [-Wstringop-truncation]
>   strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
>   ^~~
>sizeof(rblkcipher.geniv));
>~
>
> This means the strnycpy might create a non null terminated string.  Fix this 
> by
> limiting the size of the string copy to include the null terminator.

That could work if the destination buffer was zero-initialized,
but it's allocated on stack and is not initialized.
Replacing strncpy with strlcpy without changing its arguments
should do the right thing.

-- 
Thanks.
-- Max

Re: [RFC PATCH 2/2] kobject: Fix -Wstringop-truncation warning

[Eric Biggers]

On Sat, Jun 23, 2018 at 11:07:53AM +0900, Stafford Horne wrote:
> When compiling with GCC 9.0.0 I am seeing the following warning:
> 
> In function ‘fill_kobj_path’,
>   inlined from ‘kobject_get_path’ at lib/kobject.c:155:2:
> lib/kobject.c:128:3: warning: ‘strncpy’ output truncated before 
> terminating nul copying as many bytes from a string as its length 
> [-Wstringop-truncation]
>strncpy(path + length, kobject_name(parent), cur);
>^
> lib/kobject.c: In function ‘kobject_get_path’:
> lib/kobject.c:125:13: note: length computed here
>int cur = strlen(kobject_name(parent));
>^~~~
> 
> This is pointing out a bug that the strncpy limit is the source string not the
> destination buffer remaining length.  Fix it.
> 
> Cc: Greg Kroah-Hartman 
> Cc: Arnd Bergmann 
> Signed-off-by: Stafford Horne 
> ---
>  lib/kobject.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index 18989b5b3b56..15338e5a96f2 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -125,7 +125,7 @@ static void fill_kobj_path(struct kobject *kobj, char 
> *path, int length)
>   int cur = strlen(kobject_name(parent));
>   /* back up enough to print this name with '/' */
>   length -= cur;
> - strncpy(path + length, kobject_name(parent), cur);
> + strncpy(path + length, kobject_name(parent), length);
>   *(path + --length) = '/';
>   }

It should be replaced with memcpy(), AFAICS; it wouldn't change the behavior but
it would quiet the gcc warning.  Your proposed "fix" is heavily broken: notice
that the code is building a string backwards (end to beginning).

- Eric

Re: [RFC PATCH 1/2] crypto: Fix -Wstringop-truncation warnings

[Eric Biggers]

On Sat, Jun 23, 2018 at 11:07:52AM +0900, Stafford Horne wrote:
> As of GCC 9.0.0 the build is reporting warnings like:
> 
> crypto/ablkcipher.c: In function ‘crypto_ablkcipher_report’:
> crypto/ablkcipher.c:374:2: warning: ‘strncpy’ specified bound 64 equals 
> destination size [-Wstringop-truncation]
>   strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
>   ^~~
>sizeof(rblkcipher.geniv));
>~
> 
> This means the strnycpy might create a non null terminated string.  Fix this 
> by
> limiting the size of the string copy to include the null terminator.
> 
> Cc: Greg Kroah-Hartman 
> Cc: Arnd Bergmann 
> Signed-off-by: Stafford Horne 
> ---
>  crypto/ablkcipher.c | 4 ++--
>  crypto/blkcipher.c  | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
> index d880a4897159..972cd7c879f6 100644
> --- a/crypto/ablkcipher.c
> +++ b/crypto/ablkcipher.c
> @@ -372,7 +372,7 @@ static int crypto_ablkcipher_report(struct sk_buff *skb, 
> struct crypto_alg *alg)
>  
>   strncpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type));
>   strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
> - sizeof(rblkcipher.geniv));
> + sizeof(rblkcipher.geniv) - 1);
>  
>   rblkcipher.blocksize = alg->cra_blocksize;
>   rblkcipher.min_keysize = alg->cra_ablkcipher.min_keysize;
> @@ -446,7 +446,7 @@ static int crypto_givcipher_report(struct sk_buff *skb, 
> struct crypto_alg *alg)
>  
>   strncpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type));
>   strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
> - sizeof(rblkcipher.geniv));
> + sizeof(rblkcipher.geniv) - 1);
>  
>   rblkcipher.blocksize = alg->cra_blocksize;
>   rblkcipher.min_keysize = alg->cra_ablkcipher.min_keysize;
> diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
> index 01c0d4aa2563..f1644b5cf68c 100644
> --- a/crypto/blkcipher.c
> +++ b/crypto/blkcipher.c
> @@ -511,7 +511,7 @@ static int crypto_blkcipher_report(struct sk_buff *skb, 
> struct crypto_alg *alg)
>  
>   strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type));
>   strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "",
> - sizeof(rblkcipher.geniv));
> + sizeof(rblkcipher.geniv) - 1);
>  
>   rblkcipher.blocksize = alg->cra_blocksize;
>   rblkcipher.min_keysize = alg->cra_blkcipher.min_keysize;

Your "fix" introduces an information disclosure bug, as it results in
uninitialized memory being copied to userspace.  This same broken patch was sent
by someone else too.

Maybe it would be best to just memset() the crypto_report_* structs to 0 after
declaration and then replace the strncpy()'s with strscpy()'s, even if just to
stop people from sending broken "fixes".  Do you want to do that?

- Eric

Re: [RFC PATCH 2/2] kobject: Fix -Wstringop-truncation warning

[Stafford Horne]

On Fri, Jun 22, 2018 at 07:31:49PM -0700, Eric Biggers wrote:
> On Sat, Jun 23, 2018 at 11:07:53AM +0900, Stafford Horne wrote:
> > When compiling with GCC 9.0.0 I am seeing the following warning:
> > 
> > In function ‘fill_kobj_path’,
> > inlined from ‘kobject_get_path’ at lib/kobject.c:155:2:
> > lib/kobject.c:128:3: warning: ‘strncpy’ output truncated before 
> > terminating nul copying as many bytes from a string as its length 
> > [-Wstringop-truncation]
> >strncpy(path + length, kobject_name(parent), cur);
> >^
> > lib/kobject.c: In function ‘kobject_get_path’:
> > lib/kobject.c:125:13: note: length computed here
> >int cur = strlen(kobject_name(parent));
> >  ^~~~
> > 
> > This is pointing out a bug that the strncpy limit is the source string not 
> > the
> > destination buffer remaining length.  Fix it.
> > 
> > Cc: Greg Kroah-Hartman 
> > Cc: Arnd Bergmann 
> > Signed-off-by: Stafford Horne 
> > ---
> >  lib/kobject.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/kobject.c b/lib/kobject.c
> > index 18989b5b3b56..15338e5a96f2 100644
> > --- a/lib/kobject.c
> > +++ b/lib/kobject.c
> > @@ -125,7 +125,7 @@ static void fill_kobj_path(struct kobject *kobj, char 
> > *path, int length)
> > int cur = strlen(kobject_name(parent));
> > /* back up enough to print this name with '/' */
> > length -= cur;
> > -   strncpy(path + length, kobject_name(parent), cur);
> > +   strncpy(path + length, kobject_name(parent), length);
> > *(path + --length) = '/';
> > }
> 
> It should be replaced with memcpy(), AFAICS; it wouldn't change the behavior 
> but
> it would quiet the gcc warning.  Your proposed "fix" is heavily broken: notice
> that the code is building a string backwards (end to beginning).

Sorry about that, I didnt notice.  I will fix.

-Stafford

Re: [RFC PATCH 1/2] crypto: Fix -Wstringop-truncation warnings

[Stafford Horne]

On Fri, Jun 22, 2018 at 07:41:49PM -0700, Eric Biggers wrote:
> On Sat, Jun 23, 2018 at 11:07:52AM +0900, Stafford Horne wrote:
> > As of GCC 9.0.0 the build is reporting warnings like:
> > 
> > crypto/ablkcipher.c: In function ‘crypto_ablkcipher_report’:
> > crypto/ablkcipher.c:374:2: warning: ‘strncpy’ specified bound 64 equals 
> > destination size [-Wstringop-truncation]
> >   strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
> >   ^~~
> >sizeof(rblkcipher.geniv));
> >~
> > 
> > This means the strnycpy might create a non null terminated string.  Fix 
> > this by
> > limiting the size of the string copy to include the null terminator.
> > 
> > Cc: Greg Kroah-Hartman 
> > Cc: Arnd Bergmann 
> > Signed-off-by: Stafford Horne 
> > ---
> >  crypto/ablkcipher.c | 4 ++--
> >  crypto/blkcipher.c  | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
> > index d880a4897159..972cd7c879f6 100644
> > --- a/crypto/ablkcipher.c
> > +++ b/crypto/ablkcipher.c
> > @@ -372,7 +372,7 @@ static int crypto_ablkcipher_report(struct sk_buff 
> > *skb, struct crypto_alg *alg)
> >  
> > strncpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type));
> > strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
> > -   sizeof(rblkcipher.geniv));
> > +   sizeof(rblkcipher.geniv) - 1);
> >  
> > rblkcipher.blocksize = alg->cra_blocksize;
> > rblkcipher.min_keysize = alg->cra_ablkcipher.min_keysize;
> > @@ -446,7 +446,7 @@ static int crypto_givcipher_report(struct sk_buff *skb, 
> > struct crypto_alg *alg)
> >  
> > strncpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type));
> > strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "",
> > -   sizeof(rblkcipher.geniv));
> > +   sizeof(rblkcipher.geniv) - 1);
> >  
> > rblkcipher.blocksize = alg->cra_blocksize;
> > rblkcipher.min_keysize = alg->cra_ablkcipher.min_keysize;
> > diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
> > index 01c0d4aa2563..f1644b5cf68c 100644
> > --- a/crypto/blkcipher.c
> > +++ b/crypto/blkcipher.c
> > @@ -511,7 +511,7 @@ static int crypto_blkcipher_report(struct sk_buff *skb, 
> > struct crypto_alg *alg)
> >  
> > strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type));
> > strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "",
> > -   sizeof(rblkcipher.geniv));
> > +   sizeof(rblkcipher.geniv) - 1);
> >  
> > rblkcipher.blocksize = alg->cra_blocksize;
> > rblkcipher.min_keysize = alg->cra_blkcipher.min_keysize;
> 
> Your "fix" introduces an information disclosure bug, as it results in
> uninitialized memory being copied to userspace.  This same broken patch was 
> sent
> by someone else too.
> 
> Maybe it would be best to just memset() the crypto_report_* structs to 0 after
> declaration and then replace the strncpy()'s with strscpy()'s, even if just to
> stop people from sending broken "fixes".  Do you want to do that?

Right, I didnt realize that we were using strncpy to also init the whole buffer.

I will do as suggest, and respic.

-Stafford

Re: [RFC PATCH 1/2] crypto: Fix -Wstringop-truncation warnings

[Stafford Horne]

On Sat, Jun 23, 2018 at 11:52:56AM +0900, Stafford Horne wrote:
> On Fri, Jun 22, 2018 at 07:41:49PM -0700, Eric Biggers wrote:
> > On Sat, Jun 23, 2018 at 11:07:52AM +0900, Stafford Horne wrote:
> > > As of GCC 9.0.0 the build is reporting warnings like:
> > > 
[...]
> > >   strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type));
> > >   strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "",
> > > - sizeof(rblkcipher.geniv));
> > > + sizeof(rblkcipher.geniv) - 1);
> > >  
> > >   rblkcipher.blocksize = alg->cra_blocksize;
> > >   rblkcipher.min_keysize = alg->cra_blkcipher.min_keysize;
> > 
> > Your "fix" introduces an information disclosure bug, as it results in
> > uninitialized memory being copied to userspace.  This same broken patch was 
> > sent
> > by someone else too.
> > 
> > Maybe it would be best to just memset() the crypto_report_* structs to 0 
> > after
> > declaration and then replace the strncpy()'s with strscpy()'s, even if just 
> > to
> > stop people from sending broken "fixes".  Do you want to do that?
> 
> Right, I didnt realize that we were using strncpy to also init the whole 
> buffer.
> 
> I will do as suggest, and respin.

Hi Eric,

I thought about this a bit, doing memset() and strscpy() seemed fine, but the
below also would work, be a bit faster and stop gcc form complaining.  What do
you think?

@@ -512,6 +512,7 @@ static int crypto_blkcipher_report(struct sk_buff *skb, 
struct crypto_alg *alg)
strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type));
strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "",
sizeof(rblkcipher.geniv));
+   rblkcipher.geniv[sizeof(rblkcipher.geniv) - 1] = '\0';

rblkcipher.blocksize = alg->cra_blocksize;
rblkcipher.min_keysize = alg->cra_blkcipher.min_keysize;

Let me know what you think.

-Stafford