crypto: omap-sham: Remove useless check for non-null request

PATCH In review Krzysztof Kozlowski 3 participants 7 messages Mar 1, 2018 – Mar 9, 2018

Subsystems (3)

Crypto API Arm Samsung Exynos Samsung S5P Security SubSystem (sss) driver

lore

Share & tools

lore

Krzysztof Kozlowski

author maintainer

2018-03-01 20:50 UTC #0 lore raw

PATCH 1/4 crypto: omap-sham: Remove useless check for non-null request

ahash_request 'req' argument passed by the caller omap_sham_handle_queue() cannot be NULL here because it is obtained from non-NULL pointer via container_of().

This fixes smatch warning:
drivers/crypto/omap-sham.c:812 omap_sham_prepare_request() warn: variable dereferenced before check 'req' (see line 805)

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 drivers/crypto/omap-sham.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/crypto/omap-sham.c +0 -3

index 86b89ace836f..7650b1b449bb 100644
@@ -809,9 +809,6 @@ static int omap_sham_prepare_request(struct ahash_request *req, bool update)
 	bool final = rctx->flags & BIT(FLAGS_FINUP);
 	int xmit_len, hash_later;
 
-	if (!req)
-		return 0;
-
 	bs = get_block_size(rctx);
 
 	if (update)

Signature · 1 line

2.7.4

Krzysztof Kozlowski

author maintainer

2018-03-01 20:50 UTC #1 lore raw

PATCH 2/4 crypto: omap-sham - Fix misleading indentation

Commit 8043bb1ae03c ("crypto: omap-sham - convert driver logic to use sgs for data xmit") removed the if() clause leaving the statement as is. The intention was in that case to finish the request always so the goto instruction seems sensible.

Remove the indentation to fix Smatch warning:
drivers/crypto/omap-sham.c:1761 omap_sham_done_task() warn: inconsistent indenting

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 drivers/crypto/omap-sham.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/crypto/omap-sham.c +1 -1

index 7650b1b449bb..6cb6ab6f52c0 100644
@@ -1758,7 +1758,7 @@ static void omap_sham_done_task(unsigned long data)
 		if (test_and_clear_bit(FLAGS_OUTPUT_READY, &dd->flags)) {
 			/* hash or semi-hash ready */
 			clear_bit(FLAGS_DMA_READY, &dd->flags);
-				goto finish;
+			goto finish;
 		}
 	}

Signature · 1 line

2.7.4

Tero Kristo

Acked-by on v1 2/4

2018-03-05 08:59 UTC #2 lore raw

Re: [PATCH 2/4] crypto: omap-sham - Fix misleading indentation

Quoted 9 lines · Krzysztof Kozlowski

Commit 8043bb1ae03c ("crypto: omap-sham - convert driver logic to use sgs for data xmit") removed the if() clause leaving the statement as is. The intention was in that case to finish the request always so the goto instruction seems sensible.
Remove the indentation to fix Smatch warning:
drivers/crypto/omap-sham.c:1761 omap_sham_done_task() warn: inconsistent indenting
Signed-off-by Krzysztof Kozlowski <[email protected]>

Acked-by Tero Kristo <[email protected]>

Quoted 10 lines

---
  drivers/crypto/omap-sham.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/crypto/omap-sham.c +0 -0

index 7650b1b449bb..6cb6ab6f52c0 100644

drivers/crypto/omap-sham.c static void omap_sham_done_task(unsigned long data) · line 1764

Quoted 8 lines

drivers/crypto/omap-sham.c +1 -1

@@ -1758,7 +1758,7 @@ static void omap_sham_done_task(unsigned long data)
  		if (test_and_clear_bit(FLAGS_OUTPUT_READY, &dd->flags)) {
  			/* hash or semi-hash ready */
  			clear_bit(FLAGS_DMA_READY, &dd->flags);
-				goto finish;
+			goto finish;
  		}
  	}

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Krzysztof Kozlowski

author maintainer

2018-03-01 20:50 UTC #3 lore raw

PATCH 3/4 crypto: s5p-sss: Remove useless check for non-null request

In reply to Krzysztof Kozlowski

ahash_request 'req' argument passed by the caller s5p_hash_handle_queue() cannot be NULL here because it is obtained from non-NULL pointer via container_of().

This fixes smatch warning:
drivers/crypto/s5p-sss.c:1213 s5p_hash_prepare_request() warn: variable dereferenced before check 'req' (see line 1208)

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 drivers/crypto/s5p-sss.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/crypto/s5p-sss.c +0 -3

index 5d64c08b7f47..d7c8163e5068 100644
@@ -1210,9 +1210,6 @@ static int s5p_hash_prepare_request(struct ahash_request *req, bool update)
 	int xmit_len, hash_later, nbytes;
 	int ret;
 
-	if (!req)
-		return 0;
-
 	if (update)
 		nbytes = req->nbytes;
 	else

Signature · 1 line

2.7.4

Krzysztof Kozlowski

author maintainer

2018-03-01 20:50 UTC #4 lore raw

PATCH 4/4 crypto: s5p-sss - Constify pointed data (arguments and local variables)

In reply to Krzysztof Kozlowski

Improve the code (safety and readability) by indicating that data passed through pointer is not modified. This adds const keyword in many places, most notably:
- the driver data (pointer to struct samsung_aes_variant),
- scatterlist addresses written as value to device registers,
- key and IV arrays.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 drivers/crypto/s5p-sss.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

drivers/crypto/s5p-sss.c +17 ~~-14~~

index d7c8163e5068..bf7163042569 100644
@@ -404,29 +404,31 @@ static const struct of_device_id s5p_sss_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
 
-static inline struct samsung_aes_variant *find_s5p_sss_version
-				   (struct platform_device *pdev)
+static inline const struct samsung_aes_variant *find_s5p_sss_version
+				   (const struct platform_device *pdev)
 {
 	if (IS_ENABLED(CONFIG_OF) && (pdev->dev.of_node)) {
 		const struct of_device_id *match;
 
 		match = of_match_node(s5p_sss_dt_match,
 					pdev->dev.of_node);
-		return (struct samsung_aes_variant *)match->data;
+		return (const struct samsung_aes_variant *)match->data;
 	}
-	return (struct samsung_aes_variant *)
+	return (const struct samsung_aes_variant *)
 			platform_get_device_id(pdev)->driver_data;
 }
 
 static struct s5p_aes_dev *s5p_dev;
 
-static void s5p_set_dma_indata(struct s5p_aes_dev *dev, struct scatterlist *sg)
+static void s5p_set_dma_indata(struct s5p_aes_dev *dev,
+			       const struct scatterlist *sg)
 {
 	SSS_WRITE(dev, FCBRDMAS, sg_dma_address(sg));
 	SSS_WRITE(dev, FCBRDMAL, sg_dma_len(sg));
 }
 
-static void s5p_set_dma_outdata(struct s5p_aes_dev *dev, struct scatterlist *sg)
+static void s5p_set_dma_outdata(struct s5p_aes_dev *dev,
+				const struct scatterlist *sg)
 {
 	SSS_WRITE(dev, FCBTDMAS, sg_dma_address(sg));
 	SSS_WRITE(dev, FCBTDMAL, sg_dma_len(sg));

@@ -619,7 +621,7 @@ static inline void s5p_hash_write(struct s5p_aes_dev *dd,
  * @sg:		scatterlist ready to DMA transmit
  */
 static void s5p_set_dma_hashdata(struct s5p_aes_dev *dev,
-				 struct scatterlist *sg)
+				 const struct scatterlist *sg)
 {
 	dev->hash_sg_cnt--;
 	SSS_WRITE(dev, FCHRDMAS, sg_dma_address(sg));

@@ -792,9 +794,9 @@ static void s5p_hash_read_msg(struct ahash_request *req)
  * @ctx:	request context
  */
 static void s5p_hash_write_ctx_iv(struct s5p_aes_dev *dd,
-				  struct s5p_hash_reqctx *ctx)
+				  const struct s5p_hash_reqctx *ctx)
 {
-	u32 *hash = (u32 *)ctx->digest;
+	const u32 *hash = (const u32 *)ctx->digest;
 	unsigned int i;
 
 	for (i = 0; i < ctx->nregs; i++)

@@ -818,7 +820,7 @@ static void s5p_hash_write_iv(struct ahash_request *req)
  */
 static void s5p_hash_copy_result(struct ahash_request *req)
 {
-	struct s5p_hash_reqctx *ctx = ahash_request_ctx(req);
+	const struct s5p_hash_reqctx *ctx = ahash_request_ctx(req);
 
 	if (!req->result)
 		return;

@@ -1290,7 +1292,7 @@ static int s5p_hash_prepare_request(struct ahash_request *req, bool update)
  */
 static void s5p_hash_update_dma_stop(struct s5p_aes_dev *dd)
 {
-	struct s5p_hash_reqctx *ctx = ahash_request_ctx(dd->hash_req);
+	const struct s5p_hash_reqctx *ctx = ahash_request_ctx(dd->hash_req);
 
 	dma_unmap_sg(dd->dev, ctx->sg, ctx->sg_len, DMA_TO_DEVICE);
 	clear_bit(HASH_FLAGS_DMA_ACTIVE, &dd->hash_flags);

@@ -1717,7 +1719,7 @@ static void s5p_hash_cra_exit(struct crypto_tfm *tfm)
  */
 static int s5p_hash_export(struct ahash_request *req, void *out)
 {
-	struct s5p_hash_reqctx *ctx = ahash_request_ctx(req);
+	const struct s5p_hash_reqctx *ctx = ahash_request_ctx(req);
 
 	memcpy(out, ctx, sizeof(*ctx) + ctx->bufcnt);

@@ -1831,7 +1833,8 @@ static struct ahash_alg algs_sha1_md5_sha256[] = {
 };
 
 static void s5p_set_aes(struct s5p_aes_dev *dev,
-			uint8_t *key, uint8_t *iv, unsigned int keylen)
+			const uint8_t *key, const uint8_t *iv,
+			unsigned int keylen)
 {
 	void __iomem *keystart;

@@ -2150,7 +2153,7 @@ static int s5p_aes_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	int i, j, err = -ENODEV;
-	struct samsung_aes_variant *variant;
+	const struct samsung_aes_variant *variant;
 	struct s5p_aes_dev *pdata;
 	struct resource *res;
 	unsigned int hash_i;

Signature · 1 line

2.7.4

Tero Kristo

Acked-by on v1 1/4

2018-03-05 08:58 UTC #5 lore raw

In reply to Krzysztof Kozlowski

Quoted 8 lines · Krzysztof Kozlowski

ahash_request 'req' argument passed by the caller omap_sham_handle_queue() cannot be NULL here because it is obtained from non-NULL pointer via container_of().
This fixes smatch warning:
drivers/crypto/omap-sham.c:812 omap_sham_prepare_request() warn: variable dereferenced before check 'req' (see line 805)
Signed-off-by Krzysztof Kozlowski <[email protected]>

Acked-by Tero Kristo <[email protected]>

Quoted 10 lines

---
  drivers/crypto/omap-sham.c | 3 ---
  1 file changed, 3 deletions(-)

drivers/crypto/omap-sham.c +0 -0

index 86b89ace836f..7650b1b449bb 100644

drivers/crypto/omap-sham.c static int omap_sham_prepare_request(struct ahash_request *req, bool update) · line 814

Quoted 9 lines

drivers/crypto/omap-sham.c +0 -3

@@ -809,9 +809,6 @@ static int omap_sham_prepare_request(struct ahash_request *req, bool update)
  	bool final = rctx->flags & BIT(FLAGS_FINUP);
  	int xmit_len, hash_later;
 
-	if (!req)
-		return 0;
-
  	bs = get_block_size(rctx);
 
  	if (update)

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Herbert Xu

maintainer

2018-03-09 15:19 UTC #6 lore raw

In reply to Krzysztof Kozlowski

Quoted 8 lines · Krzysztof Kozlowski

ahash_request 'req' argument passed by the caller omap_sham_handle_queue() cannot be NULL here because it is obtained from non-NULL pointer via container_of().
This fixes smatch warning:
drivers/crypto/omap-sham.c:812 omap_sham_prepare_request() warn: variable dereferenced before check 'req' (see line 805)
Signed-off-by Krzysztof Kozlowski <[email protected]>

All applied. Thanks.

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt