All of lore.kernel.org
 help / color / mirror / Atom feed
* [6 PATCHes] IXP4xx crypto driver fixes.
@ 2010-01-10 15:34 Krzysztof Halasa
  2010-01-10 17:30 ` Krzysztof Halasa
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-10 15:34 UTC (permalink / raw)
  To: Herbert Xu, Christian Hohnstaedt, linux-crypto

Christian, Herbert,

I'm attaching 6 patches for IXP4xx crypto accelerator:

      IXP4xx: Fix ixp4xx_crypto little-endian operation.
      IXP4xx: Fix ixp4xx_crypto sparse warnings.
      IXP4xx: Simplify get_crypt_desc() and get_crypt_desc_emerg() in ixp4xx_crypto.
      IXP4xx: Fix whitespace problems in ixp4xx_crypto.
      ixp4xx_crypto: Fix possible NULL ptr dereference.
      ixp4xx_crypto: simplyfy the code a bit.

 drivers/crypto/ixp4xx_crypto.c |  219 +++++++++++++++++++---------------------
 1 files changed, 102 insertions(+), 117 deletions(-)


The first one fixes this on little-endian IXP425:

NPE-C: firmware functionality 0x5, revision 0x2:1
alg: skcipher: Test 1 failed on encryption for ecb(des)-ixp4xx
00000000: 01 23 45 67 89 ab cd e7
alg: skcipher: Test 1 failed on encryption for ecb(des3_ede)-ixp4xx
00000000: 73 6f 6d 65 64 61 74 61
alg: skcipher: Test 1 failed on encryption for ecb(aes)-ixp4xx
00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff

There are missing tests there, not sure if they should be implemented
but it's a different story:

alg: No test for ...
authenc(hmac(md5),cbc(des))-ixp4xx
authenc(hmac(md5),cbc(des3_ede))-ixp4xx
authenc(hmac(sha1),cbc(des))-ixp4xx
authenc(hmac(sha1),cbc(des3_ede))-ixp4xx
authenc(hmac(md5),cbc(aes))-ixp4xx
authenc(hmac(sha1),cbc(aes))-ixp4xx

Not tested in any practical application yet, just got rid of the
warnings.

Since the changes are more related to IXP425 than they are to the crypto
code, I can send the patches to Linus myself, or you can do that, your
call.
Please state your ACK.
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [6 PATCHes] IXP4xx crypto driver fixes.
  2010-01-10 15:34 [6 PATCHes] IXP4xx crypto driver fixes Krzysztof Halasa
@ 2010-01-10 17:30 ` Krzysztof Halasa
  2010-01-11  9:07   ` Christian Hohnstaedt
  2010-01-10 17:32 ` IXP4xx: Fix ixp4xx_crypto sparse warnings Krzysztof Halasa
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-10 17:30 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Christian Hohnstaedt, linux-crypto

IXP4xx: Fix ixp4xx_crypto little-endian operation.

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

Fixes the following on IXP425 little-endian:

NPE-C: firmware functionality 0x5, revision 0x2:1
alg: skcipher: Test 1 failed on encryption for ecb(des)-ixp4xx
00000000: 01 23 45 67 89 ab cd e7
alg: skcipher: Test 1 failed on encryption for ecb(des3_ede)-ixp4xx
00000000: 73 6f 6d 65 64 61 74 61
alg: skcipher: Test 1 failed on encryption for ecb(aes)-ixp4xx
00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index 6c6656d..cac026a 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -96,8 +96,13 @@
 
 struct buffer_desc {
 	u32 phys_next;
+#ifdef __ARMEB__
 	u16 buf_len;
 	u16 pkt_len;
+#else
+	u16 pkt_len;
+	u16 buf_len;
+#endif
 	u32 phys_addr;
 	u32 __reserved[4];
 	struct buffer_desc *next;
@@ -105,17 +110,30 @@ struct buffer_desc {
 };
 
 struct crypt_ctl {
+#ifdef __ARMEB__
 	u8 mode;		/* NPE_OP_*  operation mode */
 	u8 init_len;
 	u16 reserved;
+#else
+	u16 reserved;
+	u8 init_len;
+	u8 mode;		/* NPE_OP_*  operation mode */
+#endif
 	u8 iv[MAX_IVLEN];	/* IV for CBC mode or CTR IV for CTR mode */
 	u32 icv_rev_aes;	/* icv or rev aes */
 	u32 src_buf;
 	u32 dst_buf;
+#ifdef __ARMEB__
 	u16 auth_offs;		/* Authentication start offset */
 	u16 auth_len;		/* Authentication data length */
 	u16 crypt_offs;		/* Cryption start offset */
 	u16 crypt_len;		/* Cryption data length */
+#else
+	u16 auth_len;		/* Authentication data length */
+	u16 auth_offs;		/* Authentication start offset */
+	u16 crypt_len;		/* Cryption data length */
+	u16 crypt_offs;		/* Cryption start offset */
+#endif
 	u32 aadAddr;		/* Additional Auth Data Addr for CCM mode */
 	u32 crypto_ctx;		/* NPE Crypto Param structure address */
 
@@ -651,6 +669,9 @@ static int setup_auth(struct crypto_tfm *tfm, int encrypt, unsigned authsize,
 
 	/* write cfg word to cryptinfo */
 	cfgword = algo->cfgword | ( authsize << 6); /* (authsize/4) << 8 */
+#ifndef __ARMEB__
+	cfgword ^= 0xAA000000; /* change the "byte swap" flags */
+#endif
 	*(u32*)cinfo = cpu_to_be32(cfgword);
 	cinfo += sizeof(cfgword);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* IXP4xx: Fix ixp4xx_crypto sparse warnings.
  2010-01-10 15:34 [6 PATCHes] IXP4xx crypto driver fixes Krzysztof Halasa
  2010-01-10 17:30 ` Krzysztof Halasa
@ 2010-01-10 17:32 ` Krzysztof Halasa
  2010-01-10 17:32 ` IXP4xx: Simplify get_crypt_desc() and get_crypt_desc_emerg() in ixp4xx_crypto Krzysztof Halasa
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-10 17:32 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Christian Hohnstaedt, linux-crypto

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index cac026a..99f06e1 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -402,7 +402,7 @@ static void one_packet(dma_addr_t phys)
 		break;
 	case CTL_FLAG_GEN_REVAES:
 		ctx = crypto_tfm_ctx(crypt->data.tfm);
-		*(u32*)ctx->decrypt.npe_ctx &= cpu_to_be32(~CIPH_ENCR);
+		*(__be32 *)ctx->decrypt.npe_ctx &= cpu_to_be32(~CIPH_ENCR);
 		if (atomic_dec_and_test(&ctx->configuring))
 			complete(&ctx->completion);
 		break;
@@ -641,7 +641,7 @@ static int register_chain_var(struct crypto_tfm *tfm, u8 xpad, u32 target,
 	crypt->init_len = init_len;
 	crypt->ctl_flags |= CTL_FLAG_GEN_ICV;
 
-	buf->next = 0;
+	buf->next = NULL;
 	buf->buf_len = HMAC_PAD_BLOCKLEN;
 	buf->pkt_len = 0;
 	buf->phys_addr = pad_phys;
@@ -672,7 +672,7 @@ static int setup_auth(struct crypto_tfm *tfm, int encrypt, unsigned authsize,
 #ifndef __ARMEB__
 	cfgword ^= 0xAA000000; /* change the "byte swap" flags */
 #endif
-	*(u32*)cinfo = cpu_to_be32(cfgword);
+	*(__be32 *)cinfo = cpu_to_be32(cfgword);
 	cinfo += sizeof(cfgword);
 
 	/* write ICV to cryptinfo */
@@ -709,7 +709,7 @@ static int gen_rev_aes_key(struct crypto_tfm *tfm)
 	if (!crypt) {
 		return -EAGAIN;
 	}
-	*(u32*)dir->npe_ctx |= cpu_to_be32(CIPH_ENCR);
+	*(__be32 *)dir->npe_ctx |= cpu_to_be32(CIPH_ENCR);
 
 	crypt->data.tfm = tfm;
 	crypt->crypt_offs = 0;
@@ -771,7 +771,7 @@ static int setup_cipher(struct crypto_tfm *tfm, int encrypt,
 		}
 	}
 	/* write cfg word to cryptinfo */
-	*(u32*)cinfo = cpu_to_be32(cipher_cfg);
+	*(__be32 *)cinfo = cpu_to_be32(cipher_cfg);
 	cinfo += sizeof(cipher_cfg);
 
 	/* write cipher key to cryptinfo */
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* IXP4xx: Simplify get_crypt_desc() and get_crypt_desc_emerg() in ixp4xx_crypto.
  2010-01-10 15:34 [6 PATCHes] IXP4xx crypto driver fixes Krzysztof Halasa
  2010-01-10 17:30 ` Krzysztof Halasa
  2010-01-10 17:32 ` IXP4xx: Fix ixp4xx_crypto sparse warnings Krzysztof Halasa
@ 2010-01-10 17:32 ` Krzysztof Halasa
  2010-01-10 17:33 ` IXP4xx: Fix whitespace problems " Krzysztof Halasa
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-10 17:32 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Christian Hohnstaedt, linux-crypto

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index 99f06e1..0c7e4f5 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -277,26 +277,25 @@ static struct crypt_ctl *get_crypt_desc(void)
 	int i;
 	static int idx = 0;
 	unsigned long flags;
+	struct crypt_ctl *desc = NULL;
 
 	spin_lock_irqsave(&desc_lock, flags);
 
 	if (unlikely(!crypt_virt))
 		setup_crypt_desc();
-	if (unlikely(!crypt_virt)) {
-		spin_unlock_irqrestore(&desc_lock, flags);
-		return NULL;
-	}
+	if (unlikely(!crypt_virt))
+		goto out;
+
 	i = idx;
 	if (crypt_virt[i].ctl_flags == CTL_FLAG_UNUSED) {
 		if (++idx >= NPE_QLEN)
 			idx = 0;
 		crypt_virt[i].ctl_flags = CTL_FLAG_USED;
-		spin_unlock_irqrestore(&desc_lock, flags);
-		return crypt_virt +i;
-	} else {
-		spin_unlock_irqrestore(&desc_lock, flags);
-		return NULL;
+		desc = crypt_virt + i;
 	}
+out:
+	spin_unlock_irqrestore(&desc_lock, flags);
+	return desc;
 }
 
 static spinlock_t emerg_lock;
@@ -319,12 +318,10 @@ static struct crypt_ctl *get_crypt_desc_emerg(void)
 		if (++idx >= NPE_QLEN_TOTAL)
 			idx = NPE_QLEN;
 		crypt_virt[i].ctl_flags = CTL_FLAG_USED;
-		spin_unlock_irqrestore(&emerg_lock, flags);
-		return crypt_virt +i;
-	} else {
-		spin_unlock_irqrestore(&emerg_lock, flags);
-		return NULL;
+		desc = crypt_virt +i;
 	}
+	spin_unlock_irqrestore(&emerg_lock, flags);
+	return desc;
 }
 
 static void free_buf_chain(struct device *dev, struct buffer_desc *buf,u32 phys)
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* IXP4xx: Fix whitespace problems in ixp4xx_crypto.
  2010-01-10 15:34 [6 PATCHes] IXP4xx crypto driver fixes Krzysztof Halasa
                   ` (2 preceding siblings ...)
  2010-01-10 17:32 ` IXP4xx: Simplify get_crypt_desc() and get_crypt_desc_emerg() in ixp4xx_crypto Krzysztof Halasa
@ 2010-01-10 17:33 ` Krzysztof Halasa
  2010-01-12 17:12   ` Christian Hohnstaedt
  2010-01-10 17:37 ` ixp4xx_crypto: Fix possible NULL ptr dereference Krzysztof Halasa
  2010-01-10 17:37 ` ixp4xx_crypto: simplyfy the code a bit Krzysztof Halasa
  5 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-10 17:33 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Christian Hohnstaedt, linux-crypto

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index 0c7e4f5..f8f6515 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -64,7 +64,7 @@
 
 #define MOD_DES     0x0000
 #define MOD_TDEA2   0x0100
-#define MOD_3DES   0x0200
+#define MOD_3DES    0x0200
 #define MOD_AES     0x0800
 #define MOD_AES128  (0x0800 | KEYLEN_128)
 #define MOD_AES192  (0x0900 | KEYLEN_192)
@@ -137,7 +137,7 @@ struct crypt_ctl {
 	u32 aadAddr;		/* Additional Auth Data Addr for CCM mode */
 	u32 crypto_ctx;		/* NPE Crypto Param structure address */
 
-	/* Used by Host: 4*4 bytes*/
+	/* Used only by host: 4 * 4 bytes */
 	unsigned ctl_flags;
 	union {
 		struct ablkcipher_request *ablk_req;
@@ -208,10 +208,10 @@ static const struct ix_hash_algo hash_alg_sha1 = {
 };
 
 static struct npe *npe_c;
-static struct dma_pool *buffer_pool = NULL;
-static struct dma_pool *ctx_pool = NULL;
+static struct dma_pool *buffer_pool;
+static struct dma_pool *ctx_pool;
 
-static struct crypt_ctl *crypt_virt = NULL;
+static struct crypt_ctl *crypt_virt;
 static dma_addr_t crypt_phys;
 
 static int support_aes = 1;
@@ -246,12 +246,12 @@ static inline struct crypt_ctl *crypt_phys2virt(dma_addr_t phys)
 
 static inline u32 cipher_cfg_enc(struct crypto_tfm *tfm)
 {
-	return container_of(tfm->__crt_alg, struct ixp_alg,crypto)->cfg_enc;
+	return container_of(tfm->__crt_alg, struct ixp_alg, crypto)->cfg_enc;
 }
 
 static inline u32 cipher_cfg_dec(struct crypto_tfm *tfm)
 {
-	return container_of(tfm->__crt_alg, struct ixp_alg,crypto)->cfg_dec;
+	return container_of(tfm->__crt_alg, struct ixp_alg, crypto)->cfg_dec;
 }
 
 static inline const struct ix_hash_algo *ix_hash(struct crypto_tfm *tfm)
@@ -275,7 +275,7 @@ static spinlock_t desc_lock;
 static struct crypt_ctl *get_crypt_desc(void)
 {
 	int i;
-	static int idx = 0;
+	static int idx;
 	unsigned long flags;
 	struct crypt_ctl *desc = NULL;
 
@@ -318,13 +318,13 @@ static struct crypt_ctl *get_crypt_desc_emerg(void)
 		if (++idx >= NPE_QLEN_TOTAL)
 			idx = NPE_QLEN;
 		crypt_virt[i].ctl_flags = CTL_FLAG_USED;
-		desc = crypt_virt +i;
+		desc = crypt_virt + i;
 	}
 	spin_unlock_irqrestore(&emerg_lock, flags);
 	return desc;
 }
 
-static void free_buf_chain(struct device *dev, struct buffer_desc *buf,u32 phys)
+static void free_buf_chain(struct device *dev, struct buffer_desc *buf, u32 phys)
 {
 	while (buf) {
 		struct buffer_desc *buf1;
@@ -349,10 +349,9 @@ static void finish_scattered_hmac(struct crypt_ctl *crypt)
 	int authsize = crypto_aead_authsize(tfm);
 	int decryptlen = req->cryptlen - authsize;
 
-	if (req_ctx->encrypt) {
+	if (req_ctx->encrypt)
 		scatterwalk_map_and_copy(req_ctx->hmac_virt,
 			req->src, decryptlen, authsize, 1);
-	}
 	dma_pool_free(buffer_pool, req_ctx->hmac_virt, crypt->icv_rev_aes);
 }
 
@@ -372,9 +371,8 @@ static void one_packet(dma_addr_t phys)
 		struct aead_ctx *req_ctx = aead_request_ctx(req);
 
 		free_buf_chain(dev, req_ctx->buffer, crypt->src_buf);
-		if (req_ctx->hmac_virt) {
+		if (req_ctx->hmac_virt)
 			finish_scattered_hmac(crypt);
-		}
 		req->base.complete(&req->base, failed);
 		break;
 	}
@@ -382,9 +380,8 @@ static void one_packet(dma_addr_t phys)
 		struct ablkcipher_request *req = crypt->data.ablk_req;
 		struct ablk_ctx *req_ctx = ablkcipher_request_ctx(req);
 
-		if (req_ctx->dst) {
+		if (req_ctx->dst)
 			free_buf_chain(dev, req_ctx->dst, crypt->dst_buf);
-		}
 		free_buf_chain(dev, req_ctx->src, crypt->src_buf);
 		req->base.complete(&req->base, failed);
 		break;
@@ -418,7 +415,7 @@ static void crypto_done_action(unsigned long arg)
 {
 	int i;
 
-	for(i=0; i<4; i++) {
+	for (i = 0; i < 4; i++) {
 		dma_addr_t phys = qmgr_get_entry(RECV_QID);
 		if (!phys)
 			return;
@@ -443,9 +440,8 @@ static int init_ixp_crypto(void)
 
 	if (!npe_running(npe_c)) {
 		ret = npe_load_firmware(npe_c, npe_name(npe_c), dev);
-		if (ret) {
+		if (ret)
 			return ret;
-		}
 		if (npe_recv_message(npe_c, msg, "STATUS_MSG"))
 			goto npe_error;
 	} else {
@@ -478,14 +474,12 @@ static int init_ixp_crypto(void)
 	buffer_pool = dma_pool_create("buffer", dev,
 			sizeof(struct buffer_desc), 32, 0);
 	ret = -ENOMEM;
-	if (!buffer_pool) {
+	if (!buffer_pool)
 		goto err;
-	}
 	ctx_pool = dma_pool_create("context", dev,
 			NPE_CTX_LEN, 16, 0);
-	if (!ctx_pool) {
+	if (!ctx_pool)
 		goto err;
-	}
 	ret = qmgr_request_queue(SEND_QID, NPE_QLEN_TOTAL, 0, 0,
 				 "ixp_crypto:out", NULL);
 	if (ret)
@@ -527,11 +521,10 @@ static void release_ixp_crypto(void)
 
 	npe_release(npe_c);
 
-	if (crypt_virt) {
+	if (crypt_virt)
 		dma_free_coherent(dev,
-			NPE_QLEN_TOTAL * sizeof( struct crypt_ctl),
+			NPE_QLEN_TOTAL * sizeof(struct crypt_ctl),
 			crypt_virt, crypt_phys);
-	}
 	return;
 }
 
@@ -545,9 +538,8 @@ static void reset_sa_dir(struct ix_sa_dir *dir)
 static int init_sa_dir(struct ix_sa_dir *dir)
 {
 	dir->npe_ctx = dma_pool_alloc(ctx_pool, GFP_KERNEL, &dir->npe_ctx_phys);
-	if (!dir->npe_ctx) {
+	if (!dir->npe_ctx)
 		return -ENOMEM;
-	}
 	reset_sa_dir(dir);
 	return 0;
 }
@@ -568,9 +560,8 @@ static int init_tfm(struct crypto_tfm *tfm)
 	if (ret)
 		return ret;
 	ret = init_sa_dir(&ctx->decrypt);
-	if (ret) {
+	if (ret)
 		free_sa_dir(&ctx->encrypt);
-	}
 	return ret;
 }
 
@@ -621,9 +612,8 @@ static int register_chain_var(struct crypto_tfm *tfm, u8 xpad, u32 target,
 
 	memcpy(pad, key, key_len);
 	memset(pad + key_len, 0, HMAC_PAD_BLOCKLEN - key_len);
-	for (i = 0; i < HMAC_PAD_BLOCKLEN; i++) {
+	for (i = 0; i < HMAC_PAD_BLOCKLEN; i++)
 		pad[i] ^= xpad;
-	}
 
 	crypt->data.tfm = tfm;
 	crypt->regist_ptr = pad;
@@ -665,7 +655,7 @@ static int setup_auth(struct crypto_tfm *tfm, int encrypt, unsigned authsize,
 	algo = ix_hash(tfm);
 
 	/* write cfg word to cryptinfo */
-	cfgword = algo->cfgword | ( authsize << 6); /* (authsize/4) << 8 */
+	cfgword = algo->cfgword | (authsize << 6); /* (authsize/4) << 8 */
 #ifndef __ARMEB__
 	cfgword ^= 0xAA000000; /* change the "byte swap" flags */
 #endif
@@ -703,9 +693,8 @@ static int gen_rev_aes_key(struct crypto_tfm *tfm)
 	struct ix_sa_dir *dir = &ctx->decrypt;
 
 	crypt = get_crypt_desc_emerg();
-	if (!crypt) {
+	if (!crypt)
 		return -EAGAIN;
-	}
 	*(__be32 *)dir->npe_ctx |= cpu_to_be32(CIPH_ENCR);
 
 	crypt->data.tfm = tfm;
@@ -740,32 +729,30 @@ static int setup_cipher(struct crypto_tfm *tfm, int encrypt,
 	if (encrypt) {
 		cipher_cfg = cipher_cfg_enc(tfm);
 		dir->npe_mode |= NPE_OP_CRYPT_ENCRYPT;
-	} else {
+	} else
 		cipher_cfg = cipher_cfg_dec(tfm);
-	}
+
 	if (cipher_cfg & MOD_AES) {
 		switch (key_len) {
-			case 16: keylen_cfg = MOD_AES128 | KEYLEN_128; break;
-			case 24: keylen_cfg = MOD_AES192 | KEYLEN_192; break;
-			case 32: keylen_cfg = MOD_AES256 | KEYLEN_256; break;
-			default:
-				*flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
-				return -EINVAL;
+		case 16: keylen_cfg = MOD_AES128 | KEYLEN_128; break;
+		case 24: keylen_cfg = MOD_AES192 | KEYLEN_192; break;
+		case 32: keylen_cfg = MOD_AES256 | KEYLEN_256; break;
+		default:
+			*flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
+			return -EINVAL;
 		}
 		cipher_cfg |= keylen_cfg;
 	} else if (cipher_cfg & MOD_3DES) {
 		const u32 *K = (const u32 *)key;
 		if (unlikely(!((K[0] ^ K[2]) | (K[1] ^ K[3])) ||
-			     !((K[2] ^ K[4]) | (K[3] ^ K[5]))))
-		{
+			     !((K[2] ^ K[4]) | (K[3] ^ K[5])))) {
 			*flags |= CRYPTO_TFM_RES_BAD_KEY_SCHED;
 			return -EINVAL;
 		}
 	} else {
 		u32 tmp[DES_EXPKEY_WORDS];
-		if (des_ekey(tmp, key) == 0) {
+		if (des_ekey(tmp, key) == 0)
 			*flags |= CRYPTO_TFM_RES_WEAK_KEY;
-		}
 	}
 	/* write cfg word to cryptinfo */
 	*(__be32 *)cinfo = cpu_to_be32(cipher_cfg);
@@ -775,14 +762,13 @@ static int setup_cipher(struct crypto_tfm *tfm, int encrypt,
 	memcpy(cinfo, key, key_len);
 	/* NPE wants keylen set to DES3_EDE_KEY_SIZE even for single DES */
 	if (key_len < DES3_EDE_KEY_SIZE && !(cipher_cfg & MOD_AES)) {
-		memset(cinfo + key_len, 0, DES3_EDE_KEY_SIZE -key_len);
+		memset(cinfo + key_len, 0, DES3_EDE_KEY_SIZE - key_len);
 		key_len = DES3_EDE_KEY_SIZE;
 	}
 	dir->npe_ctx_idx = sizeof(cipher_cfg) + key_len;
 	dir->npe_mode |= NPE_OP_CRYPT_ENABLE;
-	if ((cipher_cfg & MOD_AES) && !encrypt) {
+	if ((cipher_cfg & MOD_AES) && !encrypt)
 		return gen_rev_aes_key(tfm);
-	}
 	return 0;
 }
 
@@ -791,7 +777,7 @@ static struct buffer_desc *chainup_buffers(struct device *dev,
 		struct buffer_desc *buf, gfp_t flags,
 		enum dma_data_direction dir)
 {
-	for (;nbytes > 0; sg = scatterwalk_sg_next(sg)) {
+	for (; nbytes > 0; sg = scatterwalk_sg_next(sg)) {
 		unsigned len = min(nbytes, sg->length);
 		struct buffer_desc *next_buf;
 		u32 next_buf_phys;
@@ -842,11 +828,10 @@ static int ablk_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
 		goto out;
 
 	if (*flags & CRYPTO_TFM_RES_WEAK_KEY) {
-		if (*flags & CRYPTO_TFM_REQ_WEAK_KEY) {
+		if (*flags & CRYPTO_TFM_REQ_WEAK_KEY)
 			ret = -EINVAL;
-		} else {
+		else
 			*flags &= ~CRYPTO_TFM_RES_WEAK_KEY;
-		}
 	}
 out:
 	if (!atomic_dec_and_test(&ctx->configuring))
@@ -918,9 +903,8 @@ static int ablk_perform(struct ablkcipher_request *req, int encrypt)
 		src_direction = DMA_TO_DEVICE;
 		req_ctx->dst = dst_hook.next;
 		crypt->dst_buf = dst_hook.phys_next;
-	} else {
+	} else
 		req_ctx->dst = NULL;
-	}
 	req_ctx->src = NULL;
 	if (!chainup_buffers(dev, req->src, nbytes, &src_hook,
 				flags, src_direction))
@@ -936,9 +920,8 @@ static int ablk_perform(struct ablkcipher_request *req, int encrypt)
 free_buf_src:
 	free_buf_chain(dev, req_ctx->src, crypt->src_buf);
 free_buf_dest:
-	if (req->src != req->dst) {
+	if (req->src != req->dst)
 		free_buf_chain(dev, req_ctx->dst, crypt->dst_buf);
-	}
 	crypt->ctl_flags = CTL_FLAG_UNUSED;
 	return -ENOMEM;
 }
@@ -962,7 +945,7 @@ static int ablk_rfc3686_crypt(struct ablkcipher_request *req)
 	int ret;
 
 	/* set up counter block */
-        memcpy(iv, ctx->nonce, CTR_RFC3686_NONCE_SIZE);
+	memcpy(iv, ctx->nonce, CTR_RFC3686_NONCE_SIZE);
 	memcpy(iv + CTR_RFC3686_NONCE_SIZE, info, CTR_RFC3686_IV_SIZE);
 
 	/* initialize counter portion of counter block */
@@ -1019,7 +1002,7 @@ static int aead_perform(struct aead_request *req, int encrypt,
 	} else {
 		dir = &ctx->decrypt;
 		/* req->cryptlen includes the authsize when decrypting */
-		cryptlen = req->cryptlen -authsize;
+		cryptlen = req->cryptlen - authsize;
 		eff_cryptlen -= authsize;
 	}
 	crypt = get_crypt_desc();
@@ -1039,9 +1022,8 @@ static int aead_perform(struct aead_request *req, int encrypt,
 	BUG_ON(ivsize && !req->iv);
 	memcpy(crypt->iv, req->iv, ivsize);
 
-	if (req->src != req->dst) {
+	if (req->src != req->dst)
 		BUG(); /* -ENOTSUP because of my lazyness */
-	}
 
 	/* ASSOC data */
 	buf = chainup_buffers(dev, req->assoc, req->assoclen, &src_hook,
@@ -1064,32 +1046,28 @@ static int aead_perform(struct aead_request *req, int encrypt,
 				&crypt->icv_rev_aes);
 		if (unlikely(!req_ctx->hmac_virt))
 			goto free_chain;
-		if (!encrypt) {
+		if (!encrypt)
 			scatterwalk_map_and_copy(req_ctx->hmac_virt,
 				req->src, cryptlen, authsize, 0);
-		}
 		req_ctx->encrypt = encrypt;
-	} else {
+	} else
 		req_ctx->hmac_virt = NULL;
-	}
 	/* Crypt */
 	buf = chainup_buffers(dev, req->src, cryptlen + authsize, buf, flags,
 			DMA_BIDIRECTIONAL);
 	if (!buf)
 		goto free_hmac_virt;
-	if (!req_ctx->hmac_virt) {
+	if (!req_ctx->hmac_virt)
 		crypt->icv_rev_aes = buf->phys_addr + buf->buf_len - authsize;
-	}
 
 	crypt->ctl_flags |= CTL_FLAG_PERFORM_AEAD;
 	qmgr_put_entry(SEND_QID, crypt_virt2phys(crypt));
 	BUG_ON(qmgr_stat_overflow(SEND_QID));
 	return -EINPROGRESS;
 free_hmac_virt:
-	if (req_ctx->hmac_virt) {
+	if (req_ctx->hmac_virt)
 		dma_pool_free(buffer_pool, req_ctx->hmac_virt,
 				crypt->icv_rev_aes);
-	}
 free_chain:
 	free_buf_chain(dev, req_ctx->buffer, crypt->src_buf);
 out:
@@ -1131,9 +1109,8 @@ static int aead_setup(struct crypto_aead *tfm, unsigned int authsize)
 		if (*flags & CRYPTO_TFM_REQ_WEAK_KEY) {
 			ret = -EINVAL;
 			goto out;
-		} else {
+		} else
 			*flags &= ~CRYPTO_TFM_RES_WEAK_KEY;
-		}
 	}
 out:
 	if (!atomic_dec_and_test(&ctx->configuring))
@@ -1219,7 +1196,7 @@ static int aead_givencrypt(struct aead_givcrypt_request *req)
 	seq = cpu_to_be64(req->seq);
 	memcpy(req->giv + ivsize - len, &seq, len);
 	return aead_perform(&req->areq, 1, req->areq.assoclen,
-			req->areq.cryptlen +ivsize, req->giv);
+			req->areq.cryptlen + ivsize, req->giv);
 }
 
 static struct ixp_alg ixp4xx_algos[] = {
@@ -1416,7 +1393,7 @@ static struct ixp_alg ixp4xx_algos[] = {
 static int __init ixp_module_init(void)
 {
 	int num = ARRAY_SIZE(ixp4xx_algos);
-	int i,err ;
+	int i, err ;
 
 	if (platform_device_register(&pseudo_dev))
 		return -ENODEV;
@@ -1429,18 +1406,14 @@ static int __init ixp_module_init(void)
 		platform_device_unregister(&pseudo_dev);
 		return err;
 	}
-	for (i=0; i< num; i++) {
+	for (i = 0; i < num; i++) {
 		struct crypto_alg *cra = &ixp4xx_algos[i].crypto;
 
 		if (snprintf(cra->cra_driver_name, CRYPTO_MAX_ALG_NAME,
-			"%s"IXP_POSTFIX, cra->cra_name) >=
-			CRYPTO_MAX_ALG_NAME)
-		{
+			"%s"IXP_POSTFIX, cra->cra_name) >= CRYPTO_MAX_ALG_NAME)
 			continue;
-		}
-		if (!support_aes && (ixp4xx_algos[i].cfg_enc & MOD_AES)) {
+		if (!support_aes && (ixp4xx_algos[i].cfg_enc & MOD_AES))
 			continue;
-		}
 		if (!ixp4xx_algos[i].hash) {
 			/* block ciphers */
 			cra->cra_type = &crypto_ablkcipher_type;
@@ -1484,7 +1457,7 @@ static void __exit ixp_module_exit(void)
 	int num = ARRAY_SIZE(ixp4xx_algos);
 	int i;
 
-	for (i=0; i< num; i++) {
+	for (i = 0; i < num; i++) {
 		if (ixp4xx_algos[i].registered)
 			crypto_unregister_alg(&ixp4xx_algos[i].crypto);
 	}
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* ixp4xx_crypto: Fix possible NULL ptr dereference.
  2010-01-10 15:34 [6 PATCHes] IXP4xx crypto driver fixes Krzysztof Halasa
                   ` (3 preceding siblings ...)
  2010-01-10 17:33 ` IXP4xx: Fix whitespace problems " Krzysztof Halasa
@ 2010-01-10 17:37 ` Krzysztof Halasa
  2010-01-11 10:07   ` Christian Hohnstaedt
  2010-01-10 17:37 ` ixp4xx_crypto: simplyfy the code a bit Krzysztof Halasa
  5 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-10 17:37 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Christian Hohnstaedt, linux-crypto

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index f8f6515..2ae7148 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -786,10 +786,8 @@ static struct buffer_desc *chainup_buffers(struct device *dev,
 		nbytes -= len;
 		ptr = page_address(sg_page(sg)) + sg->offset;
 		next_buf = dma_pool_alloc(buffer_pool, flags, &next_buf_phys);
-		if (!next_buf) {
-			buf = NULL;
-			break;
-		}
+		if (!next_buf)
+			return NULL;
 		sg_dma_address(sg) = dma_map_single(dev, ptr, len, dir);
 		buf->next = next_buf;
 		buf->phys_next = next_buf_phys;
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* ixp4xx_crypto: simplyfy the code a bit.
  2010-01-10 15:34 [6 PATCHes] IXP4xx crypto driver fixes Krzysztof Halasa
                   ` (4 preceding siblings ...)
  2010-01-10 17:37 ` ixp4xx_crypto: Fix possible NULL ptr dereference Krzysztof Halasa
@ 2010-01-10 17:37 ` Krzysztof Halasa
  5 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-10 17:37 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Christian Hohnstaedt, linux-crypto

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index 2ae7148..a28df93 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -104,7 +104,7 @@ struct buffer_desc {
 	u16 buf_len;
 #endif
 	u32 phys_addr;
-	u32 __reserved[4];
+	u32 __reserved[5];
 	struct buffer_desc *next;
 	enum dma_data_direction dir;
 };
@@ -120,7 +120,7 @@ struct crypt_ctl {
 	u8 mode;		/* NPE_OP_*  operation mode */
 #endif
 	u8 iv[MAX_IVLEN];	/* IV for CBC mode or CTR IV for CTR mode */
-	u32 icv_rev_aes;	/* icv or rev aes */
+	u32 icv_rev_aes;	/* address to store icv or rev aes */
 	u32 src_buf;
 	u32 dst_buf;
 #ifdef __ARMEB__
@@ -429,8 +429,8 @@ static int init_ixp_crypto(void)
 	int ret = -ENODEV;
 	u32 msg[2] = { 0, 0 };
 
-	if (! ( ~(*IXP4XX_EXP_CFG2) & (IXP4XX_FEATURE_HASH |
-				IXP4XX_FEATURE_AES | IXP4XX_FEATURE_DES))) {
+	if (!(ixp4xx_read_feature_bits() &
+	      (IXP4XX_FEATURE_HASH | IXP4XX_FEATURE_AES | IXP4XX_FEATURE_DES))) {
 		printk(KERN_ERR "ixp_crypto: No HW crypto available\n");
 		return ret;
 	}
@@ -442,15 +442,11 @@ static int init_ixp_crypto(void)
 		ret = npe_load_firmware(npe_c, npe_name(npe_c), dev);
 		if (ret)
 			return ret;
-		if (npe_recv_message(npe_c, msg, "STATUS_MSG"))
-			goto npe_error;
-	} else {
-		if (npe_send_message(npe_c, msg, "STATUS_MSG"))
-			goto npe_error;
+	} else if (npe_send_message(npe_c, msg, "STATUS_MSG"))
+		goto npe_error;
 
-		if (npe_recv_message(npe_c, msg, "STATUS_MSG"))
-			goto npe_error;
-	}
+	if (npe_recv_message(npe_c, msg, "STATUS_MSG"))
+		goto npe_error;
 
 	switch ((msg[1]>>16) & 0xff) {
 	case 3:
@@ -1120,7 +1116,7 @@ static int aead_setauthsize(struct crypto_aead *tfm, unsigned int authsize)
 {
 	int max = crypto_aead_alg(tfm)->maxauthsize >> 2;
 
-	if ((authsize>>2) < 1 || (authsize>>2) > max || (authsize & 3))
+	if ((authsize >> 2) < 1 || (authsize >> 2) > max || (authsize & 3))
 		return -EINVAL;
 	return aead_setup(tfm, authsize);
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [6 PATCHes] IXP4xx crypto driver fixes.
  2010-01-10 17:30 ` Krzysztof Halasa
@ 2010-01-11  9:07   ` Christian Hohnstaedt
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Hohnstaedt @ 2010-01-11  9:07 UTC (permalink / raw)
  To: Krzysztof Halasa; +Cc: Herbert Xu, Christian Hohnstaedt, linux-crypto

On Sun, Jan 10, 2010 at 06:30:53PM +0100, Krzysztof Halasa wrote:
> IXP4xx: Fix ixp4xx_crypto little-endian operation.
> 
> Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>
Acked-by: Christian Hohnstaedt <chohnstaedt@innominate.com>

> 
> Fixes the following on IXP425 little-endian:
> 
> NPE-C: firmware functionality 0x5, revision 0x2:1
> alg: skcipher: Test 1 failed on encryption for ecb(des)-ixp4xx
> 00000000: 01 23 45 67 89 ab cd e7
> alg: skcipher: Test 1 failed on encryption for ecb(des3_ede)-ixp4xx
> 00000000: 73 6f 6d 65 64 61 74 61
> alg: skcipher: Test 1 failed on encryption for ecb(aes)-ixp4xx
> 00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff
> 
> diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
> index 6c6656d..cac026a 100644
> --- a/drivers/crypto/ixp4xx_crypto.c
> +++ b/drivers/crypto/ixp4xx_crypto.c
> @@ -96,8 +96,13 @@
>  
>  struct buffer_desc {
>  	u32 phys_next;
> +#ifdef __ARMEB__
>  	u16 buf_len;
>  	u16 pkt_len;
> +#else
> +	u16 pkt_len;
> +	u16 buf_len;
> +#endif
>  	u32 phys_addr;
>  	u32 __reserved[4];
>  	struct buffer_desc *next;
> @@ -105,17 +110,30 @@ struct buffer_desc {
>  };
>  
>  struct crypt_ctl {
> +#ifdef __ARMEB__
>  	u8 mode;		/* NPE_OP_*  operation mode */
>  	u8 init_len;
>  	u16 reserved;
> +#else
> +	u16 reserved;
> +	u8 init_len;
> +	u8 mode;		/* NPE_OP_*  operation mode */
> +#endif
>  	u8 iv[MAX_IVLEN];	/* IV for CBC mode or CTR IV for CTR mode */
>  	u32 icv_rev_aes;	/* icv or rev aes */
>  	u32 src_buf;
>  	u32 dst_buf;
> +#ifdef __ARMEB__
>  	u16 auth_offs;		/* Authentication start offset */
>  	u16 auth_len;		/* Authentication data length */
>  	u16 crypt_offs;		/* Cryption start offset */
>  	u16 crypt_len;		/* Cryption data length */
> +#else
> +	u16 auth_len;		/* Authentication data length */
> +	u16 auth_offs;		/* Authentication start offset */
> +	u16 crypt_len;		/* Cryption data length */
> +	u16 crypt_offs;		/* Cryption start offset */
> +#endif
>  	u32 aadAddr;		/* Additional Auth Data Addr for CCM mode */
>  	u32 crypto_ctx;		/* NPE Crypto Param structure address */
>  
> @@ -651,6 +669,9 @@ static int setup_auth(struct crypto_tfm *tfm, int encrypt, unsigned authsize,
>  
>  	/* write cfg word to cryptinfo */
>  	cfgword = algo->cfgword | ( authsize << 6); /* (authsize/4) << 8 */
> +#ifndef __ARMEB__
> +	cfgword ^= 0xAA000000; /* change the "byte swap" flags */
> +#endif
>  	*(u32*)cinfo = cpu_to_be32(cfgword);
>  	cinfo += sizeof(cfgword);
>  
Christian Hohnstaedt

-- 
Christian Hohnstaedt / Project Manager Hardware and Manufacturing

Innominate Security Technologies AG / protecting industrial networks
tel: +49.30.921028.208 / fax: +49.30.921028.020
Rudower Chaussee 13, D-12489 Berlin / http://www.innominate.com

Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Volker Bibelhausen
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: ixp4xx_crypto: Fix possible NULL ptr dereference.
  2010-01-10 17:37 ` ixp4xx_crypto: Fix possible NULL ptr dereference Krzysztof Halasa
@ 2010-01-11 10:07   ` Christian Hohnstaedt
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Hohnstaedt @ 2010-01-11 10:07 UTC (permalink / raw)
  To: Krzysztof Halasa; +Cc: Herbert Xu, Christian Hohnstaedt, linux-crypto

On Sun, Jan 10, 2010 at 06:37:25PM +0100, Krzysztof Halasa wrote:
> Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>
> 
> diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
> index f8f6515..2ae7148 100644
> --- a/drivers/crypto/ixp4xx_crypto.c
> +++ b/drivers/crypto/ixp4xx_crypto.c
> @@ -786,10 +786,8 @@ static struct buffer_desc *chainup_buffers(struct device *dev,
>  		nbytes -= len;
>  		ptr = page_address(sg_page(sg)) + sg->offset;
>  		next_buf = dma_pool_alloc(buffer_pool, flags, &next_buf_phys);
> -		if (!next_buf) {
> -			buf = NULL;
> -			break;
> -		}
> +		if (!next_buf)
> +			return NULL;

This leaves buf->next uninitialized, but
free_buf_chain() iterates over buf->next.

We need:

	if (!next_buf) {
		buf->next = NULL;
		return NULL;
	}

Or get rid of next_buf and next_buf_phys:

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index b8cc714..c961b0f 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -794,21 +794,15 @@ static struct buffer_desc *chainup_buffers(struct device *dev,
 {
 	for (;nbytes > 0; sg = scatterwalk_sg_next(sg)) {
 		unsigned len = min(nbytes, sg->length);
-		struct buffer_desc *next_buf;
-		u32 next_buf_phys;
 		void *ptr;
 
 		nbytes -= len;
 		ptr = page_address(sg_page(sg)) + sg->offset;
-		next_buf = dma_pool_alloc(buffer_pool, flags, &next_buf_phys);
-		if (!next_buf) {
-			buf = NULL;
-			break;
-		}
+		buf->next = dma_pool_alloc(buffer_pool, flags, &buf->phys_next);
+		if (!buf->next)
+			return NULL;
 		sg_dma_address(sg) = dma_map_single(dev, ptr, len, dir);
-		buf->next = next_buf;
-		buf->phys_next = next_buf_phys;
-		buf = next_buf;
+		buf = buf->next;
 
 		buf->phys_addr = sg_dma_address(sg);
 		buf->buf_len = len;


Christian Hohnstaedt

-- 
Christian Hohnstaedt / Project Manager Hardware and Manufacturing

Innominate Security Technologies AG / protecting industrial networks
tel: +49.30.921028.208 / fax: +49.30.921028.020
Rudower Chaussee 13, D-12489 Berlin / http://www.innominate.com

Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Volker Bibelhausen
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: IXP4xx: Fix whitespace problems in ixp4xx_crypto.
  2010-01-10 17:33 ` IXP4xx: Fix whitespace problems " Krzysztof Halasa
@ 2010-01-12 17:12   ` Christian Hohnstaedt
  2010-01-12 18:09     ` Krzysztof Halasa
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Hohnstaedt @ 2010-01-12 17:12 UTC (permalink / raw)
  To: Krzysztof Halasa; +Cc: Herbert Xu, Christian Hohnstaedt, linux-crypto

On Sun, Jan 10, 2010 at 06:33:37PM +0100, Krzysztof Halasa wrote:
> Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>
> 
> diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
> index 0c7e4f5..f8f6515 100644
> --- a/drivers/crypto/ixp4xx_crypto.c
> +++ b/drivers/crypto/ixp4xx_crypto.c
> @@ -64,7 +64,7 @@
>  
>  #define MOD_DES     0x0000
>  #define MOD_TDEA2   0x0100
> -#define MOD_3DES   0x0200
> +#define MOD_3DES    0x0200
>  #define MOD_AES     0x0800
>  #define MOD_AES128  (0x0800 | KEYLEN_128)
>  #define MOD_AES192  (0x0900 | KEYLEN_192)
> @@ -137,7 +137,7 @@ struct crypt_ctl {
>  	u32 aadAddr;		/* Additional Auth Data Addr for CCM mode */
>  	u32 crypto_ctx;		/* NPE Crypto Param structure address */
>  
> -	/* Used by Host: 4*4 bytes*/
> +	/* Used only by host: 4 * 4 bytes */
>  	unsigned ctl_flags;
>  	union {
>  		struct ablkcipher_request *ablk_req;
> @@ -208,10 +208,10 @@ static const struct ix_hash_algo hash_alg_sha1 = {
>  };
>  
>  static struct npe *npe_c;
> -static struct dma_pool *buffer_pool = NULL;
> -static struct dma_pool *ctx_pool = NULL;
> +static struct dma_pool *buffer_pool;
> +static struct dma_pool *ctx_pool;
>  
> -static struct crypt_ctl *crypt_virt = NULL;
> +static struct crypt_ctl *crypt_virt;

This is not a whitespace-fix.
The error-path in init_ixp_crypto() depends on them being either NULL
or correctly allocated.

Or is it guaranteed that static variables are always initially zero ?

>  static dma_addr_t crypt_phys;
>  
>  static int support_aes = 1;

But this initialization is superflous, since it will be initialized before use.

> @@ -246,12 +246,12 @@ static inline struct crypt_ctl *crypt_phys2virt(dma_addr_t phys)
>  
>  static inline u32 cipher_cfg_enc(struct crypto_tfm *tfm)
>  {
> -	return container_of(tfm->__crt_alg, struct ixp_alg,crypto)->cfg_enc;
> +	return container_of(tfm->__crt_alg, struct ixp_alg, crypto)->cfg_enc;
>  }
>  
>  static inline u32 cipher_cfg_dec(struct crypto_tfm *tfm)
>  {
> -	return container_of(tfm->__crt_alg, struct ixp_alg,crypto)->cfg_dec;
> +	return container_of(tfm->__crt_alg, struct ixp_alg, crypto)->cfg_dec;
>  }
>  
>  static inline const struct ix_hash_algo *ix_hash(struct crypto_tfm *tfm)
> @@ -275,7 +275,7 @@ static spinlock_t desc_lock;
>  static struct crypt_ctl *get_crypt_desc(void)
>  {
>  	int i;
> -	static int idx = 0;
> +	static int idx;

This static index must be initialized with 0.

>  	unsigned long flags;
>  	struct crypt_ctl *desc = NULL;
>  
> @@ -318,13 +318,13 @@ static struct crypt_ctl *get_crypt_desc_emerg(void)
>  		if (++idx >= NPE_QLEN_TOTAL)
>  			idx = NPE_QLEN;
>  		crypt_virt[i].ctl_flags = CTL_FLAG_USED;
> -		desc = crypt_virt +i;
> +		desc = crypt_virt + i;
>  	}
>  	spin_unlock_irqrestore(&emerg_lock, flags);
>  	return desc;
>  }
>  
> -static void free_buf_chain(struct device *dev, struct buffer_desc *buf,u32 phys)
> +static void free_buf_chain(struct device *dev, struct buffer_desc *buf, u32 phys)

Introduces a line-length > 80.

>  {
>  	while (buf) {
>  		struct buffer_desc *buf1;
> @@ -349,10 +349,9 @@ static void finish_scattered_hmac(struct crypt_ctl *crypt)
>  	int authsize = crypto_aead_authsize(tfm);

[ snip ]

> @@ -1416,7 +1393,7 @@ static struct ixp_alg ixp4xx_algos[] = {
>  static int __init ixp_module_init(void)
>  {
>  	int num = ARRAY_SIZE(ixp4xx_algos);
> -	int i,err ;
> +	int i, err ;

Missed one before the ;

>  
>  	if (platform_device_register(&pseudo_dev))
>  		return -ENODEV;
> @@ -1429,18 +1406,14 @@ static int __init ixp_module_init(void)
>  		platform_device_unregister(&pseudo_dev);
>  		return err;
>  	}
> -	for (i=0; i< num; i++) {
> +	for (i = 0; i < num; i++) {
>  		struct crypto_alg *cra = &ixp4xx_algos[i].crypto;
>  
>  		if (snprintf(cra->cra_driver_name, CRYPTO_MAX_ALG_NAME,
> -			"%s"IXP_POSTFIX, cra->cra_name) >=
> -			CRYPTO_MAX_ALG_NAME)
> -		{
> +			"%s"IXP_POSTFIX, cra->cra_name) >= CRYPTO_MAX_ALG_NAME)
>  			continue;
> -		}
> -		if (!support_aes && (ixp4xx_algos[i].cfg_enc & MOD_AES)) {
> +		if (!support_aes && (ixp4xx_algos[i].cfg_enc & MOD_AES))
>  			continue;
> -		}
>  		if (!ixp4xx_algos[i].hash) {
>  			/* block ciphers */
>  			cra->cra_type = &crypto_ablkcipher_type;
> @@ -1484,7 +1457,7 @@ static void __exit ixp_module_exit(void)
>  	int num = ARRAY_SIZE(ixp4xx_algos);
>  	int i;
>  
> -	for (i=0; i< num; i++) {
> +	for (i = 0; i < num; i++) {
>  		if (ixp4xx_algos[i].registered)
>  			crypto_unregister_alg(&ixp4xx_algos[i].crypto);
>  	}

Christian Hohnstaedt

-- 
Christian Hohnstaedt / Project Manager Hardware and Manufacturing

Innominate Security Technologies AG / protecting industrial networks
tel: +49.30.921028.208 / fax: +49.30.921028.020
Rudower Chaussee 13, D-12489 Berlin / http://www.innominate.com

Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Volker Bibelhausen
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IXP4xx: Fix whitespace problems in ixp4xx_crypto.
  2010-01-12 17:12   ` Christian Hohnstaedt
@ 2010-01-12 18:09     ` Krzysztof Halasa
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Halasa @ 2010-01-12 18:09 UTC (permalink / raw)
  To: Christian Hohnstaedt; +Cc: Herbert Xu, linux-crypto

Christian Hohnstaedt <chohnstaedt@innominate.com> writes:

>>  static struct npe *npe_c;
>> -static struct dma_pool *buffer_pool = NULL;
>> -static struct dma_pool *ctx_pool = NULL;
>> +static struct dma_pool *buffer_pool;
>> +static struct dma_pool *ctx_pool;
>>  
>> -static struct crypt_ctl *crypt_virt = NULL;
>> +static struct crypt_ctl *crypt_virt;
>
> This is not a whitespace-fix.

Right, that's trivial non-whitespace fix :-)

> The error-path in init_ixp_crypto() depends on them being either NULL
> or correctly allocated.
>
> Or is it guaranteed that static variables are always initially zero ?

Yes, the BSS is cleared at boot (modprobe etc). This simply makes the
on-disk image a bit smaller.

>>  static dma_addr_t crypt_phys;
>>  
>>  static int support_aes = 1;
>
> But this initialization is superflous, since it will be initialized
> before use.

I didn't touch it, but will remove the initialization if it's unneeded,
of course.

>> -	static int idx = 0;
>> +	static int idx;
>
> This static index must be initialized with 0.

It is, same as the crypt_virt and co.

>> -static void free_buf_chain(struct device *dev, struct buffer_desc *buf,u32 phys)
>> +static void free_buf_chain(struct device *dev, struct buffer_desc *buf, u32 phys)
>
> Introduces a line-length > 80.

This limit has been lifted recently :-)

>>  	int num = ARRAY_SIZE(ixp4xx_algos);
>> -	int i,err ;
>> +	int i, err ;
>
> Missed one before the ;

Right.


I will fix/change these, not today but soon. Thanks for looking.
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-01-12 18:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-10 15:34 [6 PATCHes] IXP4xx crypto driver fixes Krzysztof Halasa
2010-01-10 17:30 ` Krzysztof Halasa
2010-01-11  9:07   ` Christian Hohnstaedt
2010-01-10 17:32 ` IXP4xx: Fix ixp4xx_crypto sparse warnings Krzysztof Halasa
2010-01-10 17:32 ` IXP4xx: Simplify get_crypt_desc() and get_crypt_desc_emerg() in ixp4xx_crypto Krzysztof Halasa
2010-01-10 17:33 ` IXP4xx: Fix whitespace problems " Krzysztof Halasa
2010-01-12 17:12   ` Christian Hohnstaedt
2010-01-12 18:09     ` Krzysztof Halasa
2010-01-10 17:37 ` ixp4xx_crypto: Fix possible NULL ptr dereference Krzysztof Halasa
2010-01-11 10:07   ` Christian Hohnstaedt
2010-01-10 17:37 ` ixp4xx_crypto: simplyfy the code a bit Krzysztof Halasa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.