* [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).