diff mbox series

[RESEND,v2] cxgbi: move cxgbi_ddp_set_one_ppod to cxgb_ppm and remove its duplicate

Message ID 20221226090609.1917788-1-d-tatianin@yandex-team.ru (mailing list archive)
State New, archived
Headers show
Series [RESEND,v2] cxgbi: move cxgbi_ddp_set_one_ppod to cxgb_ppm and remove its duplicate | expand

Commit Message

Daniil Tatianin Dec. 26, 2022, 9:06 a.m. UTC 
cxgbit and libcxgbi both used the exact same function but with slightly
different names, and a missing NULL check in one case. Move the function
to libcxgb/libcxgb_ppm.c and nuke the duplicate.

This also renames the function to cxgbi_ppm_set_one_ppod so that it
matches the rest of the functions in cxgb_ppm.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Acked-by: Varun Prakash <varun@chelsio.com>
---
 .../ethernet/chelsio/libcxgb/libcxgb_ppm.c    | 56 ++++++++++++++++++
 .../ethernet/chelsio/libcxgb/libcxgb_ppm.h    |  5 ++
 drivers/scsi/cxgbi/cxgb3i/cxgb3i.c            |  2 +-
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c            |  2 +-
 drivers/scsi/cxgbi/libcxgbi.c                 | 55 ------------------
 drivers/scsi/cxgbi/libcxgbi.h                 |  3 -
 drivers/target/iscsi/cxgbit/cxgbit_ddp.c      | 57 +------------------
 7 files changed, 64 insertions(+), 116 deletions(-)

Comments

Dan Carpenter Dec. 29, 2022, 1:53 p.m. UTC | #1 
Hi Daniil,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniil-Tatianin/cxgbi-move-cxgbi_ddp_set_one_ppod-to-cxgb_ppm-and-remove-its-duplicate/20221226-171028
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20221226090609.1917788-1-d-tatianin%40yandex-team.ru
patch subject: [RESEND PATCH v2] cxgbi: move cxgbi_ddp_set_one_ppod to cxgb_ppm and remove its duplicate
config: powerpc-randconfig-m031-20221226
compiler: powerpc-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

smatch warnings:
drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:571 cxgbi_ppm_set_one_ppod() error: we previously assumed 'sg_off' could be null (see line 536)

vim +/sg_off +571 drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c

e775e542c914b4 Daniil Tatianin 2022-12-26  530  void
e775e542c914b4 Daniil Tatianin 2022-12-26  531  cxgbi_ppm_set_one_ppod(struct cxgbi_pagepod *ppod,
e775e542c914b4 Daniil Tatianin 2022-12-26  532  		       struct cxgbi_task_tag_info *ttinfo,
e775e542c914b4 Daniil Tatianin 2022-12-26  533  		       struct scatterlist **sg_pp, unsigned int *sg_off)
e775e542c914b4 Daniil Tatianin 2022-12-26  534  {
e775e542c914b4 Daniil Tatianin 2022-12-26  535  	struct scatterlist *sg = sg_pp ? *sg_pp : NULL;
e775e542c914b4 Daniil Tatianin 2022-12-26 @536  	unsigned int offset = sg_off ? *sg_off : 0;
                                                                              ^^^^^^
Check for NULL

e775e542c914b4 Daniil Tatianin 2022-12-26  537  	dma_addr_t addr = 0UL;
e775e542c914b4 Daniil Tatianin 2022-12-26  538  	unsigned int len = 0;
e775e542c914b4 Daniil Tatianin 2022-12-26  539  	int i;
e775e542c914b4 Daniil Tatianin 2022-12-26  540  
e775e542c914b4 Daniil Tatianin 2022-12-26  541  	memcpy(ppod, &ttinfo->hdr, sizeof(struct cxgbi_pagepod_hdr));
e775e542c914b4 Daniil Tatianin 2022-12-26  542  
e775e542c914b4 Daniil Tatianin 2022-12-26  543  	if (sg) {
e775e542c914b4 Daniil Tatianin 2022-12-26  544  		addr = sg_dma_address(sg);
e775e542c914b4 Daniil Tatianin 2022-12-26  545  		len = sg_dma_len(sg);
e775e542c914b4 Daniil Tatianin 2022-12-26  546  	}
e775e542c914b4 Daniil Tatianin 2022-12-26  547  
e775e542c914b4 Daniil Tatianin 2022-12-26  548  	for (i = 0; i < PPOD_PAGES_MAX; i++) {
e775e542c914b4 Daniil Tatianin 2022-12-26  549  		if (sg) {
e775e542c914b4 Daniil Tatianin 2022-12-26  550  			ppod->addr[i] = cpu_to_be64(addr + offset);
e775e542c914b4 Daniil Tatianin 2022-12-26  551  			offset += PAGE_SIZE;
e775e542c914b4 Daniil Tatianin 2022-12-26  552  			if (offset == (len + sg->offset)) {
e775e542c914b4 Daniil Tatianin 2022-12-26  553  				offset = 0;
e775e542c914b4 Daniil Tatianin 2022-12-26  554  				sg = sg_next(sg);
e775e542c914b4 Daniil Tatianin 2022-12-26  555  				if (sg) {
e775e542c914b4 Daniil Tatianin 2022-12-26  556  					addr = sg_dma_address(sg);
e775e542c914b4 Daniil Tatianin 2022-12-26  557  					len = sg_dma_len(sg);
e775e542c914b4 Daniil Tatianin 2022-12-26  558  				}
e775e542c914b4 Daniil Tatianin 2022-12-26  559  			}
e775e542c914b4 Daniil Tatianin 2022-12-26  560  		} else {
e775e542c914b4 Daniil Tatianin 2022-12-26  561  			ppod->addr[i] = 0ULL;
e775e542c914b4 Daniil Tatianin 2022-12-26  562  		}
e775e542c914b4 Daniil Tatianin 2022-12-26  563  	}
e775e542c914b4 Daniil Tatianin 2022-12-26  564  
e775e542c914b4 Daniil Tatianin 2022-12-26  565  	/*
e775e542c914b4 Daniil Tatianin 2022-12-26  566  	 * the fifth address needs to be repeated in the next ppod, so do
e775e542c914b4 Daniil Tatianin 2022-12-26  567  	 * not move sg
e775e542c914b4 Daniil Tatianin 2022-12-26  568  	 */
e775e542c914b4 Daniil Tatianin 2022-12-26  569  	if (sg_pp) {
e775e542c914b4 Daniil Tatianin 2022-12-26  570  		*sg_pp = sg;
e775e542c914b4 Daniil Tatianin 2022-12-26 @571  		*sg_off = offset;
                                                                ^^^^^^^
Unchecked dereference.

e775e542c914b4 Daniil Tatianin 2022-12-26  572  	}
e775e542c914b4 Daniil Tatianin 2022-12-26  573  
e775e542c914b4 Daniil Tatianin 2022-12-26  574  	if (offset == len) {
e775e542c914b4 Daniil Tatianin 2022-12-26  575  		offset = 0;
e775e542c914b4 Daniil Tatianin 2022-12-26  576  		if (sg) {
e775e542c914b4 Daniil Tatianin 2022-12-26  577  			sg = sg_next(sg);
e775e542c914b4 Daniil Tatianin 2022-12-26  578  			if (sg)
e775e542c914b4 Daniil Tatianin 2022-12-26  579  				addr = sg_dma_address(sg);
e775e542c914b4 Daniil Tatianin 2022-12-26  580  		}
e775e542c914b4 Daniil Tatianin 2022-12-26  581  	}
e775e542c914b4 Daniil Tatianin 2022-12-26  582  	ppod->addr[i] = sg ? cpu_to_be64(addr + offset) : 0ULL;
e775e542c914b4 Daniil Tatianin 2022-12-26  583  }
diff mbox series

Patch

diff --git a/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c b/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c
index 854d87e1125c..9103826b0d27 100644
--- a/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c
+++ b/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c
@@ -527,6 +527,62 @@  unsigned int cxgbi_tagmask_set(unsigned int ppmax)
 }
 EXPORT_SYMBOL(cxgbi_tagmask_set);
 
+void
+cxgbi_ppm_set_one_ppod(struct cxgbi_pagepod *ppod,
+		       struct cxgbi_task_tag_info *ttinfo,
+		       struct scatterlist **sg_pp, unsigned int *sg_off)
+{
+	struct scatterlist *sg = sg_pp ? *sg_pp : NULL;
+	unsigned int offset = sg_off ? *sg_off : 0;
+	dma_addr_t addr = 0UL;
+	unsigned int len = 0;
+	int i;
+
+	memcpy(ppod, &ttinfo->hdr, sizeof(struct cxgbi_pagepod_hdr));
+
+	if (sg) {
+		addr = sg_dma_address(sg);
+		len = sg_dma_len(sg);
+	}
+
+	for (i = 0; i < PPOD_PAGES_MAX; i++) {
+		if (sg) {
+			ppod->addr[i] = cpu_to_be64(addr + offset);
+			offset += PAGE_SIZE;
+			if (offset == (len + sg->offset)) {
+				offset = 0;
+				sg = sg_next(sg);
+				if (sg) {
+					addr = sg_dma_address(sg);
+					len = sg_dma_len(sg);
+				}
+			}
+		} else {
+			ppod->addr[i] = 0ULL;
+		}
+	}
+
+	/*
+	 * the fifth address needs to be repeated in the next ppod, so do
+	 * not move sg
+	 */
+	if (sg_pp) {
+		*sg_pp = sg;
+		*sg_off = offset;
+	}
+
+	if (offset == len) {
+		offset = 0;
+		if (sg) {
+			sg = sg_next(sg);
+			if (sg)
+				addr = sg_dma_address(sg);
+		}
+	}
+	ppod->addr[i] = sg ? cpu_to_be64(addr + offset) : 0ULL;
+}
+EXPORT_SYMBOL(cxgbi_ppm_set_one_ppod);
+
 MODULE_AUTHOR("Chelsio Communications");
 MODULE_DESCRIPTION("Chelsio common library");
 MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.h b/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.h
index 1b4156461ba1..f2178ee0b18a 100644
--- a/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.h
+++ b/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.h
@@ -332,4 +332,9 @@  int cxgbi_ppm_release(struct cxgbi_ppm *ppm);
 void cxgbi_tagmask_check(unsigned int tagmask, struct cxgbi_tag_format *);
 unsigned int cxgbi_tagmask_set(unsigned int ppmax);
 
+void
+cxgbi_ppm_set_one_ppod(struct cxgbi_pagepod *ppod,
+		       struct cxgbi_task_tag_info *ttinfo,
+		       struct scatterlist **sg_pp, unsigned int *sg_off);
+
 #endif	/*__LIBCXGB_PPM_H__*/
diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
index ff9d4287937a..0399e82362b7 100644
--- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
+++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
@@ -1115,7 +1115,7 @@  static int ddp_set_map(struct cxgbi_ppm *ppm, struct cxgbi_sock *csk,
 		req = (struct ulp_mem_io *)skb->head;
 		ppod = (struct cxgbi_pagepod *)(req + 1);
 		sg_off = i * PPOD_PAGES_MAX;
-		cxgbi_ddp_set_one_ppod(ppod, ttinfo, &sg,
+		cxgbi_ppm_set_one_ppod(ppod, ttinfo, &sg,
 				       &sg_off);
 		skb->priority = CPL_PRIORITY_CONTROL;
 		cxgb3_ofld_send(ppm->lldev, skb);
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index c07d2e3b4bcf..1f768cc3fbfb 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -2035,7 +2035,7 @@  static int ddp_ppod_write_idata(struct cxgbi_ppm *ppm, struct cxgbi_sock *csk,
 	ppod = (struct cxgbi_pagepod *)(idata + 1);
 
 	for (i = 0; i < npods; i++, ppod++)
-		cxgbi_ddp_set_one_ppod(ppod, ttinfo, sg_pp, sg_off);
+		cxgbi_ppm_set_one_ppod(ppod, ttinfo, sg_pp, sg_off);
 
 	cxgbi_skcb_set_flag(skb, SKCBF_TX_MEM_WRITE);
 	cxgbi_skcb_set_flag(skb, SKCBF_TX_FLAG_COMPL);
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index af281e271f88..6a2627a73f26 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -1151,61 +1151,6 @@  scmd_get_params(struct scsi_cmnd *sc, struct scatterlist **sgl,
 	/* Caution: for protection sdb, sdb->length is invalid */
 }
 
-void cxgbi_ddp_set_one_ppod(struct cxgbi_pagepod *ppod,
-			    struct cxgbi_task_tag_info *ttinfo,
-			    struct scatterlist **sg_pp, unsigned int *sg_off)
-{
-	struct scatterlist *sg = sg_pp ? *sg_pp : NULL;
-	unsigned int offset = sg_off ? *sg_off : 0;
-	dma_addr_t addr = 0UL;
-	unsigned int len = 0;
-	int i;
-
-	memcpy(ppod, &ttinfo->hdr, sizeof(struct cxgbi_pagepod_hdr));
-
-	if (sg) {
-		addr = sg_dma_address(sg);
-		len = sg_dma_len(sg);
-	}
-
-	for (i = 0; i < PPOD_PAGES_MAX; i++) {
-		if (sg) {
-			ppod->addr[i] = cpu_to_be64(addr + offset);
-			offset += PAGE_SIZE;
-			if (offset == (len + sg->offset)) {
-				offset = 0;
-				sg = sg_next(sg);
-				if (sg) {
-					addr = sg_dma_address(sg);
-					len = sg_dma_len(sg);
-				}
-			}
-		} else {
-			ppod->addr[i] = 0ULL;
-		}
-	}
-
-	/*
-	 * the fifth address needs to be repeated in the next ppod, so do
-	 * not move sg
-	 */
-	if (sg_pp) {
-		*sg_pp = sg;
-		*sg_off = offset;
-	}
-
-	if (offset == len) {
-		offset = 0;
-		sg = sg_next(sg);
-		if (sg) {
-			addr = sg_dma_address(sg);
-			len = sg_dma_len(sg);
-		}
-	}
-	ppod->addr[i] = sg ? cpu_to_be64(addr + offset) : 0ULL;
-}
-EXPORT_SYMBOL_GPL(cxgbi_ddp_set_one_ppod);
-
 /*
  * APIs interacting with open-iscsi libraries
  */
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 3687b5c0cf90..f90e747bcb4f 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -636,9 +636,6 @@  int cxgbi_ddp_init(struct cxgbi_device *, unsigned int, unsigned int,
 			unsigned int, unsigned int);
 int cxgbi_ddp_cleanup(struct cxgbi_device *);
 void cxgbi_ddp_page_size_factor(int *);
-void cxgbi_ddp_set_one_ppod(struct cxgbi_pagepod *,
-			    struct cxgbi_task_tag_info *,
-			    struct scatterlist **sg_pp, unsigned int *sg_off);
 int cxgbi_ddp_ppm_setup(void **ppm_pp, struct cxgbi_device *cdev,
 			struct cxgbi_tag_format *tformat,
 			unsigned int iscsi_size, unsigned int llimit,
diff --git a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
index 17fd0d8cc490..fe29f4962058 100644
--- a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
+++ b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
@@ -5,61 +5,6 @@ 
 
 #include "cxgbit.h"
 
-static void
-cxgbit_set_one_ppod(struct cxgbi_pagepod *ppod,
-		    struct cxgbi_task_tag_info *ttinfo,
-		    struct scatterlist **sg_pp, unsigned int *sg_off)
-{
-	struct scatterlist *sg = sg_pp ? *sg_pp : NULL;
-	unsigned int offset = sg_off ? *sg_off : 0;
-	dma_addr_t addr = 0UL;
-	unsigned int len = 0;
-	int i;
-
-	memcpy(ppod, &ttinfo->hdr, sizeof(struct cxgbi_pagepod_hdr));
-
-	if (sg) {
-		addr = sg_dma_address(sg);
-		len = sg_dma_len(sg);
-	}
-
-	for (i = 0; i < PPOD_PAGES_MAX; i++) {
-		if (sg) {
-			ppod->addr[i] = cpu_to_be64(addr + offset);
-			offset += PAGE_SIZE;
-			if (offset == (len + sg->offset)) {
-				offset = 0;
-				sg = sg_next(sg);
-				if (sg) {
-					addr = sg_dma_address(sg);
-					len = sg_dma_len(sg);
-				}
-			}
-		} else {
-			ppod->addr[i] = 0ULL;
-		}
-	}
-
-	/*
-	 * the fifth address needs to be repeated in the next ppod, so do
-	 * not move sg
-	 */
-	if (sg_pp) {
-		*sg_pp = sg;
-		*sg_off = offset;
-	}
-
-	if (offset == len) {
-		offset = 0;
-		if (sg) {
-			sg = sg_next(sg);
-			if (sg)
-				addr = sg_dma_address(sg);
-		}
-	}
-	ppod->addr[i] = sg ? cpu_to_be64(addr + offset) : 0ULL;
-}
-
 static struct sk_buff *
 cxgbit_ppod_init_idata(struct cxgbit_device *cdev, struct cxgbi_ppm *ppm,
 		       unsigned int idx, unsigned int npods, unsigned int tid)
@@ -116,7 +61,7 @@  cxgbit_ppod_write_idata(struct cxgbi_ppm *ppm, struct cxgbit_sock *csk,
 	ppod = (struct cxgbi_pagepod *)(idata + 1);
 
 	for (i = 0; i < npods; i++, ppod++)
-		cxgbit_set_one_ppod(ppod, ttinfo, sg_pp, sg_off);
+		cxgbi_ppm_set_one_ppod(ppod, ttinfo, sg_pp, sg_off);
 
 	__skb_queue_tail(&csk->ppodq, skb);