crypto: rsa - Do not gratuitously drop leading zeroes

crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

This was prompted by the caam RSA submission where a lot of work
was done just to strip the RSA output of leading zeroes.  This is
in fact completely pointless because the only user of RSA in the
kernel then promptly puts them back.

This patch series resolves this madness by simply leaving any
leading zeroes in place.  Note that we're not requiring authors
to add leading zeroes, even though that is encouraged if it is
easy to do.  In practice you'd only run into this every 2^32 or
2^64 operations so please don't overdo it.

I've also taken the opportunity to cleanup the pkcs1pad code.

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

[PATCH 1/8] crypto: testmgr - Allow leading zeros in RSA

[Herbert Xu]

This patch allows RSA implementations to produce output with
leading zeroes.  testmgr will skip leading zeroes when comparing
the output.

This patch also tries to make the RSA test function generic enough
to potentially handle other akcipher algorithms.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/testmgr.c |   51 ++++++++++++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index b773a56..f4982da 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1777,8 +1777,8 @@ static int alg_test_drbg(const struct alg_test_desc *desc, const char *driver,
 
 }
 
-static int do_test_rsa(struct crypto_akcipher *tfm,
-		       struct akcipher_testvec *vecs)
+static int test_akcipher_one(struct crypto_akcipher *tfm,
+			     struct akcipher_testvec *vecs)
 {
 	char *xbuf[XBUFSIZE];
 	struct akcipher_request *req;
@@ -1829,17 +1829,18 @@ static int do_test_rsa(struct crypto_akcipher *tfm,
 	/* Run RSA encrypt - c = m^e mod n;*/
 	err = wait_async_op(&result, crypto_akcipher_encrypt(req));
 	if (err) {
-		pr_err("alg: rsa: encrypt test failed. err %d\n", err);
+		pr_err("alg: akcipher: encrypt test failed. err %d\n", err);
 		goto free_all;
 	}
 	if (req->dst_len != vecs->c_size) {
-		pr_err("alg: rsa: encrypt test failed. Invalid output len\n");
+		pr_err("alg: akcipher: encrypt test failed. Invalid output len\n");
 		err = -EINVAL;
 		goto free_all;
 	}
 	/* verify that encrypted message is equal to expected */
 	if (memcmp(vecs->c, outbuf_enc, vecs->c_size)) {
-		pr_err("alg: rsa: encrypt test failed. Invalid output\n");
+		pr_err("alg: akcipher: encrypt test failed. Invalid output\n");
+		hexdump(outbuf_enc, vecs->c_size);
 		err = -EINVAL;
 		goto free_all;
 	}
@@ -1867,18 +1868,22 @@ static int do_test_rsa(struct crypto_akcipher *tfm,
 	/* Run RSA decrypt - m = c^d mod n;*/
 	err = wait_async_op(&result, crypto_akcipher_decrypt(req));
 	if (err) {
-		pr_err("alg: rsa: decrypt test failed. err %d\n", err);
+		pr_err("alg: akcipher: decrypt test failed. err %d\n", err);
 		goto free_all;
 	}
 	out_len = req->dst_len;
-	if (out_len != vecs->m_size) {
-		pr_err("alg: rsa: decrypt test failed. Invalid output len\n");
+	if (out_len < vecs->m_size) {
+		pr_err("alg: akcipher: decrypt test failed. "
+		       "Invalid output len %u\n", out_len);
 		err = -EINVAL;
 		goto free_all;
 	}
 	/* verify that decrypted message is equal to the original msg */
-	if (memcmp(vecs->m, outbuf_dec, vecs->m_size)) {
-		pr_err("alg: rsa: decrypt test failed. Invalid output\n");
+	if (memchr_inv(outbuf_dec, 0, out_len - vecs->m_size) ||
+	    memcmp(vecs->m, outbuf_dec + out_len - vecs->m_size,
+		   vecs->m_size)) {
+		pr_err("alg: akcipher: decrypt test failed. Invalid output\n");
+		hexdump(outbuf_dec, out_len);
 		err = -EINVAL;
 	}
 free_all:
@@ -1891,28 +1896,20 @@ free_xbuf:
 	return err;
 }
 
-static int test_rsa(struct crypto_akcipher *tfm, struct akcipher_testvec *vecs,
-		    unsigned int tcount)
+static int test_akcipher(struct crypto_akcipher *tfm, const char *alg,
+			 struct akcipher_testvec *vecs, unsigned int tcount)
 {
 	int ret, i;
 
 	for (i = 0; i < tcount; i++) {
-		ret = do_test_rsa(tfm, vecs++);
-		if (ret) {
-			pr_err("alg: rsa: test failed on vector %d, err=%d\n",
-			       i + 1, ret);
-			return ret;
-		}
-	}
-	return 0;
-}
-
-static int test_akcipher(struct crypto_akcipher *tfm, const char *alg,
-			 struct akcipher_testvec *vecs, unsigned int tcount)
-{
-	if (strncmp(alg, "rsa", 3) == 0)
-		return test_rsa(tfm, vecs, tcount);
+		ret = test_akcipher_one(tfm, vecs++);
+		if (!ret)
+			continue;
 
+		pr_err("alg: akcipher: test failed on vector %d, err=%d\n",
+		       i + 1, ret);
+		return ret;
+	}
 	return 0;
 }
 

[PATCH 2/8] crypto: rsa - Generate fixed-length output

[Herbert Xu]

Every implementation of RSA that we have naturally generates
output with leading zeroes.  The one and only user of RSA,
pkcs1pad wants to have those leading zeroes in place, in fact
because they are currently absent it has to write those zeroes
itself.

So we shouldn't be stripping leading zeroes in the first place.
In fact this patch makes rsa-generic produce output with fixed
length so that pkcs1pad does not need to do any extra work.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa.c        |    8 +++----
 include/linux/mpi.h |    2 -
 lib/mpi/mpicoder.c  |   55 ++++++++++++++++++++++++----------------------------
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/crypto/rsa.c b/crypto/rsa.c
index dc692d4..4c280b6 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -108,7 +108,7 @@ static int rsa_enc(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(c, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(c, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -147,7 +147,7 @@ static int rsa_dec(struct akcipher_request *req)
 	if (ret)
 		goto err_free_c;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_c;
 
@@ -185,7 +185,7 @@ static int rsa_sign(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(s, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(s, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -226,7 +226,7 @@ static int rsa_verify(struct akcipher_request *req)
 	if (ret)
 		goto err_free_s;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_s;
 
diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index f219559..1cc5ffb 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -80,7 +80,7 @@ void *mpi_get_buffer(MPI a, unsigned *nbytes, int *sign);
 int mpi_read_buffer(MPI a, uint8_t *buf, unsigned buf_len, unsigned *nbytes,
 		    int *sign);
 void *mpi_get_secure_buffer(MPI a, unsigned *nbytes, int *sign);
-int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned nbytes,
 		     int *sign);
 
 #define log_mpidump g10_log_mpidump
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 823cf5f..7150e5c 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -237,16 +237,13 @@ EXPORT_SYMBOL_GPL(mpi_get_buffer);
  * @a:		a multi precision integer
  * @sgl:	scatterlist to write to. Needs to be at least
  *		mpi_get_size(a) long.
- * @nbytes:	in/out param - it has the be set to the maximum number of
- *		bytes that can be written to sgl. This has to be at least
- *		the size of the integer a. On return it receives the actual
- *		length of the data written on success or the data that would
- *		be written if buffer was too small.
+ * @nbytes:	the number of bytes to write.  Leading bytes will be
+ *		filled with zero.
  * @sign:	if not NULL, it will be set to the sign of a.
  *
  * Return:	0 on success or error code in case of error
  */
-int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		     int *sign)
 {
 	u8 *p, *p2;
@@ -258,43 +255,44 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
-	int i, x, y = 0, lzeros, buf_len;
-
-	if (!nbytes)
-		return -EINVAL;
+	int i, x, buf_len;
 
 	if (sign)
 		*sign = a->sign;
 
-	lzeros = count_lzeros(a);
-
-	if (*nbytes < n - lzeros) {
-		*nbytes = n - lzeros;
+	if (nbytes < n)
 		return -EOVERFLOW;
-	}
 
-	*nbytes = n - lzeros;
 	buf_len = sgl->length;
 	p2 = sg_virt(sgl);
 
-	for (i = a->nlimbs - 1 - lzeros / BYTES_PER_MPI_LIMB,
-			lzeros %= BYTES_PER_MPI_LIMB;
-		i >= 0; i--) {
+	while (nbytes > n) {
+		if (!buf_len) {
+			sgl = sg_next(sgl);
+			if (!sgl)
+				return -EINVAL;
+			buf_len = sgl->length;
+			p2 = sg_virt(sgl);
+		}
+
+		i = min_t(unsigned, nbytes - n, buf_len);
+		memset(p2, 0, i);
+		p2 += i;
+		buf_len -= i;
+		nbytes -= i;
+	}
+
+	for (i = a->nlimbs - 1; i >= 0; i--) {
 #if BYTES_PER_MPI_LIMB == 4
-		alimb = cpu_to_be32(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be32(a->d[i]) : 0;
 #elif BYTES_PER_MPI_LIMB == 8
-		alimb = cpu_to_be64(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be64(a->d[i]) : 0;
 #else
 #error please implement for this limb size.
 #endif
-		if (lzeros) {
-			y = lzeros;
-			lzeros = 0;
-		}
-
-		p = (u8 *)&alimb + y;
+		p = (u8 *)&alimb;
 
-		for (x = 0; x < sizeof(alimb) - y; x++) {
+		for (x = 0; x < sizeof(alimb); x++) {
 			if (!buf_len) {
 				sgl = sg_next(sgl);
 				if (!sgl)
@@ -305,7 +303,6 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 			*p2++ = *p++;
 			buf_len--;
 		}
-		y = 0;
 	}
 	return 0;
 }

[PATCH 3/8] lib/mpi: Do not do sg_virt

[Herbert Xu]

Currently the mpi SG helpers use sg_virt which is completely
broken.  It happens to work with normal kernel memory but will
fail with anything that is not linearly mapped.

This patch fixes this by using the SG iterator helpers.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 lib/mpi/mpicoder.c |   86 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 36 deletions(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 7150e5c..c6272ae 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/count_zeros.h>
 #include <linux/byteorder/generic.h>
+#include <linux/scatterlist.h>
 #include <linux/string.h>
 #include "mpi-internal.h"
 
@@ -255,7 +256,9 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
+	struct sg_mapping_iter miter;
 	int i, x, buf_len;
+	int nents;
 
 	if (sign)
 		*sign = a->sign;
@@ -263,23 +266,27 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 	if (nbytes < n)
 		return -EOVERFLOW;
 
-	buf_len = sgl->length;
-	p2 = sg_virt(sgl);
+	nents = sg_nents_for_len(sgl, nbytes);
+	if (nents < 0)
+		return -EINVAL;
 
-	while (nbytes > n) {
-		if (!buf_len) {
-			sgl = sg_next(sgl);
-			if (!sgl)
-				return -EINVAL;
-			buf_len = sgl->length;
-			p2 = sg_virt(sgl);
-		}
+	sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC | SG_MITER_TO_SG);
+	sg_miter_next(&miter);
+	buf_len = miter.length;
+	p2 = miter.addr;
 
+	while (nbytes > n) {
 		i = min_t(unsigned, nbytes - n, buf_len);
 		memset(p2, 0, i);
 		p2 += i;
-		buf_len -= i;
 		nbytes -= i;
+
+		buf_len -= i;
+		if (!buf_len) {
+			sg_miter_next(&miter);
+			buf_len = miter.length;
+			p2 = miter.addr;
+		}
 	}
 
 	for (i = a->nlimbs - 1; i >= 0; i--) {
@@ -293,17 +300,16 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		p = (u8 *)&alimb;
 
 		for (x = 0; x < sizeof(alimb); x++) {
-			if (!buf_len) {
-				sgl = sg_next(sgl);
-				if (!sgl)
-					return -EINVAL;
-				buf_len = sgl->length;
-				p2 = sg_virt(sgl);
-			}
 			*p2++ = *p++;
-			buf_len--;
+			if (!--buf_len) {
+				sg_miter_next(&miter);
+				buf_len = miter.length;
+				p2 = miter.addr;
+			}
 		}
 	}
+
+	sg_miter_stop(&miter);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
@@ -323,19 +329,23 @@ EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
  */
 MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 {
-	struct scatterlist *sg;
-	int x, i, j, z, lzeros, ents;
+	struct sg_mapping_iter miter;
 	unsigned int nbits, nlimbs;
+	int x, j, z, lzeros, ents;
+	unsigned int len;
+	const u8 *buff;
 	mpi_limb_t a;
 	MPI val = NULL;
 
-	lzeros = 0;
-	ents = sg_nents(sgl);
+	ents = sg_nents_for_len(sgl, nbytes);
+	if (ents < 0)
+		return NULL;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buff = sg_virt(sg);
-		int len = sg->length;
+	sg_miter_start(&miter, sgl, ents, SG_MITER_ATOMIC | SG_MITER_FROM_SG);
 
+	lzeros = 0;
+	len = 0;
+	while (nbytes > 0) {
 		while (len && !*buff) {
 			lzeros++;
 			len--;
@@ -345,12 +355,14 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 		if (len && *buff)
 			break;
 
-		ents--;
+		sg_miter_next(&miter);
+		buff = miter.addr;
+		len = miter.length;
+
 		nbytes -= lzeros;
 		lzeros = 0;
 	}
 
-	sgl = sg;
 	nbytes -= lzeros;
 	nbits = nbytes * 8;
 	if (nbits > MAX_EXTERN_MPI_BITS) {
@@ -359,8 +371,7 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	}
 
 	if (nbytes > 0)
-		nbits -= count_leading_zeros(*(u8 *)(sg_virt(sgl) + lzeros)) -
-			(BITS_PER_LONG - 8);
+		nbits -= count_leading_zeros(*buff) - (BITS_PER_LONG - 8);
 
 	nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
 	val = mpi_alloc(nlimbs);
@@ -379,21 +390,24 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	z = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
 	z %= BYTES_PER_MPI_LIMB;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buffer = sg_virt(sg) + lzeros;
-		int len = sg->length - lzeros;
-
+	for (;;) {
 		for (x = 0; x < len; x++) {
 			a <<= 8;
-			a |= *buffer++;
+			a |= *buff++;
 			if (((z + x + 1) % BYTES_PER_MPI_LIMB) == 0) {
 				val->d[j--] = a;
 				a = 0;
 			}
 		}
 		z += x;
-		lzeros = 0;
+
+		if (!sg_miter_next(&miter))
+			break;
+
+		buff = miter.addr;
+		len = miter.length;
 	}
+
 	return val;
 }
 EXPORT_SYMBOL_GPL(mpi_read_raw_from_sgl);

[PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

The only user of rsa-pkcs1pad always uses the hash so there is
no reason to support the case of not having a hash.

This patch also changes the digest info lookup so that it is
only done once during template instantiation rather than on each
operation.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   83 ++++++++++++++++++--------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ead8dc0..5c1c78e 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -92,13 +92,12 @@ static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
 
 struct pkcs1pad_ctx {
 	struct crypto_akcipher *child;
-	const char *hash_name;
 	unsigned int key_size;
 };
 
 struct pkcs1pad_inst_ctx {
 	struct crypto_akcipher_spawn spawn;
-	const char *hash_name;
+	const struct rsa_asn1_template *digest_info;
 };
 
 struct pkcs1pad_request {
@@ -416,20 +415,16 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info = NULL;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	int err;
 	unsigned int ps_end, digest_size = 0;
 
 	if (!ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			return -EINVAL;
-
-		digest_size = digest_info->size;
-	}
+	digest_size = digest_info->size;
 
 	if (req->src_len + digest_size > ctx->key_size - 11)
 		return -EOVERFLOW;
@@ -462,10 +457,8 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	memset(req_ctx->in_buf + 1, 0xff, ps_end - 1);
 	req_ctx->in_buf[ps_end] = 0x00;
 
-	if (digest_info) {
-		memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
-		       digest_info->size);
-	}
+	memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
+	       digest_info->size);
 
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
@@ -499,7 +492,9 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	unsigned int pos;
 
 	if (err == -EOVERFLOW)
@@ -527,17 +522,11 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 		goto done;
 	pos++;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			goto done;
+	if (memcmp(req_ctx->out_buf + pos, digest_info->data,
+		   digest_info->size))
+		goto done;
 
-		if (memcmp(req_ctx->out_buf + pos, digest_info->data,
-			   digest_info->size))
-			goto done;
-
-		pos += digest_info->size;
-	}
+	pos += digest_info->size;
 
 	err = 0;
 
@@ -626,12 +615,11 @@ static int pkcs1pad_init_tfm(struct crypto_akcipher *tfm)
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct crypto_akcipher *child_tfm;
 
-	child_tfm = crypto_spawn_akcipher(akcipher_instance_ctx(inst));
+	child_tfm = crypto_spawn_akcipher(&ictx->spawn);
 	if (IS_ERR(child_tfm))
 		return PTR_ERR(child_tfm);
 
 	ctx->child = child_tfm;
-	ctx->hash_name = ictx->hash_name;
 	return 0;
 }
 
@@ -648,12 +636,12 @@ static void pkcs1pad_free(struct akcipher_instance *inst)
 	struct crypto_akcipher_spawn *spawn = &ctx->spawn;
 
 	crypto_drop_akcipher(spawn);
-	kfree(ctx->hash_name);
 	kfree(inst);
 }
 
 static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 {
+	const struct rsa_asn1_template *digest_info;
 	struct crypto_attr_type *algt;
 	struct akcipher_instance *inst;
 	struct pkcs1pad_inst_ctx *ctx;
@@ -676,7 +664,11 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	hash_name = crypto_attr_alg_name(tb[2]);
 	if (IS_ERR(hash_name))
-		hash_name = NULL;
+		return PTR_ERR(hash_name);
+
+	digest_info = rsa_lookup_asn1(hash_name);
+	if (!digest_info)
+		return -EINVAL;
 
 	inst = kzalloc(sizeof(*inst) + sizeof(*ctx), GFP_KERNEL);
 	if (!inst)
@@ -684,7 +676,7 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	ctx = akcipher_instance_ctx(inst);
 	spawn = &ctx->spawn;
-	ctx->hash_name = hash_name ? kstrdup(hash_name, GFP_KERNEL) : NULL;
+	ctx->digest_info = digest_info;
 
 	crypto_set_spawn(&spawn->base, akcipher_crypto_instance(inst));
 	err = crypto_grab_akcipher(spawn, rsa_alg_name, 0,
@@ -696,27 +688,14 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = -ENAMETOOLONG;
 
-	if (!hash_name) {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_name) >=
-					CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_driver_name) >=
-					CRYPTO_MAX_ALG_NAME)
+	if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)", rsa_alg->base.cra_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME ||
+	    snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)",
+		     rsa_alg->base.cra_driver_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME)
 		goto out_drop_alg;
-	} else {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_name, hash_name) >=
-				CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_driver_name, hash_name) >=
-					CRYPTO_MAX_ALG_NAME)
-		goto out_free_hash;
-	}
 
 	inst->alg.base.cra_flags = rsa_alg->base.cra_flags & CRYPTO_ALG_ASYNC;
 	inst->alg.base.cra_priority = rsa_alg->base.cra_priority;
@@ -738,12 +717,10 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = akcipher_register_instance(tmpl, inst);
 	if (err)
-		goto out_free_hash;
+		goto out_drop_alg;
 
 	return 0;
 
-out_free_hash:
-	kfree(ctx->hash_name);
 out_drop_alg:
 	crypto_drop_akcipher(spawn);
 out_free_inst:

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Andrzej Zaborowski]

Hi Herbert,

On 22 June 2016 at 12:16, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> The only user of rsa-pkcs1pad always uses the hash so there is
> no reason to support the case of not having a hash.

We use pkcs1pad with AF_ALG to implement lightweight TLS.  TLS
versions < 1.2 use a non-standard hash so we'd have to move the PKCS#1
padding back to userspace if this is changed.

The remaining patches make sense, I think there was some reason there
were those PAGE_SIZE checks in every operation but it's probably no
longer valid.

Best regards

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

On Wed, Jun 22, 2016 at 03:20:51PM +0200, Andrzej Zaborowski wrote:
>
> We use pkcs1pad with AF_ALG to implement lightweight TLS.  TLS
> versions < 1.2 use a non-standard hash so we'd have to move the PKCS#1
> padding back to userspace if this is changed.

When this is submitted for upstream inclusion we can add support
for it.

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

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Denis Kenzior]

Hi Herbert,

On 06/22/2016 09:02 AM, Herbert Xu wrote:
> On Wed, Jun 22, 2016 at 03:20:51PM +0200, Andrzej Zaborowski wrote:
>>
>> We use pkcs1pad with AF_ALG to implement lightweight TLS.  TLS
>> versions < 1.2 use a non-standard hash so we'd have to move the PKCS#1
>> padding back to userspace if this is changed.
>
> When this is submitted for upstream inclusion we can add support
> for it.
>

Just to clarify, we use this from userspace.  So we _already_ depend on 
this functionality.  Please keep the hash and non-hash versions of 
pkcs1pad available.

Regards,
-Denis

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

On Wed, Jun 22, 2016 at 09:19:16AM -0500, Denis Kenzior wrote:
> Just to clarify, we use this from userspace.  So we _already_ depend
> on this functionality.  Please keep the hash and non-hash versions
> of pkcs1pad available.

How can you be depending on this in userspace when we haven't
even exported akcipher to userspace? Colour me confused.

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

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Denis Kenzior]

Hi Herbert,

On 06/22/2016 09:20 AM, Herbert Xu wrote:
> On Wed, Jun 22, 2016 at 09:19:16AM -0500, Denis Kenzior wrote:
>> Just to clarify, we use this from userspace.  So we _already_ depend
>> on this functionality.  Please keep the hash and non-hash versions
>> of pkcs1pad available.
>
> How can you be depending on this in userspace when we haven't
> even exported akcipher to userspace? Colour me confused.
>

We live on the bleeding edge :)

I realize that these features are not upstream yet, but that doesn't 
mean that we can't influence / see the direction that the kernel is 
taking and act accordingly.

We'd like to have both pkcs1pad + hash, and simple pkcs1pad go upstream. 
  That will make our job in userspace much easier.  Andrew submitted 
pkcs1pad transform to the kernel specifically so we could get rid of 
this logic in our userspace code.  So please consider leaving both 
versions for upstream inclusion.

Regards,
-Denis

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

On Wed, Jun 22, 2016 at 09:30:12AM -0500, Denis Kenzior wrote:
>
> We live on the bleeding edge :)
> 
> I realize that these features are not upstream yet, but that doesn't
> mean that we can't influence / see the direction that the kernel is
> taking and act accordingly.
> 
> We'd like to have both pkcs1pad + hash, and simple pkcs1pad go
> upstream.  That will make our job in userspace much easier.  Andrew
> submitted pkcs1pad transform to the kernel specifically so we could
> get rid of this logic in our userspace code.  So please consider
> leaving both versions for upstream inclusion.

Sorry but the crypto API isn't a repository for general algorithms.
It's first and foremost a place for algorithms that we use in the
kernel.

The user-space interface (if we ever add one for akcipher, right now
there are strong objections against it) is mainly there to allow
access to hardware accelerators.  So I'm afraid I cannot keep the
hashless pkcs1pad until such a time that either we have a kernel
user for it or there is a piece of hardware implementing it.

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

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Mat Martineau]

Herbert,

On Wed, 22 Jun 2016, Herbert Xu wrote:

> On Wed, Jun 22, 2016 at 09:30:12AM -0500, Denis Kenzior wrote:
>>
>> We live on the bleeding edge :)
>>
>> I realize that these features are not upstream yet, but that doesn't
>> mean that we can't influence / see the direction that the kernel is
>> taking and act accordingly.
>>
>> We'd like to have both pkcs1pad + hash, and simple pkcs1pad go
>> upstream.  That will make our job in userspace much easier.  Andrew
>> submitted pkcs1pad transform to the kernel specifically so we could
>> get rid of this logic in our userspace code.  So please consider
>> leaving both versions for upstream inclusion.
>
> Sorry but the crypto API isn't a repository for general algorithms.
> It's first and foremost a place for algorithms that we use in the
> kernel.
>
> The user-space interface (if we ever add one for akcipher, right now
> there are strong objections against it) is mainly there to allow
> access to hardware accelerators.  So I'm afraid I cannot keep the
> hashless pkcs1pad until such a time that either we have a kernel
> user for it or there is a piece of hardware implementing it.

David Howells has a keyctl patch set in progress that makes use of 
pkcs1pad, with or without a hash:

https://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=keys-asym-keyctl&id=6fe3b4aa7df524f4867868b01d4cb4345b1bf2de

Please leave the non-hash code in, or consider deferring this patch until 
we can also discuss the issue at the upcoming security summit. We've been 
having a lot of trouble getting agreement on userspace access to 
asymmetric ciphers and I think we could make some progress with in-person 
discussion. (Mailing list discussion is also important because not 
everyone concerned can attend the summit)


Thanks,

--
Mat Martineau
Intel OTC

Re: [PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

On Wed, Jun 22, 2016 at 08:39:09AM -0700, Mat Martineau wrote:
>
> David Howells has a keyctl patch set in progress that makes use of
> pkcs1pad, with or without a hash:
> 
> https://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=keys-asym-keyctl&id=6fe3b4aa7df524f4867868b01d4cb4345b1bf2de

I'm sorry but I'm not going to allow arbitrary software algorithms
to be exported to userspace like this.

As I said, if we have a legitimate kernel user or a real hardware
implementation I'll reconsider this.

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

[PATCH 5/8] crypto: rsa-pkcs1pad - Remove bogus page splitting

[Herbert Xu]

The helper pkcs1pad_sg_set_buf tries to split a buffer that crosses
a page boundary into two SG entries.  This is unnecessary.  This
patch removes that.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 5c1c78e..d9baefb 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -103,7 +103,7 @@ struct pkcs1pad_inst_ctx {
 struct pkcs1pad_request {
 	struct akcipher_request child_req;
 
-	struct scatterlist in_sg[3], out_sg[2];
+	struct scatterlist in_sg[2], out_sg[1];
 	uint8_t *in_buf, *out_buf;
 };
 
@@ -163,19 +163,10 @@ static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
 static void pkcs1pad_sg_set_buf(struct scatterlist *sg, void *buf, size_t len,
 		struct scatterlist *next)
 {
-	int nsegs = next ? 1 : 0;
-
-	if (offset_in_page(buf) + len <= PAGE_SIZE) {
-		nsegs += 1;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg, buf, len);
-	} else {
-		nsegs += 2;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg + 0, buf, PAGE_SIZE - offset_in_page(buf));
-		sg_set_buf(sg + 1, buf + PAGE_SIZE - offset_in_page(buf),
-				offset_in_page(buf) + len - PAGE_SIZE);
-	}
+	int nsegs = next ? 2 : 1;
+
+	sg_init_table(sg, nsegs);
+	sg_set_buf(sg, buf, len);
 
 	if (next)
 		sg_chain(sg, nsegs, next);

[PATCH 6/8] crypto: rsa-pkcs1pad - Always use GFP_KERNEL

[Herbert Xu]

We don't currently support using akcipher in atomic contexts,
so GFP_KERNEL should always be used.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index d9baefb..db19284 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -260,8 +260,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -274,9 +273,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -379,9 +376,7 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size ;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 
@@ -438,8 +433,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -454,9 +448,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -577,9 +569,7 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 

[PATCH 7/8] crypto: rsa-pkcs1pad - Move key size check to setkey

[Herbert Xu]

Rather than repeatedly checking the key size on each operation,
we should be checking it once when the key is set.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   56 +++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index db19284..ebd8514 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -111,40 +111,48 @@ static int pkcs1pad_set_pub_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_pub_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_set_priv_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_priv_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
@@ -247,9 +255,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -367,9 +372,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;
@@ -420,9 +422,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -560,9 +559,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;

[PATCH 8/8] crypto: rsa-pkcs1pad - Avoid copying output when possible

[Herbert Xu]

In the vast majority of cases (2^-32 on 32-bit and 2^-64 on 64-bit)
cases, the result from encryption/signing will require no padding.

This patch makes these two operations write their output directly
to the final destination.  Only in the exceedingly rare cases where
fixup is needed to we copy it out and back to add the leading zeroes.

This patch also makes use of the crypto_akcipher_set_crypt API
instead of writing the akcipher request directly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |  112 ++++++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 67 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ebd8514..8ccfdd7 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -185,37 +185,36 @@ static int pkcs1pad_encrypt_sign_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	size_t pad_len = ctx->key_size - req_ctx->child_req.dst_len;
-	size_t chunk_len, pad_left;
-	struct sg_mapping_iter miter;
-
-	if (!err) {
-		if (pad_len) {
-			sg_miter_start(&miter, req->dst,
-					sg_nents_for_len(req->dst, pad_len),
-					SG_MITER_ATOMIC | SG_MITER_TO_SG);
-
-			pad_left = pad_len;
-			while (pad_left) {
-				sg_miter_next(&miter);
-
-				chunk_len = min(miter.length, pad_left);
-				memset(miter.addr, 0, chunk_len);
-				pad_left -= chunk_len;
-			}
-
-			sg_miter_stop(&miter);
-		}
-
-		sg_pcopy_from_buffer(req->dst,
-				sg_nents_for_len(req->dst, ctx->key_size),
-				req_ctx->out_buf, req_ctx->child_req.dst_len,
-				pad_len);
-	}
+	unsigned int pad_len;
+	unsigned int len;
+	u8 *out_buf;
+
+	if (err)
+		goto out;
+
+	len = req_ctx->child_req.dst_len;
+	pad_len = ctx->key_size - len;
+
+	/* Four billion to one */
+	if (likely(!pad_len))
+		goto out;
+
+	out_buf = kzalloc(ctx->key_size, GFP_ATOMIC);
+	err = -ENOMEM;
+	if (!out_buf)
+		goto out;
+
+	sg_copy_to_buffer(req->dst, sg_nents_for_len(req->dst, len),
+			  out_buf + pad_len, len);
+	sg_copy_from_buffer(req->dst,
+			    sg_nents_for_len(req->dst, ctx->key_size),
+			    out_buf, ctx->key_size);
+	kzfree(out_buf);
+
+out:
 	req->dst_len = ctx->key_size;
 
 	kfree(req_ctx->in_buf);
-	kzfree(req_ctx->out_buf);
 
 	return err;
 }
@@ -255,15 +254,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -291,6 +281,10 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_encrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -372,12 +366,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size ;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -389,6 +377,11 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_decrypt_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_decrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -422,15 +415,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -447,19 +431,14 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
-	if (!req_ctx->out_buf) {
-		kfree(req_ctx->in_buf);
-		return -ENOMEM;
-	}
-
-	pkcs1pad_sg_set_buf(req_ctx->out_sg, req_ctx->out_buf,
-			ctx->key_size, NULL);
-
 	akcipher_request_set_tfm(&req_ctx->child_req, ctx->child);
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_sign(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -559,12 +538,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -576,6 +549,11 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_verify_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_verify(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Tadeusz Struk]

Hi Herbert,
On 06/22/2016 03:14 AM, Herbert Xu wrote:
> This was prompted by the caam RSA submission where a lot of work
> was done just to strip the RSA output of leading zeroes.  This is
> in fact completely pointless because the only user of RSA in the
> kernel then promptly puts them back.
> 
> This patch series resolves this madness by simply leaving any
> leading zeroes in place.

The reason why mpi_write_to_sgl() strips the leading zeros is only
because we said that it needs to work in the same way as the
mpi_read_buffer(), which does remove it for whatever reason.
So should we now change the mpi_read_buffer() as well?
The mpi_read_buffer() is called from mpi_get_buffer(), which is used
only by lib/digsig.c
We also need to change the qat rsa implementation because it does remove
zeros as well, but it will be very easy to do.
Thanks,
--
TS

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

On Thu, Jun 23, 2016 at 08:25:05AM -0700, Tadeusz Struk wrote:
> 
> The reason why mpi_write_to_sgl() strips the leading zeros is only
> because we said that it needs to work in the same way as the
> mpi_read_buffer(), which does remove it for whatever reason.
> So should we now change the mpi_read_buffer() as well?

Didn't we add mpi_read_buffer specifically for akcipher before
we switched over to SGs? If nobody is using it we should just
delete it.

> We also need to change the qat rsa implementation because it does remove
> zeros as well, but it will be very easy to do.

The way it's done with my patches you don't have to do the conversion
right away because it'll cope with either stripping or not stripping
leading zeroes.  But yes it should simplify the code in qat so please
send in patches when you have time.

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

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Tadeusz Struk]

On 06/24/2016 07:28 AM, Herbert Xu wrote:
> Didn't we add mpi_read_buffer specifically for akcipher before
> we switched over to SGs? If nobody is using it we should just
> delete it.
Yes, but now mpi_get_buffer() calls mpi_read_buffer() and it is also
used by security/keys/dh.c
Thanks,
-- 
TS

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

On Fri, Jun 24, 2016 at 08:25:02AM -0700, Tadeusz Struk wrote:
> Yes, but now mpi_get_buffer() calls mpi_read_buffer() and it is also
> used by security/keys/dh.c

It appears to be using mpi_read_buffer gratuitously.  It should
simply use mpi_get_buffer.

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

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Stephan Mueller]

Am Mittwoch, 22. Juni 2016, 18:14:32 schrieb Herbert Xu:

Hi Herbert,

Something breaks with this patch set in public_key_verify_signature

I get tons of these:

[    1.838720] PKCS#7 signature not signed with a trusted key


Furthermore, my CAVS testing with public_key_verify_signature always EINVAL.

SigGen using the kernel crypto API interfaces work though.

Ciao
Stephan

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

On Fri, Jun 24, 2016 at 09:27:12AM +0200, Stephan Mueller wrote:
> Am Mittwoch, 22. Juni 2016, 18:14:32 schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> Something breaks with this patch set in public_key_verify_signature
> 
> I get tons of these:
> 
> [    1.838720] PKCS#7 signature not signed with a trusted key
> 
> 
> Furthermore, my CAVS testing with public_key_verify_signature always EINVAL.
> 
> SigGen using the kernel crypto API interfaces work though.

Hi Stephan:

Can you bisect this down to a specific patch please?

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

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Stephan Mueller]

Am Freitag, 24. Juni 2016, 16:41:47 schrieb Herbert Xu:

Hi Herbert,

> On Fri, Jun 24, 2016 at 09:27:12AM +0200, Stephan Mueller wrote:
> > Am Mittwoch, 22. Juni 2016, 18:14:32 schrieb Herbert Xu:
> > 
> > Hi Herbert,
> > 
> > Something breaks with this patch set in public_key_verify_signature
> > 
> > I get tons of these:
> > 
> > [    1.838720] PKCS#7 signature not signed with a trusted key
> > 
> > 
> > Furthermore, my CAVS testing with public_key_verify_signature always
> > EINVAL.
> > 
> > SigGen using the kernel crypto API interfaces work though.
> 
> Hi Stephan:
> 
> Can you bisect this down to a specific patch please?

Will do.
> 
> Thanks!


Ciao
Stephan

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Stephan Mueller]

Am Freitag, 24. Juni 2016, 16:41:47 schrieb Herbert Xu:

Hi Herbert,

> On Fri, Jun 24, 2016 at 09:27:12AM +0200, Stephan Mueller wrote:
> > Am Mittwoch, 22. Juni 2016, 18:14:32 schrieb Herbert Xu:
> > 
> > Hi Herbert,
> > 
> > Something breaks with this patch set in public_key_verify_signature
> > 
> > I get tons of these:
> > 
> > [    1.838720] PKCS#7 signature not signed with a trusted key
> > 
> > 
> > Furthermore, my CAVS testing with public_key_verify_signature always
> > EINVAL.
> > 
> > SigGen using the kernel crypto API interfaces work though.
> 
> Hi Stephan:
> 
> Can you bisect this down to a specific patch please?

Patch 2 introduces the bug.

Note, with patch 2, there is also a compile warning with crypto/dh.c.
> 
> Thanks!


Ciao
Stephan

Re: crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

On Fri, Jun 24, 2016 at 11:23:06AM +0200, Stephan Mueller wrote:
> 
> Patch 2 introduces the bug.
> 
> Note, with patch 2, there is also a compile warning with crypto/dh.c.

Oh I see.  It's a conflict with the kpp stuff that didn't exist
when I wrote this.

Let me respin the patches on top of kpp.

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

[v2 PATCH 0/7] crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

Hi:

This was prompted by the caam RSA submission where a lot of work
was done just to strip the RSA output of leading zeroes.  This is
in fact completely pointless because the only user of RSA in the
kernel then promptly puts them back.

This patch series resolves this madness by simply leaving any
leading zeroes in place.  Note that we're not requiring authors
to add leading zeroes, even though that is encouraged if it is
easy to do.  In practice you'd only run into this every 2^32 or
2^64 operations so please don't overdo it.

I've also taken the opportunity to cleanup the pkcs1pad code.

v2 fixes the newly added dh to use the new MPI SG interface.

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

[v2 PATCH 1/7] crypto: rsa - Generate fixed-length output

[Herbert Xu]

Every implementation of RSA that we have naturally generates
output with leading zeroes.  The one and only user of RSA,
pkcs1pad wants to have those leading zeroes in place, in fact
because they are currently absent it has to write those zeroes
itself.

So we shouldn't be stripping leading zeroes in the first place.
In fact this patch makes rsa-generic produce output with fixed
length so that pkcs1pad does not need to do any extra work.

This patch also changes DH to use the new interface.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa.c        |    8 +++----
 include/linux/mpi.h |    2 -
 lib/mpi/mpicoder.c  |   55 ++++++++++++++++++++++++----------------------------
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/crypto/rsa.c b/crypto/rsa.c
index dc692d4..4c280b6 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -108,7 +108,7 @@ static int rsa_enc(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(c, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(c, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -147,7 +147,7 @@ static int rsa_dec(struct akcipher_request *req)
 	if (ret)
 		goto err_free_c;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_c;
 
@@ -185,7 +185,7 @@ static int rsa_sign(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(s, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(s, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -226,7 +226,7 @@ static int rsa_verify(struct akcipher_request *req)
 	if (ret)
 		goto err_free_s;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_s;
 
diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index f219559..1cc5ffb 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -80,7 +80,7 @@ void *mpi_get_buffer(MPI a, unsigned *nbytes, int *sign);
 int mpi_read_buffer(MPI a, uint8_t *buf, unsigned buf_len, unsigned *nbytes,
 		    int *sign);
 void *mpi_get_secure_buffer(MPI a, unsigned *nbytes, int *sign);
-int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned nbytes,
 		     int *sign);
 
 #define log_mpidump g10_log_mpidump
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 823cf5f..7150e5c 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -237,16 +237,13 @@ EXPORT_SYMBOL_GPL(mpi_get_buffer);
  * @a:		a multi precision integer
  * @sgl:	scatterlist to write to. Needs to be at least
  *		mpi_get_size(a) long.
- * @nbytes:	in/out param - it has the be set to the maximum number of
- *		bytes that can be written to sgl. This has to be at least
- *		the size of the integer a. On return it receives the actual
- *		length of the data written on success or the data that would
- *		be written if buffer was too small.
+ * @nbytes:	the number of bytes to write.  Leading bytes will be
+ *		filled with zero.
  * @sign:	if not NULL, it will be set to the sign of a.
  *
  * Return:	0 on success or error code in case of error
  */
-int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		     int *sign)
 {
 	u8 *p, *p2;
@@ -258,43 +255,44 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
-	int i, x, y = 0, lzeros, buf_len;
-
-	if (!nbytes)
-		return -EINVAL;
+	int i, x, buf_len;
 
 	if (sign)
 		*sign = a->sign;
 
-	lzeros = count_lzeros(a);
-
-	if (*nbytes < n - lzeros) {
-		*nbytes = n - lzeros;
+	if (nbytes < n)
 		return -EOVERFLOW;
-	}
 
-	*nbytes = n - lzeros;
 	buf_len = sgl->length;
 	p2 = sg_virt(sgl);
 
-	for (i = a->nlimbs - 1 - lzeros / BYTES_PER_MPI_LIMB,
-			lzeros %= BYTES_PER_MPI_LIMB;
-		i >= 0; i--) {
+	while (nbytes > n) {
+		if (!buf_len) {
+			sgl = sg_next(sgl);
+			if (!sgl)
+				return -EINVAL;
+			buf_len = sgl->length;
+			p2 = sg_virt(sgl);
+		}
+
+		i = min_t(unsigned, nbytes - n, buf_len);
+		memset(p2, 0, i);
+		p2 += i;
+		buf_len -= i;
+		nbytes -= i;
+	}
+
+	for (i = a->nlimbs - 1; i >= 0; i--) {
 #if BYTES_PER_MPI_LIMB == 4
-		alimb = cpu_to_be32(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be32(a->d[i]) : 0;
 #elif BYTES_PER_MPI_LIMB == 8
-		alimb = cpu_to_be64(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be64(a->d[i]) : 0;
 #else
 #error please implement for this limb size.
 #endif
-		if (lzeros) {
-			y = lzeros;
-			lzeros = 0;
-		}
-
-		p = (u8 *)&alimb + y;
+		p = (u8 *)&alimb;
 
-		for (x = 0; x < sizeof(alimb) - y; x++) {
+		for (x = 0; x < sizeof(alimb); x++) {
 			if (!buf_len) {
 				sgl = sg_next(sgl);
 				if (!sgl)
@@ -305,7 +303,6 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 			*p2++ = *p++;
 			buf_len--;
 		}
-		y = 0;
 	}
 	return 0;
 }

[v2 PATCH 2/7] lib/mpi: Do not do sg_virt

[Herbert Xu]

Currently the mpi SG helpers use sg_virt which is completely
broken.  It happens to work with normal kernel memory but will
fail with anything that is not linearly mapped.

This patch fixes this by using the SG iterator helpers.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 lib/mpi/mpicoder.c |   86 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 36 deletions(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 7150e5c..c6272ae 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/count_zeros.h>
 #include <linux/byteorder/generic.h>
+#include <linux/scatterlist.h>
 #include <linux/string.h>
 #include "mpi-internal.h"
 
@@ -255,7 +256,9 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
+	struct sg_mapping_iter miter;
 	int i, x, buf_len;
+	int nents;
 
 	if (sign)
 		*sign = a->sign;
@@ -263,23 +266,27 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 	if (nbytes < n)
 		return -EOVERFLOW;
 
-	buf_len = sgl->length;
-	p2 = sg_virt(sgl);
+	nents = sg_nents_for_len(sgl, nbytes);
+	if (nents < 0)
+		return -EINVAL;
 
-	while (nbytes > n) {
-		if (!buf_len) {
-			sgl = sg_next(sgl);
-			if (!sgl)
-				return -EINVAL;
-			buf_len = sgl->length;
-			p2 = sg_virt(sgl);
-		}
+	sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC | SG_MITER_TO_SG);
+	sg_miter_next(&miter);
+	buf_len = miter.length;
+	p2 = miter.addr;
 
+	while (nbytes > n) {
 		i = min_t(unsigned, nbytes - n, buf_len);
 		memset(p2, 0, i);
 		p2 += i;
-		buf_len -= i;
 		nbytes -= i;
+
+		buf_len -= i;
+		if (!buf_len) {
+			sg_miter_next(&miter);
+			buf_len = miter.length;
+			p2 = miter.addr;
+		}
 	}
 
 	for (i = a->nlimbs - 1; i >= 0; i--) {
@@ -293,17 +300,16 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		p = (u8 *)&alimb;
 
 		for (x = 0; x < sizeof(alimb); x++) {
-			if (!buf_len) {
-				sgl = sg_next(sgl);
-				if (!sgl)
-					return -EINVAL;
-				buf_len = sgl->length;
-				p2 = sg_virt(sgl);
-			}
 			*p2++ = *p++;
-			buf_len--;
+			if (!--buf_len) {
+				sg_miter_next(&miter);
+				buf_len = miter.length;
+				p2 = miter.addr;
+			}
 		}
 	}
+
+	sg_miter_stop(&miter);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
@@ -323,19 +329,23 @@ EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
  */
 MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 {
-	struct scatterlist *sg;
-	int x, i, j, z, lzeros, ents;
+	struct sg_mapping_iter miter;
 	unsigned int nbits, nlimbs;
+	int x, j, z, lzeros, ents;
+	unsigned int len;
+	const u8 *buff;
 	mpi_limb_t a;
 	MPI val = NULL;
 
-	lzeros = 0;
-	ents = sg_nents(sgl);
+	ents = sg_nents_for_len(sgl, nbytes);
+	if (ents < 0)
+		return NULL;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buff = sg_virt(sg);
-		int len = sg->length;
+	sg_miter_start(&miter, sgl, ents, SG_MITER_ATOMIC | SG_MITER_FROM_SG);
 
+	lzeros = 0;
+	len = 0;
+	while (nbytes > 0) {
 		while (len && !*buff) {
 			lzeros++;
 			len--;
@@ -345,12 +355,14 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 		if (len && *buff)
 			break;
 
-		ents--;
+		sg_miter_next(&miter);
+		buff = miter.addr;
+		len = miter.length;
+
 		nbytes -= lzeros;
 		lzeros = 0;
 	}
 
-	sgl = sg;
 	nbytes -= lzeros;
 	nbits = nbytes * 8;
 	if (nbits > MAX_EXTERN_MPI_BITS) {
@@ -359,8 +371,7 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	}
 
 	if (nbytes > 0)
-		nbits -= count_leading_zeros(*(u8 *)(sg_virt(sgl) + lzeros)) -
-			(BITS_PER_LONG - 8);
+		nbits -= count_leading_zeros(*buff) - (BITS_PER_LONG - 8);
 
 	nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
 	val = mpi_alloc(nlimbs);
@@ -379,21 +390,24 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	z = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
 	z %= BYTES_PER_MPI_LIMB;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buffer = sg_virt(sg) + lzeros;
-		int len = sg->length - lzeros;
-
+	for (;;) {
 		for (x = 0; x < len; x++) {
 			a <<= 8;
-			a |= *buffer++;
+			a |= *buff++;
 			if (((z + x + 1) % BYTES_PER_MPI_LIMB) == 0) {
 				val->d[j--] = a;
 				a = 0;
 			}
 		}
 		z += x;
-		lzeros = 0;
+
+		if (!sg_miter_next(&miter))
+			break;
+
+		buff = miter.addr;
+		len = miter.length;
 	}
+
 	return val;
 }
 EXPORT_SYMBOL_GPL(mpi_read_raw_from_sgl);

[v2 PATCH 3/7] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

The only user of rsa-pkcs1pad always uses the hash so there is
no reason to support the case of not having a hash.

This patch also changes the digest info lookup so that it is
only done once during template instantiation rather than on each
operation.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   83 ++++++++++++++++++--------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ead8dc0..5c1c78e 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -92,13 +92,12 @@ static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
 
 struct pkcs1pad_ctx {
 	struct crypto_akcipher *child;
-	const char *hash_name;
 	unsigned int key_size;
 };
 
 struct pkcs1pad_inst_ctx {
 	struct crypto_akcipher_spawn spawn;
-	const char *hash_name;
+	const struct rsa_asn1_template *digest_info;
 };
 
 struct pkcs1pad_request {
@@ -416,20 +415,16 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info = NULL;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	int err;
 	unsigned int ps_end, digest_size = 0;
 
 	if (!ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			return -EINVAL;
-
-		digest_size = digest_info->size;
-	}
+	digest_size = digest_info->size;
 
 	if (req->src_len + digest_size > ctx->key_size - 11)
 		return -EOVERFLOW;
@@ -462,10 +457,8 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	memset(req_ctx->in_buf + 1, 0xff, ps_end - 1);
 	req_ctx->in_buf[ps_end] = 0x00;
 
-	if (digest_info) {
-		memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
-		       digest_info->size);
-	}
+	memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
+	       digest_info->size);
 
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
@@ -499,7 +492,9 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	unsigned int pos;
 
 	if (err == -EOVERFLOW)
@@ -527,17 +522,11 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 		goto done;
 	pos++;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			goto done;
+	if (memcmp(req_ctx->out_buf + pos, digest_info->data,
+		   digest_info->size))
+		goto done;
 
-		if (memcmp(req_ctx->out_buf + pos, digest_info->data,
-			   digest_info->size))
-			goto done;
-
-		pos += digest_info->size;
-	}
+	pos += digest_info->size;
 
 	err = 0;
 
@@ -626,12 +615,11 @@ static int pkcs1pad_init_tfm(struct crypto_akcipher *tfm)
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct crypto_akcipher *child_tfm;
 
-	child_tfm = crypto_spawn_akcipher(akcipher_instance_ctx(inst));
+	child_tfm = crypto_spawn_akcipher(&ictx->spawn);
 	if (IS_ERR(child_tfm))
 		return PTR_ERR(child_tfm);
 
 	ctx->child = child_tfm;
-	ctx->hash_name = ictx->hash_name;
 	return 0;
 }
 
@@ -648,12 +636,12 @@ static void pkcs1pad_free(struct akcipher_instance *inst)
 	struct crypto_akcipher_spawn *spawn = &ctx->spawn;
 
 	crypto_drop_akcipher(spawn);
-	kfree(ctx->hash_name);
 	kfree(inst);
 }
 
 static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 {
+	const struct rsa_asn1_template *digest_info;
 	struct crypto_attr_type *algt;
 	struct akcipher_instance *inst;
 	struct pkcs1pad_inst_ctx *ctx;
@@ -676,7 +664,11 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	hash_name = crypto_attr_alg_name(tb[2]);
 	if (IS_ERR(hash_name))
-		hash_name = NULL;
+		return PTR_ERR(hash_name);
+
+	digest_info = rsa_lookup_asn1(hash_name);
+	if (!digest_info)
+		return -EINVAL;
 
 	inst = kzalloc(sizeof(*inst) + sizeof(*ctx), GFP_KERNEL);
 	if (!inst)
@@ -684,7 +676,7 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	ctx = akcipher_instance_ctx(inst);
 	spawn = &ctx->spawn;
-	ctx->hash_name = hash_name ? kstrdup(hash_name, GFP_KERNEL) : NULL;
+	ctx->digest_info = digest_info;
 
 	crypto_set_spawn(&spawn->base, akcipher_crypto_instance(inst));
 	err = crypto_grab_akcipher(spawn, rsa_alg_name, 0,
@@ -696,27 +688,14 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = -ENAMETOOLONG;
 
-	if (!hash_name) {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_name) >=
-					CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_driver_name) >=
-					CRYPTO_MAX_ALG_NAME)
+	if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)", rsa_alg->base.cra_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME ||
+	    snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)",
+		     rsa_alg->base.cra_driver_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME)
 		goto out_drop_alg;
-	} else {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_name, hash_name) >=
-				CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_driver_name, hash_name) >=
-					CRYPTO_MAX_ALG_NAME)
-		goto out_free_hash;
-	}
 
 	inst->alg.base.cra_flags = rsa_alg->base.cra_flags & CRYPTO_ALG_ASYNC;
 	inst->alg.base.cra_priority = rsa_alg->base.cra_priority;
@@ -738,12 +717,10 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = akcipher_register_instance(tmpl, inst);
 	if (err)
-		goto out_free_hash;
+		goto out_drop_alg;
 
 	return 0;
 
-out_free_hash:
-	kfree(ctx->hash_name);
 out_drop_alg:
 	crypto_drop_akcipher(spawn);
 out_free_inst:

[v2 PATCH 4/7] crypto: rsa-pkcs1pad - Remove bogus page splitting

[Herbert Xu]

The helper pkcs1pad_sg_set_buf tries to split a buffer that crosses
a page boundary into two SG entries.  This is unnecessary.  This
patch removes that.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 5c1c78e..d9baefb 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -103,7 +103,7 @@ struct pkcs1pad_inst_ctx {
 struct pkcs1pad_request {
 	struct akcipher_request child_req;
 
-	struct scatterlist in_sg[3], out_sg[2];
+	struct scatterlist in_sg[2], out_sg[1];
 	uint8_t *in_buf, *out_buf;
 };
 
@@ -163,19 +163,10 @@ static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
 static void pkcs1pad_sg_set_buf(struct scatterlist *sg, void *buf, size_t len,
 		struct scatterlist *next)
 {
-	int nsegs = next ? 1 : 0;
-
-	if (offset_in_page(buf) + len <= PAGE_SIZE) {
-		nsegs += 1;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg, buf, len);
-	} else {
-		nsegs += 2;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg + 0, buf, PAGE_SIZE - offset_in_page(buf));
-		sg_set_buf(sg + 1, buf + PAGE_SIZE - offset_in_page(buf),
-				offset_in_page(buf) + len - PAGE_SIZE);
-	}
+	int nsegs = next ? 2 : 1;
+
+	sg_init_table(sg, nsegs);
+	sg_set_buf(sg, buf, len);
 
 	if (next)
 		sg_chain(sg, nsegs, next);

[v2 PATCH 5/7] crypto: rsa-pkcs1pad - Always use GFP_KERNEL

[Herbert Xu]

We don't currently support using akcipher in atomic contexts,
so GFP_KERNEL should always be used.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index d9baefb..db19284 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -260,8 +260,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -274,9 +273,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -379,9 +376,7 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size ;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 
@@ -438,8 +433,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -454,9 +448,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -577,9 +569,7 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 

[v2 PATCH 6/7] crypto: rsa-pkcs1pad - Move key size check to setkey

[Herbert Xu]

Rather than repeatedly checking the key size on each operation,
we should be checking it once when the key is set.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   56 +++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index db19284..ebd8514 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -111,40 +111,48 @@ static int pkcs1pad_set_pub_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_pub_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_set_priv_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_priv_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
@@ -247,9 +255,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -367,9 +372,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;
@@ -420,9 +422,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -560,9 +559,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;

[v2 PATCH 7/7] crypto: rsa-pkcs1pad - Avoid copying output when possible

[Herbert Xu]

In the vast majority of cases (2^-32 on 32-bit and 2^-64 on 64-bit)
cases, the result from encryption/signing will require no padding.

This patch makes these two operations write their output directly
to the final destination.  Only in the exceedingly rare cases where
fixup is needed to we copy it out and back to add the leading zeroes.

This patch also makes use of the crypto_akcipher_set_crypt API
instead of writing the akcipher request directly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |  112 ++++++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 67 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ebd8514..8ccfdd7 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -185,37 +185,36 @@ static int pkcs1pad_encrypt_sign_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	size_t pad_len = ctx->key_size - req_ctx->child_req.dst_len;
-	size_t chunk_len, pad_left;
-	struct sg_mapping_iter miter;
-
-	if (!err) {
-		if (pad_len) {
-			sg_miter_start(&miter, req->dst,
-					sg_nents_for_len(req->dst, pad_len),
-					SG_MITER_ATOMIC | SG_MITER_TO_SG);
-
-			pad_left = pad_len;
-			while (pad_left) {
-				sg_miter_next(&miter);
-
-				chunk_len = min(miter.length, pad_left);
-				memset(miter.addr, 0, chunk_len);
-				pad_left -= chunk_len;
-			}
-
-			sg_miter_stop(&miter);
-		}
-
-		sg_pcopy_from_buffer(req->dst,
-				sg_nents_for_len(req->dst, ctx->key_size),
-				req_ctx->out_buf, req_ctx->child_req.dst_len,
-				pad_len);
-	}
+	unsigned int pad_len;
+	unsigned int len;
+	u8 *out_buf;
+
+	if (err)
+		goto out;
+
+	len = req_ctx->child_req.dst_len;
+	pad_len = ctx->key_size - len;
+
+	/* Four billion to one */
+	if (likely(!pad_len))
+		goto out;
+
+	out_buf = kzalloc(ctx->key_size, GFP_ATOMIC);
+	err = -ENOMEM;
+	if (!out_buf)
+		goto out;
+
+	sg_copy_to_buffer(req->dst, sg_nents_for_len(req->dst, len),
+			  out_buf + pad_len, len);
+	sg_copy_from_buffer(req->dst,
+			    sg_nents_for_len(req->dst, ctx->key_size),
+			    out_buf, ctx->key_size);
+	kzfree(out_buf);
+
+out:
 	req->dst_len = ctx->key_size;
 
 	kfree(req_ctx->in_buf);
-	kzfree(req_ctx->out_buf);
 
 	return err;
 }
@@ -255,15 +254,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -291,6 +281,10 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_encrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -372,12 +366,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size ;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -389,6 +377,11 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_decrypt_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_decrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -422,15 +415,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -447,19 +431,14 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
-	if (!req_ctx->out_buf) {
-		kfree(req_ctx->in_buf);
-		return -ENOMEM;
-	}
-
-	pkcs1pad_sg_set_buf(req_ctx->out_sg, req_ctx->out_buf,
-			ctx->key_size, NULL);
-
 	akcipher_request_set_tfm(&req_ctx->child_req, ctx->child);
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_sign(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -559,12 +538,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -576,6 +549,11 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_verify_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_verify(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||

[v3 PATCH 0/8] crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

Hi:

This was prompted by the caam RSA submission where a lot of work
was done just to strip the RSA output of leading zeroes.  This is
in fact completely pointless because the only user of RSA in the
kernel then promptly puts them back.

This patch series resolves this madness by simply leaving any
leading zeroes in place.  Note that we're not requiring authors
to add leading zeroes, even though that is encouraged if it is
easy to do.  In practice you'd only run into this every 2^32 or
2^64 operations so please don't overdo it.

I've also taken the opportunity to cleanup the pkcs1pad code.

v2 fixes the newly added dh to use the new MPI SG interface.
v3 adds a patch that went AWOL in v2.

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

[v3 PATCH 1/8] crypto: testmgr - Allow leading zeros in RSA

[Herbert Xu]

This patch allows RSA implementations to produce output with
leading zeroes.  testmgr will skip leading zeroes when comparing
the output.

This patch also tries to make the RSA test function generic enough
to potentially handle other akcipher algorithms.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/testmgr.c |   51 ++++++++++++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 537fdc3..38e23be31 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1911,8 +1911,8 @@ static int alg_test_kpp(const struct alg_test_desc *desc, const char *driver,
 	return err;
 }
 
-static int do_test_rsa(struct crypto_akcipher *tfm,
-		       struct akcipher_testvec *vecs)
+static int test_akcipher_one(struct crypto_akcipher *tfm,
+			     struct akcipher_testvec *vecs)
 {
 	char *xbuf[XBUFSIZE];
 	struct akcipher_request *req;
@@ -1963,17 +1963,18 @@ static int do_test_rsa(struct crypto_akcipher *tfm,
 	/* Run RSA encrypt - c = m^e mod n;*/
 	err = wait_async_op(&result, crypto_akcipher_encrypt(req));
 	if (err) {
-		pr_err("alg: rsa: encrypt test failed. err %d\n", err);
+		pr_err("alg: akcipher: encrypt test failed. err %d\n", err);
 		goto free_all;
 	}
 	if (req->dst_len != vecs->c_size) {
-		pr_err("alg: rsa: encrypt test failed. Invalid output len\n");
+		pr_err("alg: akcipher: encrypt test failed. Invalid output len\n");
 		err = -EINVAL;
 		goto free_all;
 	}
 	/* verify that encrypted message is equal to expected */
 	if (memcmp(vecs->c, outbuf_enc, vecs->c_size)) {
-		pr_err("alg: rsa: encrypt test failed. Invalid output\n");
+		pr_err("alg: akcipher: encrypt test failed. Invalid output\n");
+		hexdump(outbuf_enc, vecs->c_size);
 		err = -EINVAL;
 		goto free_all;
 	}
@@ -2001,18 +2002,22 @@ static int do_test_rsa(struct crypto_akcipher *tfm,
 	/* Run RSA decrypt - m = c^d mod n;*/
 	err = wait_async_op(&result, crypto_akcipher_decrypt(req));
 	if (err) {
-		pr_err("alg: rsa: decrypt test failed. err %d\n", err);
+		pr_err("alg: akcipher: decrypt test failed. err %d\n", err);
 		goto free_all;
 	}
 	out_len = req->dst_len;
-	if (out_len != vecs->m_size) {
-		pr_err("alg: rsa: decrypt test failed. Invalid output len\n");
+	if (out_len < vecs->m_size) {
+		pr_err("alg: akcipher: decrypt test failed. "
+		       "Invalid output len %u\n", out_len);
 		err = -EINVAL;
 		goto free_all;
 	}
 	/* verify that decrypted message is equal to the original msg */
-	if (memcmp(vecs->m, outbuf_dec, vecs->m_size)) {
-		pr_err("alg: rsa: decrypt test failed. Invalid output\n");
+	if (memchr_inv(outbuf_dec, 0, out_len - vecs->m_size) ||
+	    memcmp(vecs->m, outbuf_dec + out_len - vecs->m_size,
+		   vecs->m_size)) {
+		pr_err("alg: akcipher: decrypt test failed. Invalid output\n");
+		hexdump(outbuf_dec, out_len);
 		err = -EINVAL;
 	}
 free_all:
@@ -2025,28 +2030,20 @@ free_xbuf:
 	return err;
 }
 
-static int test_rsa(struct crypto_akcipher *tfm, struct akcipher_testvec *vecs,
-		    unsigned int tcount)
+static int test_akcipher(struct crypto_akcipher *tfm, const char *alg,
+			 struct akcipher_testvec *vecs, unsigned int tcount)
 {
 	int ret, i;
 
 	for (i = 0; i < tcount; i++) {
-		ret = do_test_rsa(tfm, vecs++);
-		if (ret) {
-			pr_err("alg: rsa: test failed on vector %d, err=%d\n",
-			       i + 1, ret);
-			return ret;
-		}
-	}
-	return 0;
-}
-
-static int test_akcipher(struct crypto_akcipher *tfm, const char *alg,
-			 struct akcipher_testvec *vecs, unsigned int tcount)
-{
-	if (strncmp(alg, "rsa", 3) == 0)
-		return test_rsa(tfm, vecs, tcount);
+		ret = test_akcipher_one(tfm, vecs++);
+		if (!ret)
+			continue;
 
+		pr_err("alg: akcipher: test failed on vector %d, err=%d\n",
+		       i + 1, ret);
+		return ret;
+	}
 	return 0;
 }
 

[v3 PATCH 2/8] crypto: rsa - Generate fixed-length output

[Herbert Xu]

Every implementation of RSA that we have naturally generates
output with leading zeroes.  The one and only user of RSA,
pkcs1pad wants to have those leading zeroes in place, in fact
because they are currently absent it has to write those zeroes
itself.

So we shouldn't be stripping leading zeroes in the first place.
In fact this patch makes rsa-generic produce output with fixed
length so that pkcs1pad does not need to do any extra work.

This patch also changes DH to use the new interface.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa.c        |    8 +++----
 include/linux/mpi.h |    2 -
 lib/mpi/mpicoder.c  |   55 ++++++++++++++++++++++++----------------------------
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/crypto/rsa.c b/crypto/rsa.c
index dc692d4..4c280b6 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -108,7 +108,7 @@ static int rsa_enc(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(c, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(c, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -147,7 +147,7 @@ static int rsa_dec(struct akcipher_request *req)
 	if (ret)
 		goto err_free_c;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_c;
 
@@ -185,7 +185,7 @@ static int rsa_sign(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(s, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(s, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -226,7 +226,7 @@ static int rsa_verify(struct akcipher_request *req)
 	if (ret)
 		goto err_free_s;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_s;
 
diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index f219559..1cc5ffb 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -80,7 +80,7 @@ void *mpi_get_buffer(MPI a, unsigned *nbytes, int *sign);
 int mpi_read_buffer(MPI a, uint8_t *buf, unsigned buf_len, unsigned *nbytes,
 		    int *sign);
 void *mpi_get_secure_buffer(MPI a, unsigned *nbytes, int *sign);
-int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned nbytes,
 		     int *sign);
 
 #define log_mpidump g10_log_mpidump
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 823cf5f..7150e5c 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -237,16 +237,13 @@ EXPORT_SYMBOL_GPL(mpi_get_buffer);
  * @a:		a multi precision integer
  * @sgl:	scatterlist to write to. Needs to be at least
  *		mpi_get_size(a) long.
- * @nbytes:	in/out param - it has the be set to the maximum number of
- *		bytes that can be written to sgl. This has to be at least
- *		the size of the integer a. On return it receives the actual
- *		length of the data written on success or the data that would
- *		be written if buffer was too small.
+ * @nbytes:	the number of bytes to write.  Leading bytes will be
+ *		filled with zero.
  * @sign:	if not NULL, it will be set to the sign of a.
  *
  * Return:	0 on success or error code in case of error
  */
-int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		     int *sign)
 {
 	u8 *p, *p2;
@@ -258,43 +255,44 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
-	int i, x, y = 0, lzeros, buf_len;
-
-	if (!nbytes)
-		return -EINVAL;
+	int i, x, buf_len;
 
 	if (sign)
 		*sign = a->sign;
 
-	lzeros = count_lzeros(a);
-
-	if (*nbytes < n - lzeros) {
-		*nbytes = n - lzeros;
+	if (nbytes < n)
 		return -EOVERFLOW;
-	}
 
-	*nbytes = n - lzeros;
 	buf_len = sgl->length;
 	p2 = sg_virt(sgl);
 
-	for (i = a->nlimbs - 1 - lzeros / BYTES_PER_MPI_LIMB,
-			lzeros %= BYTES_PER_MPI_LIMB;
-		i >= 0; i--) {
+	while (nbytes > n) {
+		if (!buf_len) {
+			sgl = sg_next(sgl);
+			if (!sgl)
+				return -EINVAL;
+			buf_len = sgl->length;
+			p2 = sg_virt(sgl);
+		}
+
+		i = min_t(unsigned, nbytes - n, buf_len);
+		memset(p2, 0, i);
+		p2 += i;
+		buf_len -= i;
+		nbytes -= i;
+	}
+
+	for (i = a->nlimbs - 1; i >= 0; i--) {
 #if BYTES_PER_MPI_LIMB == 4
-		alimb = cpu_to_be32(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be32(a->d[i]) : 0;
 #elif BYTES_PER_MPI_LIMB == 8
-		alimb = cpu_to_be64(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be64(a->d[i]) : 0;
 #else
 #error please implement for this limb size.
 #endif
-		if (lzeros) {
-			y = lzeros;
-			lzeros = 0;
-		}
-
-		p = (u8 *)&alimb + y;
+		p = (u8 *)&alimb;
 
-		for (x = 0; x < sizeof(alimb) - y; x++) {
+		for (x = 0; x < sizeof(alimb); x++) {
 			if (!buf_len) {
 				sgl = sg_next(sgl);
 				if (!sgl)
@@ -305,7 +303,6 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 			*p2++ = *p++;
 			buf_len--;
 		}
-		y = 0;
 	}
 	return 0;
 }

RE: [v3 PATCH 2/8] crypto: rsa - Generate fixed-length output

[Benedetto, Salvatore]

Hi Herbert,

> 
> This patch also changes DH to use the new interface.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
> 
>  crypto/rsa.c        |    8 +++----
>  include/linux/mpi.h |    2 -
>  lib/mpi/mpicoder.c  |   55 ++++++++++++++++++++++++-----------------------
> -----
>  3 files changed, 31 insertions(+), 34 deletions(-)
> 

I don't see the DH change in this patch.

Regards,
Salvatore

Re: [v3 PATCH 2/8] crypto: rsa - Generate fixed-length output

[Herbert Xu]

On Wed, Jun 29, 2016 at 11:23:06AM +0000, Benedetto, Salvatore wrote:
> Hi Herbert,
> 
> > 
> > This patch also changes DH to use the new interface.
> > 
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> > ---
> > 
> >  crypto/rsa.c        |    8 +++----
> >  include/linux/mpi.h |    2 -
> >  lib/mpi/mpicoder.c  |   55 ++++++++++++++++++++++++-----------------------
> > -----
> >  3 files changed, 31 insertions(+), 34 deletions(-)
> > 
> 
> I don't see the DH change in this patch.

Doh, one more posting is needed.

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

[v3 PATCH 3/8] lib/mpi: Do not do sg_virt

[Herbert Xu]

Currently the mpi SG helpers use sg_virt which is completely
broken.  It happens to work with normal kernel memory but will
fail with anything that is not linearly mapped.

This patch fixes this by using the SG iterator helpers.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 lib/mpi/mpicoder.c |   86 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 36 deletions(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 7150e5c..c6272ae 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/count_zeros.h>
 #include <linux/byteorder/generic.h>
+#include <linux/scatterlist.h>
 #include <linux/string.h>
 #include "mpi-internal.h"
 
@@ -255,7 +256,9 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
+	struct sg_mapping_iter miter;
 	int i, x, buf_len;
+	int nents;
 
 	if (sign)
 		*sign = a->sign;
@@ -263,23 +266,27 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 	if (nbytes < n)
 		return -EOVERFLOW;
 
-	buf_len = sgl->length;
-	p2 = sg_virt(sgl);
+	nents = sg_nents_for_len(sgl, nbytes);
+	if (nents < 0)
+		return -EINVAL;
 
-	while (nbytes > n) {
-		if (!buf_len) {
-			sgl = sg_next(sgl);
-			if (!sgl)
-				return -EINVAL;
-			buf_len = sgl->length;
-			p2 = sg_virt(sgl);
-		}
+	sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC | SG_MITER_TO_SG);
+	sg_miter_next(&miter);
+	buf_len = miter.length;
+	p2 = miter.addr;
 
+	while (nbytes > n) {
 		i = min_t(unsigned, nbytes - n, buf_len);
 		memset(p2, 0, i);
 		p2 += i;
-		buf_len -= i;
 		nbytes -= i;
+
+		buf_len -= i;
+		if (!buf_len) {
+			sg_miter_next(&miter);
+			buf_len = miter.length;
+			p2 = miter.addr;
+		}
 	}
 
 	for (i = a->nlimbs - 1; i >= 0; i--) {
@@ -293,17 +300,16 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		p = (u8 *)&alimb;
 
 		for (x = 0; x < sizeof(alimb); x++) {
-			if (!buf_len) {
-				sgl = sg_next(sgl);
-				if (!sgl)
-					return -EINVAL;
-				buf_len = sgl->length;
-				p2 = sg_virt(sgl);
-			}
 			*p2++ = *p++;
-			buf_len--;
+			if (!--buf_len) {
+				sg_miter_next(&miter);
+				buf_len = miter.length;
+				p2 = miter.addr;
+			}
 		}
 	}
+
+	sg_miter_stop(&miter);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
@@ -323,19 +329,23 @@ EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
  */
 MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 {
-	struct scatterlist *sg;
-	int x, i, j, z, lzeros, ents;
+	struct sg_mapping_iter miter;
 	unsigned int nbits, nlimbs;
+	int x, j, z, lzeros, ents;
+	unsigned int len;
+	const u8 *buff;
 	mpi_limb_t a;
 	MPI val = NULL;
 
-	lzeros = 0;
-	ents = sg_nents(sgl);
+	ents = sg_nents_for_len(sgl, nbytes);
+	if (ents < 0)
+		return NULL;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buff = sg_virt(sg);
-		int len = sg->length;
+	sg_miter_start(&miter, sgl, ents, SG_MITER_ATOMIC | SG_MITER_FROM_SG);
 
+	lzeros = 0;
+	len = 0;
+	while (nbytes > 0) {
 		while (len && !*buff) {
 			lzeros++;
 			len--;
@@ -345,12 +355,14 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 		if (len && *buff)
 			break;
 
-		ents--;
+		sg_miter_next(&miter);
+		buff = miter.addr;
+		len = miter.length;
+
 		nbytes -= lzeros;
 		lzeros = 0;
 	}
 
-	sgl = sg;
 	nbytes -= lzeros;
 	nbits = nbytes * 8;
 	if (nbits > MAX_EXTERN_MPI_BITS) {
@@ -359,8 +371,7 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	}
 
 	if (nbytes > 0)
-		nbits -= count_leading_zeros(*(u8 *)(sg_virt(sgl) + lzeros)) -
-			(BITS_PER_LONG - 8);
+		nbits -= count_leading_zeros(*buff) - (BITS_PER_LONG - 8);
 
 	nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
 	val = mpi_alloc(nlimbs);
@@ -379,21 +390,24 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	z = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
 	z %= BYTES_PER_MPI_LIMB;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buffer = sg_virt(sg) + lzeros;
-		int len = sg->length - lzeros;
-
+	for (;;) {
 		for (x = 0; x < len; x++) {
 			a <<= 8;
-			a |= *buffer++;
+			a |= *buff++;
 			if (((z + x + 1) % BYTES_PER_MPI_LIMB) == 0) {
 				val->d[j--] = a;
 				a = 0;
 			}
 		}
 		z += x;
-		lzeros = 0;
+
+		if (!sg_miter_next(&miter))
+			break;
+
+		buff = miter.addr;
+		len = miter.length;
 	}
+
 	return val;
 }
 EXPORT_SYMBOL_GPL(mpi_read_raw_from_sgl);

[v3 PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

The only user of rsa-pkcs1pad always uses the hash so there is
no reason to support the case of not having a hash.

This patch also changes the digest info lookup so that it is
only done once during template instantiation rather than on each
operation.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   83 ++++++++++++++++++--------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ead8dc0..5c1c78e 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -92,13 +92,12 @@ static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
 
 struct pkcs1pad_ctx {
 	struct crypto_akcipher *child;
-	const char *hash_name;
 	unsigned int key_size;
 };
 
 struct pkcs1pad_inst_ctx {
 	struct crypto_akcipher_spawn spawn;
-	const char *hash_name;
+	const struct rsa_asn1_template *digest_info;
 };
 
 struct pkcs1pad_request {
@@ -416,20 +415,16 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info = NULL;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	int err;
 	unsigned int ps_end, digest_size = 0;
 
 	if (!ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			return -EINVAL;
-
-		digest_size = digest_info->size;
-	}
+	digest_size = digest_info->size;
 
 	if (req->src_len + digest_size > ctx->key_size - 11)
 		return -EOVERFLOW;
@@ -462,10 +457,8 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	memset(req_ctx->in_buf + 1, 0xff, ps_end - 1);
 	req_ctx->in_buf[ps_end] = 0x00;
 
-	if (digest_info) {
-		memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
-		       digest_info->size);
-	}
+	memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
+	       digest_info->size);
 
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
@@ -499,7 +492,9 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	unsigned int pos;
 
 	if (err == -EOVERFLOW)
@@ -527,17 +522,11 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 		goto done;
 	pos++;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			goto done;
+	if (memcmp(req_ctx->out_buf + pos, digest_info->data,
+		   digest_info->size))
+		goto done;
 
-		if (memcmp(req_ctx->out_buf + pos, digest_info->data,
-			   digest_info->size))
-			goto done;
-
-		pos += digest_info->size;
-	}
+	pos += digest_info->size;
 
 	err = 0;
 
@@ -626,12 +615,11 @@ static int pkcs1pad_init_tfm(struct crypto_akcipher *tfm)
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct crypto_akcipher *child_tfm;
 
-	child_tfm = crypto_spawn_akcipher(akcipher_instance_ctx(inst));
+	child_tfm = crypto_spawn_akcipher(&ictx->spawn);
 	if (IS_ERR(child_tfm))
 		return PTR_ERR(child_tfm);
 
 	ctx->child = child_tfm;
-	ctx->hash_name = ictx->hash_name;
 	return 0;
 }
 
@@ -648,12 +636,12 @@ static void pkcs1pad_free(struct akcipher_instance *inst)
 	struct crypto_akcipher_spawn *spawn = &ctx->spawn;
 
 	crypto_drop_akcipher(spawn);
-	kfree(ctx->hash_name);
 	kfree(inst);
 }
 
 static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 {
+	const struct rsa_asn1_template *digest_info;
 	struct crypto_attr_type *algt;
 	struct akcipher_instance *inst;
 	struct pkcs1pad_inst_ctx *ctx;
@@ -676,7 +664,11 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	hash_name = crypto_attr_alg_name(tb[2]);
 	if (IS_ERR(hash_name))
-		hash_name = NULL;
+		return PTR_ERR(hash_name);
+
+	digest_info = rsa_lookup_asn1(hash_name);
+	if (!digest_info)
+		return -EINVAL;
 
 	inst = kzalloc(sizeof(*inst) + sizeof(*ctx), GFP_KERNEL);
 	if (!inst)
@@ -684,7 +676,7 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	ctx = akcipher_instance_ctx(inst);
 	spawn = &ctx->spawn;
-	ctx->hash_name = hash_name ? kstrdup(hash_name, GFP_KERNEL) : NULL;
+	ctx->digest_info = digest_info;
 
 	crypto_set_spawn(&spawn->base, akcipher_crypto_instance(inst));
 	err = crypto_grab_akcipher(spawn, rsa_alg_name, 0,
@@ -696,27 +688,14 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = -ENAMETOOLONG;
 
-	if (!hash_name) {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_name) >=
-					CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_driver_name) >=
-					CRYPTO_MAX_ALG_NAME)
+	if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)", rsa_alg->base.cra_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME ||
+	    snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)",
+		     rsa_alg->base.cra_driver_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME)
 		goto out_drop_alg;
-	} else {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_name, hash_name) >=
-				CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_driver_name, hash_name) >=
-					CRYPTO_MAX_ALG_NAME)
-		goto out_free_hash;
-	}
 
 	inst->alg.base.cra_flags = rsa_alg->base.cra_flags & CRYPTO_ALG_ASYNC;
 	inst->alg.base.cra_priority = rsa_alg->base.cra_priority;
@@ -738,12 +717,10 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = akcipher_register_instance(tmpl, inst);
 	if (err)
-		goto out_free_hash;
+		goto out_drop_alg;
 
 	return 0;
 
-out_free_hash:
-	kfree(ctx->hash_name);
 out_drop_alg:
 	crypto_drop_akcipher(spawn);
 out_free_inst:

[v3 PATCH 5/8] crypto: rsa-pkcs1pad - Remove bogus page splitting

[Herbert Xu]

The helper pkcs1pad_sg_set_buf tries to split a buffer that crosses
a page boundary into two SG entries.  This is unnecessary.  This
patch removes that.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 5c1c78e..d9baefb 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -103,7 +103,7 @@ struct pkcs1pad_inst_ctx {
 struct pkcs1pad_request {
 	struct akcipher_request child_req;
 
-	struct scatterlist in_sg[3], out_sg[2];
+	struct scatterlist in_sg[2], out_sg[1];
 	uint8_t *in_buf, *out_buf;
 };
 
@@ -163,19 +163,10 @@ static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
 static void pkcs1pad_sg_set_buf(struct scatterlist *sg, void *buf, size_t len,
 		struct scatterlist *next)
 {
-	int nsegs = next ? 1 : 0;
-
-	if (offset_in_page(buf) + len <= PAGE_SIZE) {
-		nsegs += 1;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg, buf, len);
-	} else {
-		nsegs += 2;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg + 0, buf, PAGE_SIZE - offset_in_page(buf));
-		sg_set_buf(sg + 1, buf + PAGE_SIZE - offset_in_page(buf),
-				offset_in_page(buf) + len - PAGE_SIZE);
-	}
+	int nsegs = next ? 2 : 1;
+
+	sg_init_table(sg, nsegs);
+	sg_set_buf(sg, buf, len);
 
 	if (next)
 		sg_chain(sg, nsegs, next);

[v3 PATCH 6/8] crypto: rsa-pkcs1pad - Always use GFP_KERNEL

[Herbert Xu]

We don't currently support using akcipher in atomic contexts,
so GFP_KERNEL should always be used.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index d9baefb..db19284 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -260,8 +260,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -274,9 +273,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -379,9 +376,7 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size ;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 
@@ -438,8 +433,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -454,9 +448,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -577,9 +569,7 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 

[v3 PATCH 7/8] crypto: rsa-pkcs1pad - Move key size check to setkey

[Herbert Xu]

Rather than repeatedly checking the key size on each operation,
we should be checking it once when the key is set.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   56 +++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index db19284..ebd8514 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -111,40 +111,48 @@ static int pkcs1pad_set_pub_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_pub_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_set_priv_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_priv_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
@@ -247,9 +255,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -367,9 +372,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;
@@ -420,9 +422,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -560,9 +559,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;

[v3 PATCH 8/8] crypto: rsa-pkcs1pad - Avoid copying output when possible

[Herbert Xu]

In the vast majority of cases (2^-32 on 32-bit and 2^-64 on 64-bit)
cases, the result from encryption/signing will require no padding.

This patch makes these two operations write their output directly
to the final destination.  Only in the exceedingly rare cases where
fixup is needed to we copy it out and back to add the leading zeroes.

This patch also makes use of the crypto_akcipher_set_crypt API
instead of writing the akcipher request directly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |  112 ++++++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 67 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ebd8514..8ccfdd7 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -185,37 +185,36 @@ static int pkcs1pad_encrypt_sign_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	size_t pad_len = ctx->key_size - req_ctx->child_req.dst_len;
-	size_t chunk_len, pad_left;
-	struct sg_mapping_iter miter;
-
-	if (!err) {
-		if (pad_len) {
-			sg_miter_start(&miter, req->dst,
-					sg_nents_for_len(req->dst, pad_len),
-					SG_MITER_ATOMIC | SG_MITER_TO_SG);
-
-			pad_left = pad_len;
-			while (pad_left) {
-				sg_miter_next(&miter);
-
-				chunk_len = min(miter.length, pad_left);
-				memset(miter.addr, 0, chunk_len);
-				pad_left -= chunk_len;
-			}
-
-			sg_miter_stop(&miter);
-		}
-
-		sg_pcopy_from_buffer(req->dst,
-				sg_nents_for_len(req->dst, ctx->key_size),
-				req_ctx->out_buf, req_ctx->child_req.dst_len,
-				pad_len);
-	}
+	unsigned int pad_len;
+	unsigned int len;
+	u8 *out_buf;
+
+	if (err)
+		goto out;
+
+	len = req_ctx->child_req.dst_len;
+	pad_len = ctx->key_size - len;
+
+	/* Four billion to one */
+	if (likely(!pad_len))
+		goto out;
+
+	out_buf = kzalloc(ctx->key_size, GFP_ATOMIC);
+	err = -ENOMEM;
+	if (!out_buf)
+		goto out;
+
+	sg_copy_to_buffer(req->dst, sg_nents_for_len(req->dst, len),
+			  out_buf + pad_len, len);
+	sg_copy_from_buffer(req->dst,
+			    sg_nents_for_len(req->dst, ctx->key_size),
+			    out_buf, ctx->key_size);
+	kzfree(out_buf);
+
+out:
 	req->dst_len = ctx->key_size;
 
 	kfree(req_ctx->in_buf);
-	kzfree(req_ctx->out_buf);
 
 	return err;
 }
@@ -255,15 +254,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -291,6 +281,10 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_encrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -372,12 +366,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size ;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -389,6 +377,11 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_decrypt_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_decrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -422,15 +415,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -447,19 +431,14 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
-	if (!req_ctx->out_buf) {
-		kfree(req_ctx->in_buf);
-		return -ENOMEM;
-	}
-
-	pkcs1pad_sg_set_buf(req_ctx->out_sg, req_ctx->out_buf,
-			ctx->key_size, NULL);
-
 	akcipher_request_set_tfm(&req_ctx->child_req, ctx->child);
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_sign(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -559,12 +538,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -576,6 +549,11 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_verify_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_verify(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||

[v4 PATCH 0/8] crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

Hi:

This was prompted by the caam RSA submission where a lot of work
was done just to strip the RSA output of leading zeroes.  This is
in fact completely pointless because the only user of RSA in the
kernel then promptly puts them back.

This patch series resolves this madness by simply leaving any
leading zeroes in place.  Note that we're not requiring authors
to add leading zeroes, even though that is encouraged if it is
easy to do.  In practice you'd only run into this every 2^32 or
2^64 operations so please don't overdo it.

I've also taken the opportunity to cleanup the pkcs1pad code.

v4 fixes the newly added dh to use the new MPI SG interface.

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

[v4 PATCH 1/8] crypto: testmgr - Allow leading zeros in RSA

[Herbert Xu]

This patch allows RSA implementations to produce output with
leading zeroes.  testmgr will skip leading zeroes when comparing
the output.

This patch also tries to make the RSA test function generic enough
to potentially handle other akcipher algorithms.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/testmgr.c |   51 ++++++++++++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 537fdc3..38e23be31 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1911,8 +1911,8 @@ static int alg_test_kpp(const struct alg_test_desc *desc, const char *driver,
 	return err;
 }
 
-static int do_test_rsa(struct crypto_akcipher *tfm,
-		       struct akcipher_testvec *vecs)
+static int test_akcipher_one(struct crypto_akcipher *tfm,
+			     struct akcipher_testvec *vecs)
 {
 	char *xbuf[XBUFSIZE];
 	struct akcipher_request *req;
@@ -1963,17 +1963,18 @@ static int do_test_rsa(struct crypto_akcipher *tfm,
 	/* Run RSA encrypt - c = m^e mod n;*/
 	err = wait_async_op(&result, crypto_akcipher_encrypt(req));
 	if (err) {
-		pr_err("alg: rsa: encrypt test failed. err %d\n", err);
+		pr_err("alg: akcipher: encrypt test failed. err %d\n", err);
 		goto free_all;
 	}
 	if (req->dst_len != vecs->c_size) {
-		pr_err("alg: rsa: encrypt test failed. Invalid output len\n");
+		pr_err("alg: akcipher: encrypt test failed. Invalid output len\n");
 		err = -EINVAL;
 		goto free_all;
 	}
 	/* verify that encrypted message is equal to expected */
 	if (memcmp(vecs->c, outbuf_enc, vecs->c_size)) {
-		pr_err("alg: rsa: encrypt test failed. Invalid output\n");
+		pr_err("alg: akcipher: encrypt test failed. Invalid output\n");
+		hexdump(outbuf_enc, vecs->c_size);
 		err = -EINVAL;
 		goto free_all;
 	}
@@ -2001,18 +2002,22 @@ static int do_test_rsa(struct crypto_akcipher *tfm,
 	/* Run RSA decrypt - m = c^d mod n;*/
 	err = wait_async_op(&result, crypto_akcipher_decrypt(req));
 	if (err) {
-		pr_err("alg: rsa: decrypt test failed. err %d\n", err);
+		pr_err("alg: akcipher: decrypt test failed. err %d\n", err);
 		goto free_all;
 	}
 	out_len = req->dst_len;
-	if (out_len != vecs->m_size) {
-		pr_err("alg: rsa: decrypt test failed. Invalid output len\n");
+	if (out_len < vecs->m_size) {
+		pr_err("alg: akcipher: decrypt test failed. "
+		       "Invalid output len %u\n", out_len);
 		err = -EINVAL;
 		goto free_all;
 	}
 	/* verify that decrypted message is equal to the original msg */
-	if (memcmp(vecs->m, outbuf_dec, vecs->m_size)) {
-		pr_err("alg: rsa: decrypt test failed. Invalid output\n");
+	if (memchr_inv(outbuf_dec, 0, out_len - vecs->m_size) ||
+	    memcmp(vecs->m, outbuf_dec + out_len - vecs->m_size,
+		   vecs->m_size)) {
+		pr_err("alg: akcipher: decrypt test failed. Invalid output\n");
+		hexdump(outbuf_dec, out_len);
 		err = -EINVAL;
 	}
 free_all:
@@ -2025,28 +2030,20 @@ free_xbuf:
 	return err;
 }
 
-static int test_rsa(struct crypto_akcipher *tfm, struct akcipher_testvec *vecs,
-		    unsigned int tcount)
+static int test_akcipher(struct crypto_akcipher *tfm, const char *alg,
+			 struct akcipher_testvec *vecs, unsigned int tcount)
 {
 	int ret, i;
 
 	for (i = 0; i < tcount; i++) {
-		ret = do_test_rsa(tfm, vecs++);
-		if (ret) {
-			pr_err("alg: rsa: test failed on vector %d, err=%d\n",
-			       i + 1, ret);
-			return ret;
-		}
-	}
-	return 0;
-}
-
-static int test_akcipher(struct crypto_akcipher *tfm, const char *alg,
-			 struct akcipher_testvec *vecs, unsigned int tcount)
-{
-	if (strncmp(alg, "rsa", 3) == 0)
-		return test_rsa(tfm, vecs, tcount);
+		ret = test_akcipher_one(tfm, vecs++);
+		if (!ret)
+			continue;
 
+		pr_err("alg: akcipher: test failed on vector %d, err=%d\n",
+		       i + 1, ret);
+		return ret;
+	}
 	return 0;
 }
 

[v4 PATCH 2/8] crypto: rsa - Generate fixed-length output

[Herbert Xu]

Every implementation of RSA that we have naturally generates
output with leading zeroes.  The one and only user of RSA,
pkcs1pad wants to have those leading zeroes in place, in fact
because they are currently absent it has to write those zeroes
itself.

So we shouldn't be stripping leading zeroes in the first place.
In fact this patch makes rsa-generic produce output with fixed
length so that pkcs1pad does not need to do any extra work.

This patch also changes DH to use the new interface.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/dh.c         |    2 -
 crypto/rsa.c        |    8 +++----
 include/linux/mpi.h |    2 -
 lib/mpi/mpicoder.c  |   55 ++++++++++++++++++++++++----------------------------
 4 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 5e960fe..9d19360 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -129,7 +129,7 @@ static int dh_compute_value(struct kpp_request *req)
 	if (ret)
 		goto err_free_base;
 
-	ret = mpi_write_to_sgl(val, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(val, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_base;
 
diff --git a/crypto/rsa.c b/crypto/rsa.c
index dc692d4..4c280b6 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -108,7 +108,7 @@ static int rsa_enc(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(c, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(c, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -147,7 +147,7 @@ static int rsa_dec(struct akcipher_request *req)
 	if (ret)
 		goto err_free_c;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_c;
 
@@ -185,7 +185,7 @@ static int rsa_sign(struct akcipher_request *req)
 	if (ret)
 		goto err_free_m;
 
-	ret = mpi_write_to_sgl(s, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(s, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_m;
 
@@ -226,7 +226,7 @@ static int rsa_verify(struct akcipher_request *req)
 	if (ret)
 		goto err_free_s;
 
-	ret = mpi_write_to_sgl(m, req->dst, &req->dst_len, &sign);
+	ret = mpi_write_to_sgl(m, req->dst, req->dst_len, &sign);
 	if (ret)
 		goto err_free_s;
 
diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index f219559..1cc5ffb 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -80,7 +80,7 @@ void *mpi_get_buffer(MPI a, unsigned *nbytes, int *sign);
 int mpi_read_buffer(MPI a, uint8_t *buf, unsigned buf_len, unsigned *nbytes,
 		    int *sign);
 void *mpi_get_secure_buffer(MPI a, unsigned *nbytes, int *sign);
-int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned nbytes,
 		     int *sign);
 
 #define log_mpidump g10_log_mpidump
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 823cf5f..7150e5c 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -237,16 +237,13 @@ EXPORT_SYMBOL_GPL(mpi_get_buffer);
  * @a:		a multi precision integer
  * @sgl:	scatterlist to write to. Needs to be at least
  *		mpi_get_size(a) long.
- * @nbytes:	in/out param - it has the be set to the maximum number of
- *		bytes that can be written to sgl. This has to be at least
- *		the size of the integer a. On return it receives the actual
- *		length of the data written on success or the data that would
- *		be written if buffer was too small.
+ * @nbytes:	the number of bytes to write.  Leading bytes will be
+ *		filled with zero.
  * @sign:	if not NULL, it will be set to the sign of a.
  *
  * Return:	0 on success or error code in case of error
  */
-int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
+int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		     int *sign)
 {
 	u8 *p, *p2;
@@ -258,43 +255,44 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
-	int i, x, y = 0, lzeros, buf_len;
-
-	if (!nbytes)
-		return -EINVAL;
+	int i, x, buf_len;
 
 	if (sign)
 		*sign = a->sign;
 
-	lzeros = count_lzeros(a);
-
-	if (*nbytes < n - lzeros) {
-		*nbytes = n - lzeros;
+	if (nbytes < n)
 		return -EOVERFLOW;
-	}
 
-	*nbytes = n - lzeros;
 	buf_len = sgl->length;
 	p2 = sg_virt(sgl);
 
-	for (i = a->nlimbs - 1 - lzeros / BYTES_PER_MPI_LIMB,
-			lzeros %= BYTES_PER_MPI_LIMB;
-		i >= 0; i--) {
+	while (nbytes > n) {
+		if (!buf_len) {
+			sgl = sg_next(sgl);
+			if (!sgl)
+				return -EINVAL;
+			buf_len = sgl->length;
+			p2 = sg_virt(sgl);
+		}
+
+		i = min_t(unsigned, nbytes - n, buf_len);
+		memset(p2, 0, i);
+		p2 += i;
+		buf_len -= i;
+		nbytes -= i;
+	}
+
+	for (i = a->nlimbs - 1; i >= 0; i--) {
 #if BYTES_PER_MPI_LIMB == 4
-		alimb = cpu_to_be32(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be32(a->d[i]) : 0;
 #elif BYTES_PER_MPI_LIMB == 8
-		alimb = cpu_to_be64(a->d[i]);
+		alimb = a->d[i] ? cpu_to_be64(a->d[i]) : 0;
 #else
 #error please implement for this limb size.
 #endif
-		if (lzeros) {
-			y = lzeros;
-			lzeros = 0;
-		}
-
-		p = (u8 *)&alimb + y;
+		p = (u8 *)&alimb;
 
-		for (x = 0; x < sizeof(alimb) - y; x++) {
+		for (x = 0; x < sizeof(alimb); x++) {
 			if (!buf_len) {
 				sgl = sg_next(sgl);
 				if (!sgl)
@@ -305,7 +303,6 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned *nbytes,
 			*p2++ = *p++;
 			buf_len--;
 		}
-		y = 0;
 	}
 	return 0;
 }

[v4 PATCH 3/8] lib/mpi: Do not do sg_virt

[Herbert Xu]

Currently the mpi SG helpers use sg_virt which is completely
broken.  It happens to work with normal kernel memory but will
fail with anything that is not linearly mapped.

This patch fixes this by using the SG iterator helpers.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 lib/mpi/mpicoder.c |   86 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 36 deletions(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 7150e5c..c6272ae 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/count_zeros.h>
 #include <linux/byteorder/generic.h>
+#include <linux/scatterlist.h>
 #include <linux/string.h>
 #include "mpi-internal.h"
 
@@ -255,7 +256,9 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 #error please implement for this limb size.
 #endif
 	unsigned int n = mpi_get_size(a);
+	struct sg_mapping_iter miter;
 	int i, x, buf_len;
+	int nents;
 
 	if (sign)
 		*sign = a->sign;
@@ -263,23 +266,27 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 	if (nbytes < n)
 		return -EOVERFLOW;
 
-	buf_len = sgl->length;
-	p2 = sg_virt(sgl);
+	nents = sg_nents_for_len(sgl, nbytes);
+	if (nents < 0)
+		return -EINVAL;
 
-	while (nbytes > n) {
-		if (!buf_len) {
-			sgl = sg_next(sgl);
-			if (!sgl)
-				return -EINVAL;
-			buf_len = sgl->length;
-			p2 = sg_virt(sgl);
-		}
+	sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC | SG_MITER_TO_SG);
+	sg_miter_next(&miter);
+	buf_len = miter.length;
+	p2 = miter.addr;
 
+	while (nbytes > n) {
 		i = min_t(unsigned, nbytes - n, buf_len);
 		memset(p2, 0, i);
 		p2 += i;
-		buf_len -= i;
 		nbytes -= i;
+
+		buf_len -= i;
+		if (!buf_len) {
+			sg_miter_next(&miter);
+			buf_len = miter.length;
+			p2 = miter.addr;
+		}
 	}
 
 	for (i = a->nlimbs - 1; i >= 0; i--) {
@@ -293,17 +300,16 @@ int mpi_write_to_sgl(MPI a, struct scatterlist *sgl, unsigned nbytes,
 		p = (u8 *)&alimb;
 
 		for (x = 0; x < sizeof(alimb); x++) {
-			if (!buf_len) {
-				sgl = sg_next(sgl);
-				if (!sgl)
-					return -EINVAL;
-				buf_len = sgl->length;
-				p2 = sg_virt(sgl);
-			}
 			*p2++ = *p++;
-			buf_len--;
+			if (!--buf_len) {
+				sg_miter_next(&miter);
+				buf_len = miter.length;
+				p2 = miter.addr;
+			}
 		}
 	}
+
+	sg_miter_stop(&miter);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
@@ -323,19 +329,23 @@ EXPORT_SYMBOL_GPL(mpi_write_to_sgl);
  */
 MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 {
-	struct scatterlist *sg;
-	int x, i, j, z, lzeros, ents;
+	struct sg_mapping_iter miter;
 	unsigned int nbits, nlimbs;
+	int x, j, z, lzeros, ents;
+	unsigned int len;
+	const u8 *buff;
 	mpi_limb_t a;
 	MPI val = NULL;
 
-	lzeros = 0;
-	ents = sg_nents(sgl);
+	ents = sg_nents_for_len(sgl, nbytes);
+	if (ents < 0)
+		return NULL;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buff = sg_virt(sg);
-		int len = sg->length;
+	sg_miter_start(&miter, sgl, ents, SG_MITER_ATOMIC | SG_MITER_FROM_SG);
 
+	lzeros = 0;
+	len = 0;
+	while (nbytes > 0) {
 		while (len && !*buff) {
 			lzeros++;
 			len--;
@@ -345,12 +355,14 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 		if (len && *buff)
 			break;
 
-		ents--;
+		sg_miter_next(&miter);
+		buff = miter.addr;
+		len = miter.length;
+
 		nbytes -= lzeros;
 		lzeros = 0;
 	}
 
-	sgl = sg;
 	nbytes -= lzeros;
 	nbits = nbytes * 8;
 	if (nbits > MAX_EXTERN_MPI_BITS) {
@@ -359,8 +371,7 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	}
 
 	if (nbytes > 0)
-		nbits -= count_leading_zeros(*(u8 *)(sg_virt(sgl) + lzeros)) -
-			(BITS_PER_LONG - 8);
+		nbits -= count_leading_zeros(*buff) - (BITS_PER_LONG - 8);
 
 	nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
 	val = mpi_alloc(nlimbs);
@@ -379,21 +390,24 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	z = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
 	z %= BYTES_PER_MPI_LIMB;
 
-	for_each_sg(sgl, sg, ents, i) {
-		const u8 *buffer = sg_virt(sg) + lzeros;
-		int len = sg->length - lzeros;
-
+	for (;;) {
 		for (x = 0; x < len; x++) {
 			a <<= 8;
-			a |= *buffer++;
+			a |= *buff++;
 			if (((z + x + 1) % BYTES_PER_MPI_LIMB) == 0) {
 				val->d[j--] = a;
 				a = 0;
 			}
 		}
 		z += x;
-		lzeros = 0;
+
+		if (!sg_miter_next(&miter))
+			break;
+
+		buff = miter.addr;
+		len = miter.length;
 	}
+
 	return val;
 }
 EXPORT_SYMBOL_GPL(mpi_read_raw_from_sgl);

[v4 PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Herbert Xu]

The only user of rsa-pkcs1pad always uses the hash so there is
no reason to support the case of not having a hash.

This patch also changes the digest info lookup so that it is
only done once during template instantiation rather than on each
operation.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   83 ++++++++++++++++++--------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ead8dc0..5c1c78e 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -92,13 +92,12 @@ static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
 
 struct pkcs1pad_ctx {
 	struct crypto_akcipher *child;
-	const char *hash_name;
 	unsigned int key_size;
 };
 
 struct pkcs1pad_inst_ctx {
 	struct crypto_akcipher_spawn spawn;
-	const char *hash_name;
+	const struct rsa_asn1_template *digest_info;
 };
 
 struct pkcs1pad_request {
@@ -416,20 +415,16 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info = NULL;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	int err;
 	unsigned int ps_end, digest_size = 0;
 
 	if (!ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			return -EINVAL;
-
-		digest_size = digest_info->size;
-	}
+	digest_size = digest_info->size;
 
 	if (req->src_len + digest_size > ctx->key_size - 11)
 		return -EOVERFLOW;
@@ -462,10 +457,8 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	memset(req_ctx->in_buf + 1, 0xff, ps_end - 1);
 	req_ctx->in_buf[ps_end] = 0x00;
 
-	if (digest_info) {
-		memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
-		       digest_info->size);
-	}
+	memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
+	       digest_info->size);
 
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
@@ -499,7 +492,9 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	unsigned int pos;
 
 	if (err == -EOVERFLOW)
@@ -527,17 +522,11 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 		goto done;
 	pos++;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			goto done;
+	if (memcmp(req_ctx->out_buf + pos, digest_info->data,
+		   digest_info->size))
+		goto done;
 
-		if (memcmp(req_ctx->out_buf + pos, digest_info->data,
-			   digest_info->size))
-			goto done;
-
-		pos += digest_info->size;
-	}
+	pos += digest_info->size;
 
 	err = 0;
 
@@ -626,12 +615,11 @@ static int pkcs1pad_init_tfm(struct crypto_akcipher *tfm)
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct crypto_akcipher *child_tfm;
 
-	child_tfm = crypto_spawn_akcipher(akcipher_instance_ctx(inst));
+	child_tfm = crypto_spawn_akcipher(&ictx->spawn);
 	if (IS_ERR(child_tfm))
 		return PTR_ERR(child_tfm);
 
 	ctx->child = child_tfm;
-	ctx->hash_name = ictx->hash_name;
 	return 0;
 }
 
@@ -648,12 +636,12 @@ static void pkcs1pad_free(struct akcipher_instance *inst)
 	struct crypto_akcipher_spawn *spawn = &ctx->spawn;
 
 	crypto_drop_akcipher(spawn);
-	kfree(ctx->hash_name);
 	kfree(inst);
 }
 
 static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 {
+	const struct rsa_asn1_template *digest_info;
 	struct crypto_attr_type *algt;
 	struct akcipher_instance *inst;
 	struct pkcs1pad_inst_ctx *ctx;
@@ -676,7 +664,11 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	hash_name = crypto_attr_alg_name(tb[2]);
 	if (IS_ERR(hash_name))
-		hash_name = NULL;
+		return PTR_ERR(hash_name);
+
+	digest_info = rsa_lookup_asn1(hash_name);
+	if (!digest_info)
+		return -EINVAL;
 
 	inst = kzalloc(sizeof(*inst) + sizeof(*ctx), GFP_KERNEL);
 	if (!inst)
@@ -684,7 +676,7 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	ctx = akcipher_instance_ctx(inst);
 	spawn = &ctx->spawn;
-	ctx->hash_name = hash_name ? kstrdup(hash_name, GFP_KERNEL) : NULL;
+	ctx->digest_info = digest_info;
 
 	crypto_set_spawn(&spawn->base, akcipher_crypto_instance(inst));
 	err = crypto_grab_akcipher(spawn, rsa_alg_name, 0,
@@ -696,27 +688,14 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = -ENAMETOOLONG;
 
-	if (!hash_name) {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_name) >=
-					CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_driver_name) >=
-					CRYPTO_MAX_ALG_NAME)
+	if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)", rsa_alg->base.cra_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME ||
+	    snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)",
+		     rsa_alg->base.cra_driver_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME)
 		goto out_drop_alg;
-	} else {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_name, hash_name) >=
-				CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_driver_name, hash_name) >=
-					CRYPTO_MAX_ALG_NAME)
-		goto out_free_hash;
-	}
 
 	inst->alg.base.cra_flags = rsa_alg->base.cra_flags & CRYPTO_ALG_ASYNC;
 	inst->alg.base.cra_priority = rsa_alg->base.cra_priority;
@@ -738,12 +717,10 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = akcipher_register_instance(tmpl, inst);
 	if (err)
-		goto out_free_hash;
+		goto out_drop_alg;
 
 	return 0;
 
-out_free_hash:
-	kfree(ctx->hash_name);
 out_drop_alg:
 	crypto_drop_akcipher(spawn);
 out_free_inst:

[v4 PATCH 5/8] crypto: rsa-pkcs1pad - Remove bogus page splitting

[Herbert Xu]

The helper pkcs1pad_sg_set_buf tries to split a buffer that crosses
a page boundary into two SG entries.  This is unnecessary.  This
patch removes that.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 5c1c78e..d9baefb 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -103,7 +103,7 @@ struct pkcs1pad_inst_ctx {
 struct pkcs1pad_request {
 	struct akcipher_request child_req;
 
-	struct scatterlist in_sg[3], out_sg[2];
+	struct scatterlist in_sg[2], out_sg[1];
 	uint8_t *in_buf, *out_buf;
 };
 
@@ -163,19 +163,10 @@ static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
 static void pkcs1pad_sg_set_buf(struct scatterlist *sg, void *buf, size_t len,
 		struct scatterlist *next)
 {
-	int nsegs = next ? 1 : 0;
-
-	if (offset_in_page(buf) + len <= PAGE_SIZE) {
-		nsegs += 1;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg, buf, len);
-	} else {
-		nsegs += 2;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg + 0, buf, PAGE_SIZE - offset_in_page(buf));
-		sg_set_buf(sg + 1, buf + PAGE_SIZE - offset_in_page(buf),
-				offset_in_page(buf) + len - PAGE_SIZE);
-	}
+	int nsegs = next ? 2 : 1;
+
+	sg_init_table(sg, nsegs);
+	sg_set_buf(sg, buf, len);
 
 	if (next)
 		sg_chain(sg, nsegs, next);

[v4 PATCH 6/8] crypto: rsa-pkcs1pad - Always use GFP_KERNEL

[Herbert Xu]

We don't currently support using akcipher in atomic contexts,
so GFP_KERNEL should always be used.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index d9baefb..db19284 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -260,8 +260,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -274,9 +273,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -379,9 +376,7 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size ;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 
@@ -438,8 +433,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -454,9 +448,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -577,9 +569,7 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 

[v4 PATCH 7/8] crypto: rsa-pkcs1pad - Move key size check to setkey

[Herbert Xu]

Rather than repeatedly checking the key size on each operation,
we should be checking it once when the key is set.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   56 +++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index db19284..ebd8514 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -111,40 +111,48 @@ static int pkcs1pad_set_pub_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_pub_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_set_priv_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_priv_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
@@ -247,9 +255,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -367,9 +372,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;
@@ -420,9 +422,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -560,9 +559,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;

[v4 PATCH 8/8] crypto: rsa-pkcs1pad - Avoid copying output when possible

[Herbert Xu]

In the vast majority of cases (2^-32 on 32-bit and 2^-64 on 64-bit)
cases, the result from encryption/signing will require no padding.

This patch makes these two operations write their output directly
to the final destination.  Only in the exceedingly rare cases where
fixup is needed to we copy it out and back to add the leading zeroes.

This patch also makes use of the crypto_akcipher_set_crypt API
instead of writing the akcipher request directly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |  112 ++++++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 67 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ebd8514..8ccfdd7 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -185,37 +185,36 @@ static int pkcs1pad_encrypt_sign_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	size_t pad_len = ctx->key_size - req_ctx->child_req.dst_len;
-	size_t chunk_len, pad_left;
-	struct sg_mapping_iter miter;
-
-	if (!err) {
-		if (pad_len) {
-			sg_miter_start(&miter, req->dst,
-					sg_nents_for_len(req->dst, pad_len),
-					SG_MITER_ATOMIC | SG_MITER_TO_SG);
-
-			pad_left = pad_len;
-			while (pad_left) {
-				sg_miter_next(&miter);
-
-				chunk_len = min(miter.length, pad_left);
-				memset(miter.addr, 0, chunk_len);
-				pad_left -= chunk_len;
-			}
-
-			sg_miter_stop(&miter);
-		}
-
-		sg_pcopy_from_buffer(req->dst,
-				sg_nents_for_len(req->dst, ctx->key_size),
-				req_ctx->out_buf, req_ctx->child_req.dst_len,
-				pad_len);
-	}
+	unsigned int pad_len;
+	unsigned int len;
+	u8 *out_buf;
+
+	if (err)
+		goto out;
+
+	len = req_ctx->child_req.dst_len;
+	pad_len = ctx->key_size - len;
+
+	/* Four billion to one */
+	if (likely(!pad_len))
+		goto out;
+
+	out_buf = kzalloc(ctx->key_size, GFP_ATOMIC);
+	err = -ENOMEM;
+	if (!out_buf)
+		goto out;
+
+	sg_copy_to_buffer(req->dst, sg_nents_for_len(req->dst, len),
+			  out_buf + pad_len, len);
+	sg_copy_from_buffer(req->dst,
+			    sg_nents_for_len(req->dst, ctx->key_size),
+			    out_buf, ctx->key_size);
+	kzfree(out_buf);
+
+out:
 	req->dst_len = ctx->key_size;
 
 	kfree(req_ctx->in_buf);
-	kzfree(req_ctx->out_buf);
 
 	return err;
 }
@@ -255,15 +254,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -291,6 +281,10 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_encrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -372,12 +366,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size ;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -389,6 +377,11 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_decrypt_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_decrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -422,15 +415,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -447,19 +431,14 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
-	if (!req_ctx->out_buf) {
-		kfree(req_ctx->in_buf);
-		return -ENOMEM;
-	}
-
-	pkcs1pad_sg_set_buf(req_ctx->out_sg, req_ctx->out_buf,
-			ctx->key_size, NULL);
-
 	akcipher_request_set_tfm(&req_ctx->child_req, ctx->child);
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_sign(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -559,12 +538,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -576,6 +549,11 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_verify_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_verify(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||

Re: [v4 PATCH 0/8] crypto: rsa - Do not gratuitously drop leading zeroes

[Stephan Mueller]

Am Mittwoch, 29. Juni 2016, 19:31:25 CEST schrieb Herbert Xu:

Hi Herbert,

I re-tested that patch set and I still see the same issues as before, namely 
that sigver does not work:

Kernel log:

PKCS#7 signature not signed with a trusted key

And my CAVS harness also fails.

Is there any prerequisite to that patch that I have to consider?

> Hi:
> 
> This was prompted by the caam RSA submission where a lot of work
> was done just to strip the RSA output of leading zeroes.  This is
> in fact completely pointless because the only user of RSA in the
> kernel then promptly puts them back.
> 
> This patch series resolves this madness by simply leaving any
> leading zeroes in place.  Note that we're not requiring authors
> to add leading zeroes, even though that is encouraged if it is
> easy to do.  In practice you'd only run into this every 2^32 or
> 2^64 operations so please don't overdo it.
> 
> I've also taken the opportunity to cleanup the pkcs1pad code.
> 
> v4 fixes the newly added dh to use the new MPI SG interface.
> 
> Cheers,


Ciao
Stephan

Re: [v4 PATCH 0/8] crypto: rsa - Do not gratuitously drop leading zeroes

[Stephan Mueller]

Am Samstag, 2. Juli 2016, 19:55:59 CEST schrieb Stephan Mueller:

Hi Herbert,

> Am Mittwoch, 29. Juni 2016, 19:31:25 CEST schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> I re-tested that patch set and I still see the same issues as before, namely
> that sigver does not work:
> 
> Kernel log:
> 
> PKCS#7 signature not signed with a trusted key
> 
> And my CAVS harness also fails.
> 
> Is there any prerequisite to that patch that I have to consider?

Note, I see that even with the patch set that went into your cryptodev-2.6 
tree.

Ciao
Stephan

Re: [v4 PATCH 0/8] crypto: rsa - Do not gratuitously drop leading zeroes

[Herbert Xu]

On Sat, Jul 02, 2016 at 07:55:59PM +0200, Stephan Mueller wrote:
> 
> I re-tested that patch set and I still see the same issues as before, namely 
> that sigver does not work:

Oops, somehow I conflated this problem with KPP.  Does this patch
fix it for you?

---8<---
Subject: crypto: rsa-pkcs1pad - Fix regression from leading zeros

As the software RSA implementation now produces fixed-length
output, we need to eliminate leading zeros in the calling code
instead.

This patch does just that for pkcs1pad signature verification.

Fixes: 9b45b7bba3d2 ("crypto: rsa - Generate fixed-length output")
Reported-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 8ccfdd7..880d3db 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -456,49 +456,55 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
 	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
 	const struct rsa_asn1_template *digest_info = ictx->digest_info;
+	unsigned int dst_len;
 	unsigned int pos;
-
-	if (err == -EOVERFLOW)
-		/* Decrypted value had no leading 0 byte */
-		err = -EINVAL;
+	u8 *out_buf;
 
 	if (err)
 		goto done;
 
-	if (req_ctx->child_req.dst_len != ctx->key_size - 1) {
-		err = -EINVAL;
+	err = -EINVAL;
+	dst_len = req_ctx->child_req.dst_len;
+	if (dst_len < ctx->key_size - 1)
 		goto done;
+
+	out_buf = req_ctx->out_buf;
+	if (dst_len == ctx->key_size) {
+		if (out_buf[0] != 0x00)
+			/* Decrypted value had no leading 0 byte */
+			goto done;
+
+		dst_len--;
+		out_buf++;
 	}
 
 	err = -EBADMSG;
-	if (req_ctx->out_buf[0] != 0x01)
+	if (out_buf[0] != 0x01)
 		goto done;
 
-	for (pos = 1; pos < req_ctx->child_req.dst_len; pos++)
-		if (req_ctx->out_buf[pos] != 0xff)
+	for (pos = 1; pos < dst_len; pos++)
+		if (out_buf[pos] != 0xff)
 			break;
 
-	if (pos < 9 || pos == req_ctx->child_req.dst_len ||
-	    req_ctx->out_buf[pos] != 0x00)
+	if (pos < 9 || pos == dst_len || out_buf[pos] != 0x00)
 		goto done;
 	pos++;
 
-	if (memcmp(req_ctx->out_buf + pos, digest_info->data,
-		   digest_info->size))
+	if (memcmp(out_buf + pos, digest_info->data, digest_info->size))
 		goto done;
 
 	pos += digest_info->size;
 
 	err = 0;
 
-	if (req->dst_len < req_ctx->child_req.dst_len - pos)
+	if (req->dst_len < dst_len - pos)
 		err = -EOVERFLOW;
-	req->dst_len = req_ctx->child_req.dst_len - pos;
+	req->dst_len = dst_len - pos;
 
 	if (!err)
 		sg_copy_from_buffer(req->dst,
 				sg_nents_for_len(req->dst, req->dst_len),
-				req_ctx->out_buf + pos, req->dst_len);
+				out_buf + pos, req->dst_len);
 done:
 	kzfree(req_ctx->out_buf);
 
-- 
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

Re: [v4 PATCH 0/8] crypto: rsa - Do not gratuitously drop leading zeroes

[Stephan Mueller]

Am Sonntag, 3. Juli 2016, 10:46:11 CEST schrieb Herbert Xu:

Hi Herbert,

> On Sat, Jul 02, 2016 at 07:55:59PM +0200, Stephan Mueller wrote:
> > I re-tested that patch set and I still see the same issues as before,
> > namely
> > that sigver does not work:
> Oops, somehow I conflated this problem with KPP.  Does this patch
> fix it for you?

Yes, that patch fixes the module signature check issue and the CAVS test shows 
a pass as well.

Ciao
Stephan