From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
To: Harman Kalra <hkalra@marvell.com>,
Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>,
Vamsi Krishna Attunuru <vattunuru@marvell.com>,
Kiran Kumar Kokkilagadda <kirankumark@marvell.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"thomas@monjalon.net" <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH] drivers/octeontx2: fix coverity issues
Date: Mon, 5 Aug 2019 04:05:16 +0000 [thread overview]
Message-ID: <BYAPR18MB2424534F9BCC4C8A90EEAC34C8DA0@BYAPR18MB2424.namprd18.prod.outlook.com> (raw)
In-Reply-To: <1564903224-16945-1-git-send-email-hkalra@marvell.com>
> -----Original Message-----
> From: Harman Kalra <hkalra@marvell.com>
> Sent: Sunday, August 4, 2019 12:51 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nithin Kumar
> Dabilpuram <ndabilpuram@marvell.com>; Vamsi Krishna Attunuru
> <vattunuru@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>
> Cc: dev@dpdk.org; Harman Kalra <hkalra@marvell.com>
> Subject: [PATCH] drivers/octeontx2: fix coverity issues
>
> Addresed issues reported by coverity: NULL pointer dereferencing issues,
> unchecked return value, uinitialized scalar value, probable deadcode cases,
> unintended sign extension, bad bit shift operation, Wrong sizeof argument
> (SIZEOF_MISMATCH)
>
> Coverity issue: 343396, 345028, 344977, 345015, 345025, 344969, Coverity
> issue: 345014, 344966, 343437, 344993, 345007, 344988, Coverity issue: 343405,
> 344999, 345003
>
> Fixes: 58f6f93c34c1 ("net/octeontx2: add module EEPROM dump")
> Fixes: 38f566280abb ("net/octeontx2: add link stats operations")
> Fixes: b5dc3140448e ("net/octeontx2: support base PTP")
> Fixes: ba1b3b081edf ("net/octeontx2: support VLAN offloads")
> Fixes: 092b38341859 ("net/octeontx2: add flow init and fini")
> Fixes: 3da1b85b6d06 ("common/octeontx2: add FLR IRQ handler")
> Fixes: 2548ab774f92 ("mempool/octeontx2: add context dump support")
> Fixes: 2b71657c8660 ("common/octeontx2: add mbox request and response
> definition")
>
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
> ---
> drivers/common/octeontx2/otx2_dev.c | 3 +--
> drivers/common/octeontx2/otx2_mbox.h | 4 ++--
> drivers/mempool/octeontx2/otx2_mempool_debug.c | 2 +-
> drivers/net/octeontx2/otx2_ethdev.c | 3 ++-
> drivers/net/octeontx2/otx2_ethdev_ops.c | 7 ++++++-
> drivers/net/octeontx2/otx2_flow.c | 10 ++++------
> drivers/net/octeontx2/otx2_link.c | 8 ++++++--
> drivers/net/octeontx2/otx2_ptp.c | 15 ++++++++++++---
> drivers/net/octeontx2/otx2_tm.h | 2 +-
> drivers/net/octeontx2/otx2_vlan.c | 12 +++++++-----
> 10 files changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/common/octeontx2/otx2_dev.c
> b/drivers/common/octeontx2/otx2_dev.c
> index f58a6e725..910270975 100644
> --- a/drivers/common/octeontx2/otx2_dev.c
> +++ b/drivers/common/octeontx2/otx2_dev.c
> @@ -791,9 +791,8 @@ otx2_pf_vf_flr_irq(void *param)
> if (!(intr & (1ULL << vf)))
> continue;
>
> - vf = 64 * i + vf;
> otx2_base_dbg("FLR: i :%d intr: 0x%" PRIx64 ", vf-
> %d",
> - i, intr, vf);
> + i, intr, (64 * i + vf));
> /* Clear interrupt */
> otx2_write64(BIT_ULL(vf), bar2 +
> RVU_PF_VFFLR_INTX(i));
> /* Disable the interrupt */
> diff --git a/drivers/common/octeontx2/otx2_mbox.h
> b/drivers/common/octeontx2/otx2_mbox.h
> index b2c59c86e..009015fa5 100644
> --- a/drivers/common/octeontx2/otx2_mbox.h
> +++ b/drivers/common/octeontx2/otx2_mbox.h
> @@ -13,8 +13,8 @@
>
> #include <otx2_common.h>
>
> -#define SZ_64K (64 * 1024)
> -#define SZ_1K (1 * 1024)
> +#define SZ_64K (64ULL * 1024ULL)
> +#define SZ_1K (1ULL * 1024ULL)
> #define MBOX_SIZE SZ_64K
>
> /* AF/PF: PF initiated, PF/VF VF initiated */ diff --git
> a/drivers/mempool/octeontx2/otx2_mempool_debug.c
> b/drivers/mempool/octeontx2/otx2_mempool_debug.c
> index eef61ef07..8bd395389 100644
> --- a/drivers/mempool/octeontx2/otx2_mempool_debug.c
> +++ b/drivers/mempool/octeontx2/otx2_mempool_debug.c
> @@ -91,7 +91,7 @@ otx2_mempool_ctx_dump(struct otx2_npa_lf *lf)
> struct npa_aq_enq_req *aq;
> struct npa_aq_enq_rsp *rsp;
> uint32_t q;
> - int rc;
> + int rc = 0;
>
> for (q = 0; q < lf->nr_pools; q++) {
> /* Skip disabled POOL */
> diff --git a/drivers/net/octeontx2/otx2_ethdev.c
> b/drivers/net/octeontx2/otx2_ethdev.c
> index 3fb7bd93f..99fcb36c3 100644
> --- a/drivers/net/octeontx2/otx2_ethdev.c
> +++ b/drivers/net/octeontx2/otx2_ethdev.c
> @@ -749,7 +749,8 @@ nix_sq_uninit(struct otx2_eth_txq *txq)
> while (count) {
> void *next_sqb;
>
> - next_sqb = *(void **)((uintptr_t)sqb_buf + ((sqes_per_sqb
> - 1) *
> + next_sqb = *(void **)((uintptr_t)sqb_buf + (uint32_t)
> + ((sqes_per_sqb - 1) *
> nix_sq_max_sqe_sz(txq)));
> npa_lf_aura_op_free(txq->sqb_pool->pool_id, 1,
> (uint64_t)sqb_buf);
> diff --git a/drivers/net/octeontx2/otx2_ethdev_ops.c
> b/drivers/net/octeontx2/otx2_ethdev_ops.c
> index 5a16a3c04..2a79e20bb 100644
> --- a/drivers/net/octeontx2/otx2_ethdev_ops.c
> +++ b/drivers/net/octeontx2/otx2_ethdev_ops.c
> @@ -352,10 +352,15 @@ nix_get_fwdata(struct otx2_eth_dev *dev) {
> struct otx2_mbox *mbox = dev->mbox;
> struct cgx_fw_data *rsp = NULL;
> + int rc;
>
> otx2_mbox_alloc_msg_cgx_get_aux_link_info(mbox);
>
> - otx2_mbox_process_msg(mbox, (void *)&rsp);
> + rc = otx2_mbox_process_msg(mbox, (void *)&rsp);
> + if (rc) {
> + otx2_err("Failed to get fw data: %d", rc);
> + return NULL;
> + }
>
> return rsp;
> }
> diff --git a/drivers/net/octeontx2/otx2_flow.c
> b/drivers/net/octeontx2/otx2_flow.c
> index c463075cb..bdbf123a9 100644
> --- a/drivers/net/octeontx2/otx2_flow.c
> +++ b/drivers/net/octeontx2/otx2_flow.c
> @@ -844,7 +844,7 @@ otx2_flow_init(struct otx2_eth_dev *hw)
> }
>
> npc->free_entries = rte_zmalloc(NULL, npc->flow_max_priority
> - * sizeof(struct rte_bitmap),
> + * sizeof(struct rte_bitmap *),
> 0);
> if (npc->free_entries == NULL) {
> otx2_err("free_entries alloc failed"); @@ -853,7 +853,7 @@
> otx2_flow_init(struct otx2_eth_dev *hw)
> }
>
> npc->free_entries_rev = rte_zmalloc(NULL, npc->flow_max_priority
> - * sizeof(struct rte_bitmap),
> + * sizeof(struct rte_bitmap *),
> 0);
> if (npc->free_entries_rev == NULL) {
> otx2_err("free_entries_rev alloc failed"); @@ -862,7 +862,7
> @@ otx2_flow_init(struct otx2_eth_dev *hw)
> }
>
> npc->live_entries = rte_zmalloc(NULL, npc->flow_max_priority
> - * sizeof(struct rte_bitmap),
> + * sizeof(struct rte_bitmap *),
> 0);
> if (npc->live_entries == NULL) {
> otx2_err("live_entries alloc failed"); @@ -871,7 +871,7 @@
> otx2_flow_init(struct otx2_eth_dev *hw)
> }
>
> npc->live_entries_rev = rte_zmalloc(NULL, npc->flow_max_priority
> - * sizeof(struct rte_bitmap),
> + * sizeof(struct rte_bitmap *),
> 0);
> if (npc->live_entries_rev == NULL) {
> otx2_err("live_entries_rev alloc failed"); @@ -948,8 +948,6
> @@ otx2_flow_init(struct otx2_eth_dev *hw)
> rte_free(npc->flow_entry_info);
> if (npc_mem)
> rte_free(npc_mem);
> - if (nix_mem)
> - rte_free(nix_mem);
> return rc;
> }
>
> diff --git a/drivers/net/octeontx2/otx2_link.c
> b/drivers/net/octeontx2/otx2_link.c
> index 8fcbdc9b7..725b793d4 100644
> --- a/drivers/net/octeontx2/otx2_link.c
> +++ b/drivers/net/octeontx2/otx2_link.c
> @@ -52,10 +52,14 @@ otx2_eth_dev_link_status_update(struct otx2_dev
> *dev,
> struct cgx_link_user_info *link)
> {
> struct otx2_eth_dev *otx2_dev = (struct otx2_eth_dev *)dev;
> - struct rte_eth_dev *eth_dev = otx2_dev->eth_dev;
> struct rte_eth_link eth_link;
> + struct rte_eth_dev *eth_dev;
>
> - if (!link || !dev || !eth_dev->data->dev_conf.intr_conf.lsc)
> + if (!link || !dev)
> + return;
> +
> + eth_dev = otx2_dev->eth_dev;
> + if (!eth_dev || !eth_dev->data->dev_conf.intr_conf.lsc)
> return;
>
> if (nix_wait_for_link_cfg(otx2_dev)) { diff --git
> a/drivers/net/octeontx2/otx2_ptp.c b/drivers/net/octeontx2/otx2_ptp.c
> index 52e5456b5..fd13c2678 100644
> --- a/drivers/net/octeontx2/otx2_ptp.c
> +++ b/drivers/net/octeontx2/otx2_ptp.c
> @@ -102,7 +102,7 @@ nix_ptp_config(struct rte_eth_dev *eth_dev, int en)
> {
> struct otx2_eth_dev *dev = otx2_eth_pmd_priv(eth_dev);
> struct otx2_mbox *mbox = dev->mbox;
> - uint8_t rc = 0;
> + uint8_t rc = -EINVAL;
>
> if (otx2_dev_is_vf(dev))
> return rc;
> @@ -136,9 +136,16 @@ int
> otx2_eth_dev_ptp_info_update(struct otx2_dev *dev, bool ptp_en) {
> struct otx2_eth_dev *otx2_dev = (struct otx2_eth_dev *)dev;
> - struct rte_eth_dev *eth_dev = otx2_dev->eth_dev;
> + struct rte_eth_dev *eth_dev;
> int i;
>
> + if (!dev)
> + return -EINVAL;
> +
> + eth_dev = otx2_dev->eth_dev;
> + if (!eth_dev)
> + return -EINVAL;
> +
> otx2_dev->ptp_en = ptp_en;
> for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> struct otx2_eth_rxq *rxq = eth_dev->data->rx_queues[i];
> @@ -174,8 +181,10 @@ otx2_nix_timesync_enable(struct rte_eth_dev
> *eth_dev)
> ts = rte_eth_dma_zone_reserve(eth_dev, "otx2_ts",
> 0, OTX2_ALIGN, OTX2_ALIGN,
> dev->node);
> - if (ts == NULL)
> + if (ts == NULL) {
> otx2_err("Failed to allocate mem for tx tstamp addr");
> + return -ENOMEM;
> + }
>
> dev->tstamp.tx_tstamp_iova = ts->iova;
> dev->tstamp.tx_tstamp = ts->addr;
> diff --git a/drivers/net/octeontx2/otx2_tm.h
> b/drivers/net/octeontx2/otx2_tm.h index 2a009eece..4712b0935 100644
> --- a/drivers/net/octeontx2/otx2_tm.h
> +++ b/drivers/net/octeontx2/otx2_tm.h
> @@ -62,7 +62,7 @@ TAILQ_HEAD(otx2_nix_tm_node_list,
> otx2_nix_tm_node); TAILQ_HEAD(otx2_nix_tm_shaper_profile_list,
> otx2_nix_tm_shaper_profile);
>
> #define MAX_SCHED_WEIGHT ((uint8_t)~0)
> -#define NIX_TM_RR_QUANTUM_MAX ((1 << 24) - 1)
> +#define NIX_TM_RR_QUANTUM_MAX (BIT_ULL(24) - 1)
>
> /* DEFAULT_RR_WEIGHT * NIX_TM_RR_QUANTUM_MAX /
> MAX_SCHED_WEIGHT */
> /* = NIX_MAX_HW_MTU */
> diff --git a/drivers/net/octeontx2/otx2_vlan.c
> b/drivers/net/octeontx2/otx2_vlan.c
> index 189c45174..c01089b44 100644
> --- a/drivers/net/octeontx2/otx2_vlan.c
> +++ b/drivers/net/octeontx2/otx2_vlan.c
> @@ -303,7 +303,7 @@ nix_vlan_mcam_config(struct rte_eth_dev *eth_dev,
> entry.kw[kwi] |= NPC_LT_LB_CTAG << mkex->lb_lt_offset;
> entry.kw_mask[kwi] |= 0xFULL << mkex->lb_lt_offset;
>
> - mcam_data = (vlan_id << 16);
> + mcam_data = ((uint32_t)vlan_id << 16);
> mcam_mask = (BIT_ULL(16) - 1) << 16;
> otx2_mbox_memcpy(key_data + mkex->lb_xtract.key_off,
> &mcam_data, mkex->lb_xtract.len + 1);
> @@ -649,7 +649,9 @@ otx2_nix_vlan_filter_set(struct rte_eth_dev
> *eth_dev, uint16_t vlan_id,
> } else {
> TAILQ_FOREACH(entry, &vlan->fltr_tbl, next) {
> if (entry->vlan_id == vlan_id) {
> - nix_vlan_mcam_free(dev, entry-
> >mcam_idx);
> + rc = nix_vlan_mcam_free(dev, entry-
> >mcam_idx);
> + if (rc)
> + return rc;
> TAILQ_REMOVE(&vlan->fltr_tbl, entry, next);
> rte_free(entry);
> break;
> @@ -707,7 +709,7 @@ otx2_nix_vlan_offload_set(struct rte_eth_dev
> *eth_dev, int mask)
> struct otx2_eth_dev *dev = otx2_eth_pmd_priv(eth_dev);
> uint64_t offloads = dev->rx_offloads;
> struct rte_eth_rxmode *rxmode;
> - int rc;
> + int rc = 0;
>
> rxmode = ð_dev->data->dev_conf.rxmode;
>
> @@ -837,8 +839,8 @@ otx2_nix_vlan_pvid_set(struct rte_eth_dev *dev,
> uint16_t vlan_id, int on)
> vtag_cfg->vtag_size = NIX_VTAGSIZE_T4;
>
> if (vlan->outer_vlan_tpid)
> - vtag_cfg->tx.vtag0 =
> - (vlan->outer_vlan_tpid << 16) | vlan_id;
> + vtag_cfg->tx.vtag0 = ((uint32_t)vlan-
> >outer_vlan_tpid
> + << 16) | vlan_id;
> else
> vtag_cfg->tx.vtag0 =
> ((RTE_ETHER_TYPE_VLAN << 16) | vlan_id);
> --
> 2.18.0
next prev parent reply other threads:[~2019-08-05 4:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-04 7:20 Harman Kalra
2019-08-05 4:05 ` Jerin Jacob Kollanukkaran [this message]
2019-08-05 16:48 ` Jerin Jacob Kollanukkaran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BYAPR18MB2424534F9BCC4C8A90EEAC34C8DA0@BYAPR18MB2424.namprd18.prod.outlook.com \
--to=jerinj@marvell.com \
--cc=dev@dpdk.org \
--cc=hkalra@marvell.com \
--cc=kirankumark@marvell.com \
--cc=ndabilpuram@marvell.com \
--cc=thomas@monjalon.net \
--cc=vattunuru@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).