diff mbox series

Staging: net: nic: Add error pointer check in otx2_flows.c

Message ID 20240922185235.50413-1-kdipendra88@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Staging: net: nic: Add error pointer check in otx2_flows.c | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7 this patch: 7
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang fail Errors and warnings before: 16 this patch: 20
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 16 this patch: 19
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 33 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Dipendra Khadka Sept. 22, 2024, 6:52 p.m. UTC
Smatch reported following:
'''
drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:123 otx2_alloc_mcam_entries() error: 'rsp' dereferencing possible ERR_PTR()
drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:201 otx2_mcam_entry_init() error: 'rsp' dereferencing possible ERR_PTR()
drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:236 otx2_mcam_entry_init() error: 'frsp' dereferencing possible ERR_PTR()
'''

Adding error pointer check after calling otx2_mbox_get_rsp.

Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
---
 .../ethernet/marvell/octeontx2/nic/otx2_flows.c   | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

kernel test robot Sept. 23, 2024, 12:26 a.m. UTC | #1
Hi Dipendra,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Dipendra-Khadka/Staging-net-nic-Add-error-pointer-check-in-otx2_flows-c/20240923-025325
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20240922185235.50413-1-kdipendra88%40gmail.com
patch subject: [PATCH] Staging: net: nic: Add error pointer check in otx2_flows.c
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240923/202409230844.gM9kqV79-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240923/202409230844.gM9kqV79-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409230844.gM9kqV79-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c: In function 'otx2_alloc_mcam_entries':
>> drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:124:39: error: 'bfvf' undeclared (first use in this function); did you mean 'pfvf'?
     124 |                         mutex_unlock(&bfvf->mbox.lock);
         |                                       ^~~~
         |                                       pfvf
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:124:39: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c: In function 'otx2_mcam_entry_init':
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:207:31: error: 'bfvf' undeclared (first use in this function); did you mean 'pfvf'?
     207 |                 mutex_unlock(&bfvf->mbox.lock);
         |                               ^~~~
         |                               pfvf


vim +124 drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c

    71	
    72	int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
    73	{
    74		struct otx2_flow_config *flow_cfg = pfvf->flow_cfg;
    75		struct npc_mcam_alloc_entry_req *req;
    76		struct npc_mcam_alloc_entry_rsp *rsp;
    77		int ent, allocated = 0;
    78	
    79		/* Free current ones and allocate new ones with requested count */
    80		otx2_free_ntuple_mcam_entries(pfvf);
    81	
    82		if (!count)
    83			return 0;
    84	
    85		flow_cfg->flow_ent = devm_kmalloc_array(pfvf->dev, count,
    86							sizeof(u16), GFP_KERNEL);
    87		if (!flow_cfg->flow_ent) {
    88			netdev_err(pfvf->netdev,
    89				   "%s: Unable to allocate memory for flow entries\n",
    90				    __func__);
    91			return -ENOMEM;
    92		}
    93	
    94		mutex_lock(&pfvf->mbox.lock);
    95	
    96		/* In a single request a max of NPC_MAX_NONCONTIG_ENTRIES MCAM entries
    97		 * can only be allocated.
    98		 */
    99		while (allocated < count) {
   100			req = otx2_mbox_alloc_msg_npc_mcam_alloc_entry(&pfvf->mbox);
   101			if (!req)
   102				goto exit;
   103	
   104			req->contig = false;
   105			req->count = (count - allocated) > NPC_MAX_NONCONTIG_ENTRIES ?
   106					NPC_MAX_NONCONTIG_ENTRIES : count - allocated;
   107	
   108			/* Allocate higher priority entries for PFs, so that VF's entries
   109			 * will be on top of PF.
   110			 */
   111			if (!is_otx2_vf(pfvf->pcifunc)) {
   112				req->priority = NPC_MCAM_HIGHER_PRIO;
   113				req->ref_entry = flow_cfg->def_ent[0];
   114			}
   115	
   116			/* Send message to AF */
   117			if (otx2_sync_mbox_msg(&pfvf->mbox))
   118				goto exit;
   119	
   120			rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
   121				(&pfvf->mbox.mbox, 0, &req->hdr);
   122	
   123			if (IS_ERR(rsp)) {
 > 124				mutex_unlock(&bfvf->mbox.lock);
   125				return PTR_ERR(rsp);
   126			}
   127	
   128			for (ent = 0; ent < rsp->count; ent++)
   129				flow_cfg->flow_ent[ent + allocated] = rsp->entry_list[ent];
   130	
   131			allocated += rsp->count;
   132	
   133			/* If this request is not fulfilled, no need to send
   134			 * further requests.
   135			 */
   136			if (rsp->count != req->count)
   137				break;
   138		}
   139	
   140		/* Multiple MCAM entry alloc requests could result in non-sequential
   141		 * MCAM entries in the flow_ent[] array. Sort them in an ascending order,
   142		 * otherwise user installed ntuple filter index and MCAM entry index will
   143		 * not be in sync.
   144		 */
   145		if (allocated)
   146			sort(&flow_cfg->flow_ent[0], allocated,
   147			     sizeof(flow_cfg->flow_ent[0]), mcam_entry_cmp, NULL);
   148	
   149	exit:
   150		mutex_unlock(&pfvf->mbox.lock);
   151	
   152		flow_cfg->max_flows = allocated;
   153	
   154		if (allocated) {
   155			pfvf->flags |= OTX2_FLAG_MCAM_ENTRIES_ALLOC;
   156			pfvf->flags |= OTX2_FLAG_NTUPLE_SUPPORT;
   157		}
   158	
   159		if (allocated != count)
   160			netdev_info(pfvf->netdev,
   161				    "Unable to allocate %d MCAM entries, got only %d\n",
   162				    count, allocated);
   163		return allocated;
   164	}
   165	EXPORT_SYMBOL(otx2_alloc_mcam_entries);
   166
kernel test robot Sept. 23, 2024, 2:57 a.m. UTC | #2
Hi Dipendra,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Dipendra-Khadka/Staging-net-nic-Add-error-pointer-check-in-otx2_flows-c/20240923-025325
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20240922185235.50413-1-kdipendra88%40gmail.com
patch subject: [PATCH] Staging: net: nic: Add error pointer check in otx2_flows.c
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240923/202409231056.4rLGE5NG-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 8663a75fa2f31299ab8d1d90288d9df92aadee88)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240923/202409231056.4rLGE5NG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409231056.4rLGE5NG-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/tcp.h:17:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:10:
   In file included from include/linux/mm.h:2228:
   include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     500 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     501 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     507 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     508 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     519 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     520 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     528 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     529 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:8:
   In file included from include/net/ipv6.h:12:
   In file included from include/linux/ipv6.h:101:
   In file included from include/linux/tcp.h:17:
   In file included from include/linux/skbuff.h:28:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:8:
   In file included from include/net/ipv6.h:12:
   In file included from include/linux/ipv6.h:101:
   In file included from include/linux/tcp.h:17:
   In file included from include/linux/skbuff.h:28:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:8:
   In file included from include/net/ipv6.h:12:
   In file included from include/linux/ipv6.h:101:
   In file included from include/linux/tcp.h:17:
   In file included from include/linux/skbuff.h:28:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:124:18: error: use of undeclared identifier 'bfvf'; did you mean 'pfvf'?
     124 |                         mutex_unlock(&bfvf->mbox.lock);
         |                                       ^~~~
         |                                       pfvf
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:72:46: note: 'pfvf' declared here
      72 | int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
         |                                              ^
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:207:17: error: use of undeclared identifier 'bfvf'; did you mean 'pfvf'?
     207 |                 mutex_unlock(&bfvf->mbox.lock);
         |                               ^~~~
         |                               pfvf
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:167:43: note: 'pfvf' declared here
     167 | int otx2_mcam_entry_init(struct otx2_nic *pfvf)
         |                                           ^
   17 warnings and 2 errors generated.


vim +124 drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c

    71	
    72	int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
    73	{
    74		struct otx2_flow_config *flow_cfg = pfvf->flow_cfg;
    75		struct npc_mcam_alloc_entry_req *req;
    76		struct npc_mcam_alloc_entry_rsp *rsp;
    77		int ent, allocated = 0;
    78	
    79		/* Free current ones and allocate new ones with requested count */
    80		otx2_free_ntuple_mcam_entries(pfvf);
    81	
    82		if (!count)
    83			return 0;
    84	
    85		flow_cfg->flow_ent = devm_kmalloc_array(pfvf->dev, count,
    86							sizeof(u16), GFP_KERNEL);
    87		if (!flow_cfg->flow_ent) {
    88			netdev_err(pfvf->netdev,
    89				   "%s: Unable to allocate memory for flow entries\n",
    90				    __func__);
    91			return -ENOMEM;
    92		}
    93	
    94		mutex_lock(&pfvf->mbox.lock);
    95	
    96		/* In a single request a max of NPC_MAX_NONCONTIG_ENTRIES MCAM entries
    97		 * can only be allocated.
    98		 */
    99		while (allocated < count) {
   100			req = otx2_mbox_alloc_msg_npc_mcam_alloc_entry(&pfvf->mbox);
   101			if (!req)
   102				goto exit;
   103	
   104			req->contig = false;
   105			req->count = (count - allocated) > NPC_MAX_NONCONTIG_ENTRIES ?
   106					NPC_MAX_NONCONTIG_ENTRIES : count - allocated;
   107	
   108			/* Allocate higher priority entries for PFs, so that VF's entries
   109			 * will be on top of PF.
   110			 */
   111			if (!is_otx2_vf(pfvf->pcifunc)) {
   112				req->priority = NPC_MCAM_HIGHER_PRIO;
   113				req->ref_entry = flow_cfg->def_ent[0];
   114			}
   115	
   116			/* Send message to AF */
   117			if (otx2_sync_mbox_msg(&pfvf->mbox))
   118				goto exit;
   119	
   120			rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
   121				(&pfvf->mbox.mbox, 0, &req->hdr);
   122	
   123			if (IS_ERR(rsp)) {
 > 124				mutex_unlock(&bfvf->mbox.lock);
   125				return PTR_ERR(rsp);
   126			}
   127	
   128			for (ent = 0; ent < rsp->count; ent++)
   129				flow_cfg->flow_ent[ent + allocated] = rsp->entry_list[ent];
   130	
   131			allocated += rsp->count;
   132	
   133			/* If this request is not fulfilled, no need to send
   134			 * further requests.
   135			 */
   136			if (rsp->count != req->count)
   137				break;
   138		}
   139	
   140		/* Multiple MCAM entry alloc requests could result in non-sequential
   141		 * MCAM entries in the flow_ent[] array. Sort them in an ascending order,
   142		 * otherwise user installed ntuple filter index and MCAM entry index will
   143		 * not be in sync.
   144		 */
   145		if (allocated)
   146			sort(&flow_cfg->flow_ent[0], allocated,
   147			     sizeof(flow_cfg->flow_ent[0]), mcam_entry_cmp, NULL);
   148	
   149	exit:
   150		mutex_unlock(&pfvf->mbox.lock);
   151	
   152		flow_cfg->max_flows = allocated;
   153	
   154		if (allocated) {
   155			pfvf->flags |= OTX2_FLAG_MCAM_ENTRIES_ALLOC;
   156			pfvf->flags |= OTX2_FLAG_NTUPLE_SUPPORT;
   157		}
   158	
   159		if (allocated != count)
   160			netdev_info(pfvf->netdev,
   161				    "Unable to allocate %d MCAM entries, got only %d\n",
   162				    count, allocated);
   163		return allocated;
   164	}
   165	EXPORT_SYMBOL(otx2_alloc_mcam_entries);
   166
Simon Horman Sept. 23, 2024, 3:56 p.m. UTC | #3
On Sun, Sep 22, 2024 at 06:52:35PM +0000, Dipendra Khadka wrote:
> Smatch reported following:
> '''
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:123 otx2_alloc_mcam_entries() error: 'rsp' dereferencing possible ERR_PTR()
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:201 otx2_mcam_entry_init() error: 'rsp' dereferencing possible ERR_PTR()
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:236 otx2_mcam_entry_init() error: 'frsp' dereferencing possible ERR_PTR()
> '''
> 
> Adding error pointer check after calling otx2_mbox_get_rsp.
> 
> Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>

Hi Dipendra,

As noted by Andrew Lunn in relation to another patch [1],
this driver isn't in Staging so the subject is not correct.
And moreover, as Andrew suggested, please take a look at [2].

[1] https://lore.kernel.org/all/13fbb6c3-661f-477a-b33b-99303cd11622@lunn.ch/
[2] https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

> ---
>  .../ethernet/marvell/octeontx2/nic/otx2_flows.c   | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> index 98c31a16c70b..4b61236c7c41 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> @@ -120,6 +120,11 @@ int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
>  		rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
>  			(&pfvf->mbox.mbox, 0, &req->hdr);

nit: No blank line here please.
     Similarly in the other hunks of this patch.

>  
> +		if (IS_ERR(rsp)) {
> +			mutex_unlock(&bfvf->mbox.lock);

This doesn't compile as bfvf doesn't exit in this context.

> +			return PTR_ERR(rsp);

Looking at error handling elsewhere in the same loop, perhaps this
is appropriate instead of returning.

			goto exit;

> +		}
> +
>  		for (ent = 0; ent < rsp->count; ent++)
>  			flow_cfg->flow_ent[ent + allocated] = rsp->entry_list[ent];
>  

...
Dipendra Khadka Sept. 23, 2024, 4:03 p.m. UTC | #4
Hi,

Thank you for the response.I had already sent the v2 patch. I will
send v3 addressing all the issues.

On Mon, 23 Sept 2024 at 21:41, Simon Horman <horms@kernel.org> wrote:
>
> On Sun, Sep 22, 2024 at 06:52:35PM +0000, Dipendra Khadka wrote:
> > Smatch reported following:
> > '''
> > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:123 otx2_alloc_mcam_entries() error: 'rsp' dereferencing possible ERR_PTR()
> > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:201 otx2_mcam_entry_init() error: 'rsp' dereferencing possible ERR_PTR()
> > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:236 otx2_mcam_entry_init() error: 'frsp' dereferencing possible ERR_PTR()
> > '''
> >
> > Adding error pointer check after calling otx2_mbox_get_rsp.
> >
> > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
>
> Hi Dipendra,
>
> As noted by Andrew Lunn in relation to another patch [1],
> this driver isn't in Staging so the subject is not correct.
> And moreover, as Andrew suggested, please take a look at [2].
>
> [1] https://lore.kernel.org/all/13fbb6c3-661f-477a-b33b-99303cd11622@lunn.ch/
> [2] https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>

> > ---
> >  .../ethernet/marvell/octeontx2/nic/otx2_flows.c   | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > index 98c31a16c70b..4b61236c7c41 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > @@ -120,6 +120,11 @@ int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
> >               rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
> >                       (&pfvf->mbox.mbox, 0, &req->hdr);
>
> nit: No blank line here please.
>      Similarly in the other hunks of this patch.
>
> >
> > +             if (IS_ERR(rsp)) {
> > +                     mutex_unlock(&bfvf->mbox.lock);
>
> This doesn't compile as bfvf doesn't exit in this context.
>
> > +                     return PTR_ERR(rsp);
>
> Looking at error handling elsewhere in the same loop, perhaps this
> is appropriate instead of returning.
>
>                         goto exit;
>
> > +             }
> > +
> >               for (ent = 0; ent < rsp->count; ent++)
> >                       flow_cfg->flow_ent[ent + allocated] = rsp->entry_list[ent];
> >
>
> ...

Best Regards,
Dipendra
Dipendra Khadka Sept. 26, 2024, 5:30 a.m. UTC | #5
Hi Simon,

On Mon, 23 Sept 2024 at 21:48, Dipendra Khadka <kdipendra88@gmail.com> wrote:
>
> Hi,
>
> Thank you for the response.I had already sent the v2 patch. I will
> send v3 addressing all the issues.
>
> On Mon, 23 Sept 2024 at 21:41, Simon Horman <horms@kernel.org> wrote:
> >
> > On Sun, Sep 22, 2024 at 06:52:35PM +0000, Dipendra Khadka wrote:
> > > Smatch reported following:
> > > '''
> > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:123 otx2_alloc_mcam_entries() error: 'rsp' dereferencing possible ERR_PTR()
> > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:201 otx2_mcam_entry_init() error: 'rsp' dereferencing possible ERR_PTR()
> > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:236 otx2_mcam_entry_init() error: 'frsp' dereferencing possible ERR_PTR()
> > > '''
> > >
> > > Adding error pointer check after calling otx2_mbox_get_rsp.
> > >
> > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> >
> > Hi Dipendra,
> >
> > As noted by Andrew Lunn in relation to another patch [1],
> > this driver isn't in Staging so the subject is not correct.
> > And moreover, as Andrew suggested, please take a look at [2].
> >
> > [1] https://lore.kernel.org/all/13fbb6c3-661f-477a-b33b-99303cd11622@lunn.ch/
> > [2] https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> >
>
> > > ---
> > >  .../ethernet/marvell/octeontx2/nic/otx2_flows.c   | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > > index 98c31a16c70b..4b61236c7c41 100644
> > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > > @@ -120,6 +120,11 @@ int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
> > >               rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
> > >                       (&pfvf->mbox.mbox, 0, &req->hdr);
> >
> > nit: No blank line here please.
> >      Similarly in the other hunks of this patch.
> >
> > >
> > > +             if (IS_ERR(rsp)) {
> > > +                     mutex_unlock(&bfvf->mbox.lock);
> >
> > This doesn't compile as bfvf doesn't exit in this context.
> >
> > > +                     return PTR_ERR(rsp);
> >
> > Looking at error handling elsewhere in the same loop, perhaps this
> > is appropriate instead of returning.
> >
> >                         goto exit;

Is this ok to follow?

if (IS_ERR(rsp)) {
                        allocated = PTR_ERR(rsp);
                        goto exit;
                }

> >
> > > +             }
> > > +
> > >               for (ent = 0; ent < rsp->count; ent++)
> > >                       flow_cfg->flow_ent[ent + allocated] = rsp->entry_list[ent];
> > >
> >
> > ...
>
> Best Regards,
> Dipendra

Best regards,
Dipendra Khadka
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
index 98c31a16c70b..4b61236c7c41 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
@@ -120,6 +120,11 @@  int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
 		rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
 			(&pfvf->mbox.mbox, 0, &req->hdr);
 
+		if (IS_ERR(rsp)) {
+			mutex_unlock(&bfvf->mbox.lock);
+			return PTR_ERR(rsp);
+		}
+
 		for (ent = 0; ent < rsp->count; ent++)
 			flow_cfg->flow_ent[ent + allocated] = rsp->entry_list[ent];
 
@@ -198,6 +203,11 @@  int otx2_mcam_entry_init(struct otx2_nic *pfvf)
 	rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
 	       (&pfvf->mbox.mbox, 0, &req->hdr);
 
+	if (IS_ERR(rsp)) {
+		mutex_unlock(&bfvf->mbox.lock);
+		return PTR_ERR(rsp);
+	}
+
 	if (rsp->count != req->count) {
 		netdev_info(pfvf->netdev,
 			    "Unable to allocate MCAM entries for ucast, vlan and vf_vlan\n");
@@ -233,6 +243,11 @@  int otx2_mcam_entry_init(struct otx2_nic *pfvf)
 	frsp = (struct npc_get_field_status_rsp *)otx2_mbox_get_rsp
 	       (&pfvf->mbox.mbox, 0, &freq->hdr);
 
+	if (IS_ERR(frsp)) {
+		mutex_unlock(&pfvf->mbox.lock);
+		return PTR_ERR(frsp);
+	}
+
 	if (frsp->enable) {
 		pfvf->flags |= OTX2_FLAG_RX_VLAN_SUPPORT;
 		pfvf->flags |= OTX2_FLAG_VF_VLAN_SUPPORT;