* [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity @ 2019-07-12 16:39 Xiao Zhang 2019-07-15 2:36 ` Yang, Qiming 2019-07-15 4:06 ` Stephen Hemminger 0 siblings, 2 replies; 5+ messages in thread From: Xiao Zhang @ 2019-07-12 16:39 UTC (permalink / raw) To: dev Cc: qi.z.zhang, xiao.w.wang, beilei.xing, wenzhuo.lu, qiming.yang, konstantin.ananyev, jingjing.wu, Xiao Zhang This patch tries to fix the coverity issues of dereference after null check. Coverity issue: 343452 Coverity issue: 343447 Coverity issue: 343422 Coverity issue: 343416 Coverity issue: 343407 Coverity issue: 343403 Coverity issue: 13245 Signed-off-by: Xiao Zhang <xiao.zhang@intel.com> --- drivers/net/fm10k/fm10k_rxtx_vec.c | 3 +++ drivers/net/i40e/i40e_rxtx_vec_common.h | 3 +++ drivers/net/iavf/iavf_rxtx_vec_common.h | 3 +++ drivers/net/ice/ice_rxtx_vec_common.h | 3 +++ drivers/net/ixgbe/ixgbe_rxtx_vec_common.h | 3 +++ 5 files changed, 15 insertions(+) diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c index 788e248..cb840de 100644 --- a/drivers/net/fm10k/fm10k_rxtx_vec.c +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c @@ -602,6 +602,9 @@ fm10k_reassemble_packets(struct fm10k_rx_queue *rxq, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end != NULL) { /* processing a split packet */ diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h index 0e6ffa0..1351e41 100644 --- a/drivers/net/i40e/i40e_rxtx_vec_common.h +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h @@ -20,6 +20,9 @@ reassemble_packets(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end != NULL) { /* processing a split packet */ diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h b/drivers/net/iavf/iavf_rxtx_vec_common.h index db509d7..ac3d62a 100644 --- a/drivers/net/iavf/iavf_rxtx_vec_common.h +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h @@ -20,6 +20,9 @@ reassemble_packets(struct iavf_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned int pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end) { /* processing a split packet */ diff --git a/drivers/net/ice/ice_rxtx_vec_common.h b/drivers/net/ice/ice_rxtx_vec_common.h index c5f0d56..11da521 100644 --- a/drivers/net/ice/ice_rxtx_vec_common.h +++ b/drivers/net/ice/ice_rxtx_vec_common.h @@ -16,6 +16,9 @@ ice_rx_reassemble_packets(struct ice_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned int pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end) { /* processing a split packet */ diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h index a97c271..a95cc0a 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h @@ -19,6 +19,9 @@ reassemble_packets(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned int pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end != NULL) { /* processing a split packet */ -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity 2019-07-12 16:39 [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity Xiao Zhang @ 2019-07-15 2:36 ` Yang, Qiming 2019-07-15 2:45 ` Zhang, Xiao 2019-07-15 4:06 ` Stephen Hemminger 1 sibling, 1 reply; 5+ messages in thread From: Yang, Qiming @ 2019-07-15 2:36 UTC (permalink / raw) To: Zhang, Xiao, dev; +Cc: Zhang, Qi Z, Wang, Xiao W, Xing, Beilei, Lu, Wenzhuo Hi, Xiao Please explain what's the issue you fixed in the commit log. And for the fix patch, fix line is needed, please refer to other merged patches. commit e0474b94f8a36672d66be7408e3f7cf00e302329 is a good reference. Qiming -----Original Message----- From: Zhang, Xiao Sent: Saturday, July 13, 2019 12:40 AM To: dev@dpdk.org Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Xiao <xiao.zhang@intel.com> Subject: [DPDK] drivers/net: fix dereference after null check coverity This patch tries to fix the coverity issues of dereference after null check. Coverity issue: 343452 Coverity issue: 343447 Coverity issue: 343422 Coverity issue: 343416 Coverity issue: 343407 Coverity issue: 343403 Coverity issue: 13245 Signed-off-by: Xiao Zhang <xiao.zhang@intel.com> --- drivers/net/fm10k/fm10k_rxtx_vec.c | 3 +++ drivers/net/i40e/i40e_rxtx_vec_common.h | 3 +++ drivers/net/iavf/iavf_rxtx_vec_common.h | 3 +++ drivers/net/ice/ice_rxtx_vec_common.h | 3 +++ drivers/net/ixgbe/ixgbe_rxtx_vec_common.h | 3 +++ 5 files changed, 15 insertions(+) diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c index 788e248..cb840de 100644 --- a/drivers/net/fm10k/fm10k_rxtx_vec.c +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c @@ -602,6 +602,9 @@ fm10k_reassemble_packets(struct fm10k_rx_queue *rxq, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end != NULL) { /* processing a split packet */ diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h index 0e6ffa0..1351e41 100644 --- a/drivers/net/i40e/i40e_rxtx_vec_common.h +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h @@ -20,6 +20,9 @@ reassemble_packets(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end != NULL) { /* processing a split packet */ diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h b/drivers/net/iavf/iavf_rxtx_vec_common.h index db509d7..ac3d62a 100644 --- a/drivers/net/iavf/iavf_rxtx_vec_common.h +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h @@ -20,6 +20,9 @@ reassemble_packets(struct iavf_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned int pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end) { /* processing a split packet */ diff --git a/drivers/net/ice/ice_rxtx_vec_common.h b/drivers/net/ice/ice_rxtx_vec_common.h index c5f0d56..11da521 100644 --- a/drivers/net/ice/ice_rxtx_vec_common.h +++ b/drivers/net/ice/ice_rxtx_vec_common.h @@ -16,6 +16,9 @@ ice_rx_reassemble_packets(struct ice_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned int pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end) { /* processing a split packet */ diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h index a97c271..a95cc0a 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h @@ -19,6 +19,9 @@ reassemble_packets(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_bufs, struct rte_mbuf *end = rxq->pkt_last_seg; unsigned int pkt_idx, buf_idx; + if (!start) + return 0; + for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { if (end != NULL) { /* processing a split packet */ -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity 2019-07-15 2:36 ` Yang, Qiming @ 2019-07-15 2:45 ` Zhang, Xiao 0 siblings, 0 replies; 5+ messages in thread From: Zhang, Xiao @ 2019-07-15 2:45 UTC (permalink / raw) To: Yang, Qiming, dev; +Cc: Zhang, Qi Z, Wang, Xiao W, Xing, Beilei, Lu, Wenzhuo Hi Qiming, I will update the commit log. Thanks, Xiao > -----Original Message----- > From: Yang, Qiming > Sent: Monday, July 15, 2019 10:37 AM > To: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W > <xiao.w.wang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo > <wenzhuo.lu@intel.com> > Subject: RE: [DPDK] drivers/net: fix dereference after null check coverity > > Hi, Xiao > Please explain what's the issue you fixed in the commit log. > And for the fix patch, fix line is needed, please refer to other merged patches. > commit e0474b94f8a36672d66be7408e3f7cf00e302329 is a good reference. > > Qiming > -----Original Message----- > From: Zhang, Xiao > Sent: Saturday, July 13, 2019 12:40 AM > To: dev@dpdk.org > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W > <xiao.w.wang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo > <wenzhuo.lu@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Ananyev, > Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing > <jingjing.wu@intel.com>; Zhang, Xiao <xiao.zhang@intel.com> > Subject: [DPDK] drivers/net: fix dereference after null check coverity > > This patch tries to fix the coverity issues of dereference after null check. > > Coverity issue: 343452 > Coverity issue: 343447 > Coverity issue: 343422 > Coverity issue: 343416 > Coverity issue: 343407 > Coverity issue: 343403 > Coverity issue: 13245 > > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com> > --- > drivers/net/fm10k/fm10k_rxtx_vec.c | 3 +++ > drivers/net/i40e/i40e_rxtx_vec_common.h | 3 +++ > drivers/net/iavf/iavf_rxtx_vec_common.h | 3 +++ > drivers/net/ice/ice_rxtx_vec_common.h | 3 +++ > drivers/net/ixgbe/ixgbe_rxtx_vec_common.h | 3 +++ > 5 files changed, 15 insertions(+) > > diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c > b/drivers/net/fm10k/fm10k_rxtx_vec.c > index 788e248..cb840de 100644 > --- a/drivers/net/fm10k/fm10k_rxtx_vec.c > +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c > @@ -602,6 +602,9 @@ fm10k_reassemble_packets(struct fm10k_rx_queue > *rxq, > struct rte_mbuf *end = rxq->pkt_last_seg; > unsigned pkt_idx, buf_idx; > > + if (!start) > + return 0; > + > for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { > if (end != NULL) { > /* processing a split packet */ > diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h > b/drivers/net/i40e/i40e_rxtx_vec_common.h > index 0e6ffa0..1351e41 100644 > --- a/drivers/net/i40e/i40e_rxtx_vec_common.h > +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h > @@ -20,6 +20,9 @@ reassemble_packets(struct i40e_rx_queue *rxq, struct > rte_mbuf **rx_bufs, > struct rte_mbuf *end = rxq->pkt_last_seg; > unsigned pkt_idx, buf_idx; > > + if (!start) > + return 0; > + > for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { > if (end != NULL) { > /* processing a split packet */ > diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h > b/drivers/net/iavf/iavf_rxtx_vec_common.h > index db509d7..ac3d62a 100644 > --- a/drivers/net/iavf/iavf_rxtx_vec_common.h > +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h > @@ -20,6 +20,9 @@ reassemble_packets(struct iavf_rx_queue *rxq, struct > rte_mbuf **rx_bufs, > struct rte_mbuf *end = rxq->pkt_last_seg; > unsigned int pkt_idx, buf_idx; > > + if (!start) > + return 0; > + > for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { > if (end) { > /* processing a split packet */ > diff --git a/drivers/net/ice/ice_rxtx_vec_common.h > b/drivers/net/ice/ice_rxtx_vec_common.h > index c5f0d56..11da521 100644 > --- a/drivers/net/ice/ice_rxtx_vec_common.h > +++ b/drivers/net/ice/ice_rxtx_vec_common.h > @@ -16,6 +16,9 @@ ice_rx_reassemble_packets(struct ice_rx_queue *rxq, > struct rte_mbuf **rx_bufs, > struct rte_mbuf *end = rxq->pkt_last_seg; > unsigned int pkt_idx, buf_idx; > > + if (!start) > + return 0; > + > for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { > if (end) { > /* processing a split packet */ > diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h > b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h > index a97c271..a95cc0a 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h > +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h > @@ -19,6 +19,9 @@ reassemble_packets(struct ixgbe_rx_queue *rxq, struct > rte_mbuf **rx_bufs, > struct rte_mbuf *end = rxq->pkt_last_seg; > unsigned int pkt_idx, buf_idx; > > + if (!start) > + return 0; > + > for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) { > if (end != NULL) { > /* processing a split packet */ > -- > 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity 2019-07-12 16:39 [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity Xiao Zhang 2019-07-15 2:36 ` Yang, Qiming @ 2019-07-15 4:06 ` Stephen Hemminger 2019-07-15 7:38 ` Zhang, Xiao 1 sibling, 1 reply; 5+ messages in thread From: Stephen Hemminger @ 2019-07-15 4:06 UTC (permalink / raw) To: Xiao Zhang Cc: dev, qi.z.zhang, xiao.w.wang, beilei.xing, wenzhuo.lu, qiming.yang, konstantin.ananyev, jingjing.wu On Sat, 13 Jul 2019 00:39:47 +0800 Xiao Zhang <xiao.zhang@intel.com> wrote: > This patch tries to fix the coverity issues of dereference after null > check. > > Coverity issue: 343452 > Coverity issue: 343447 > Coverity issue: 343422 > Coverity issue: 343416 > Coverity issue: 343407 > Coverity issue: 343403 > Coverity issue: 13245 > > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com> I think this should be fixed deeper in the vector code. Example for ixgbe. static inline uint16_t reassemble_packets(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_bufs, uint16_t nb_bufs, uint8_t *split_flags) { struct rte_mbuf *pkts[nb_bufs]; /*finished pkts*/ struct rte_mbuf *start = rxq->pkt_first_seg; So start is rxq->pkt_first_seg. But caller has already checked for NULL here. It has iterated across the first packets but not updated rxq->first_seg. if (rxq->pkt_first_seg == NULL) { /* find the first split flag, and only reassemble then*/ while (i < nb_bufs && !split_flags[i]) i++; if (i == nb_bufs) return nb_bufs; } return i + reassemble_packets(rxq, &rx_pkts[i], nb_bufs - i, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity 2019-07-15 4:06 ` Stephen Hemminger @ 2019-07-15 7:38 ` Zhang, Xiao 0 siblings, 0 replies; 5+ messages in thread From: Zhang, Xiao @ 2019-07-15 7:38 UTC (permalink / raw) To: Stephen Hemminger Cc: dev, Zhang, Qi Z, Wang, Xiao W, Xing, Beilei, Lu, Wenzhuo, Yang, Qiming, Ananyev, Konstantin, Wu, Jingjing > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Monday, July 15, 2019 12:06 PM > To: Zhang, Xiao <xiao.zhang@intel.com> > Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W > <xiao.w.wang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo > <wenzhuo.lu@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Ananyev, > Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing > <jingjing.wu@intel.com> > Subject: Re: [dpdk-dev] [DPDK] drivers/net: fix dereference after null check > coverity > > On Sat, 13 Jul 2019 00:39:47 +0800 > Xiao Zhang <xiao.zhang@intel.com> wrote: > > > This patch tries to fix the coverity issues of dereference after null > > check. > > > > Coverity issue: 343452 > > Coverity issue: 343447 > > Coverity issue: 343422 > > Coverity issue: 343416 > > Coverity issue: 343407 > > Coverity issue: 343403 > > Coverity issue: 13245 > > > > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com> > > I think this should be fixed deeper in the vector code. > > Example for ixgbe. > > > static inline uint16_t > reassemble_packets(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_bufs, > uint16_t nb_bufs, uint8_t *split_flags) { > struct rte_mbuf *pkts[nb_bufs]; /*finished pkts*/ > struct rte_mbuf *start = rxq->pkt_first_seg; > > So start is rxq->pkt_first_seg. > > But caller has already checked for NULL here. > It has iterated across the first packets but not updated rxq->first_seg. Yes, this seems to be a bug but not a coverity issue. I will fix it. > if (rxq->pkt_first_seg == NULL) { > /* find the first split flag, and only reassemble then*/ > while (i < nb_bufs && !split_flags[i]) > i++; > if (i == nb_bufs) > return nb_bufs; > } > return i + reassemble_packets(rxq, &rx_pkts[i], nb_bufs - i, ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-15 7:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-12 16:39 [dpdk-dev] [DPDK] drivers/net: fix dereference after null check coverity Xiao Zhang 2019-07-15 2:36 ` Yang, Qiming 2019-07-15 2:45 ` Zhang, Xiao 2019-07-15 4:06 ` Stephen Hemminger 2019-07-15 7:38 ` Zhang, Xiao
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).