* [PATCH] net/ice: fix null pointer dereferences @ 2024-03-01 5:20 Wenwu Ma 2024-03-01 10:34 ` Bruce Richardson 2024-03-04 5:37 ` [PATCH v2] " Wenwu Ma 0 siblings, 2 replies; 5+ messages in thread From: Wenwu Ma @ 2024-03-01 5:20 UTC (permalink / raw) To: dev; +Cc: songx.jiale, Wenwu Ma This patch fixes two null pointer dereferences detected by coverity scan. Coverity issue: 414096 Fixes: 6ccef90ff5d3 ("net/ice: support VSI level bandwidth config") Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> --- drivers/net/ice/ice_tm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ice/ice_tm.c b/drivers/net/ice/ice_tm.c index fbab0b8808..e10ac855f9 100644 --- a/drivers/net/ice/ice_tm.c +++ b/drivers/net/ice/ice_tm.c @@ -616,7 +616,10 @@ static int ice_set_node_rate(struct ice_hw *hw, ICE_MAX_BW, rate); if (status) { - PMD_DRV_LOG(ERR, "Failed to set max bandwidth for node %u", tm_node->id); + if (tm_node != NULL) + PMD_DRV_LOG(ERR, "Failed to set max bandwidth for node %u", tm_node->id); + else + PMD_DRV_LOG(ERR, "Failed to set max bandwidth"); return -EINVAL; } @@ -630,7 +633,10 @@ static int ice_set_node_rate(struct ice_hw *hw, ICE_MIN_BW, rate); if (status) { - PMD_DRV_LOG(ERR, "Failed to set min bandwidth for node %u", tm_node->id); + if (tm_node != NULL) + PMD_DRV_LOG(ERR, "Failed to set min bandwidth for node %u", tm_node->id); + else + PMD_DRV_LOG(ERR, "Failed to set min bandwidth"); return -EINVAL; } -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net/ice: fix null pointer dereferences 2024-03-01 5:20 [PATCH] net/ice: fix null pointer dereferences Wenwu Ma @ 2024-03-01 10:34 ` Bruce Richardson 2024-03-04 1:43 ` Ma, WenwuX 2024-03-04 5:37 ` [PATCH v2] " Wenwu Ma 1 sibling, 1 reply; 5+ messages in thread From: Bruce Richardson @ 2024-03-01 10:34 UTC (permalink / raw) To: Wenwu Ma; +Cc: dev, songx.jiale On Fri, Mar 01, 2024 at 01:20:29PM +0800, Wenwu Ma wrote: > This patch fixes two null pointer dereferences detected by > coverity scan. > > Coverity issue: 414096 > Fixes: 6ccef90ff5d3 ("net/ice: support VSI level bandwidth config") > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > --- > drivers/net/ice/ice_tm.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ice/ice_tm.c b/drivers/net/ice/ice_tm.c > index fbab0b8808..e10ac855f9 100644 > --- a/drivers/net/ice/ice_tm.c > +++ b/drivers/net/ice/ice_tm.c > @@ -616,7 +616,10 @@ static int ice_set_node_rate(struct ice_hw *hw, > ICE_MAX_BW, > rate); > if (status) { > - PMD_DRV_LOG(ERR, "Failed to set max bandwidth for node %u", tm_node->id); > + if (tm_node != NULL) > + PMD_DRV_LOG(ERR, "Failed to set max bandwidth for node %u", tm_node->id); > + else > + PMD_DRV_LOG(ERR, "Failed to set max bandwidth"); > return -EINVAL; > } > > @@ -630,7 +633,10 @@ static int ice_set_node_rate(struct ice_hw *hw, > ICE_MIN_BW, > rate); > if (status) { > - PMD_DRV_LOG(ERR, "Failed to set min bandwidth for node %u", tm_node->id); > + if (tm_node != NULL) > + PMD_DRV_LOG(ERR, "Failed to set min bandwidth for node %u", tm_node->id); > + else > + PMD_DRV_LOG(ERR, "Failed to set min bandwidth"); > return -EINVAL; > } > Hi Wenwu, I'm not sure that this is the best fix here, since the error message doesn't seem particularly useful without the node id. Looking at the code, this is a static function, so non-public, and only called in three places in rte_tm.c: from ice_cfg_hw_node, ice_do_hierarchy_commit and ice_reset_nolead_nodes. In all three cases, failure of this function is immediately followed by a more specific error message from the calling function. Therefore, I think we can solve the coverity problem by just deleting the error prints from here completely, and let the callers manage error reporting. What do you think? /Bruce ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net/ice: fix null pointer dereferences 2024-03-01 10:34 ` Bruce Richardson @ 2024-03-04 1:43 ` Ma, WenwuX 0 siblings, 0 replies; 5+ messages in thread From: Ma, WenwuX @ 2024-03-04 1:43 UTC (permalink / raw) To: Richardson, Bruce; +Cc: dev, Jiale, SongX > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Friday, March 1, 2024 6:35 PM > To: Ma, WenwuX <wenwux.ma@intel.com> > Cc: dev@dpdk.org; Jiale, SongX <songx.jiale@intel.com> > Subject: Re: [PATCH] net/ice: fix null pointer dereferences > > On Fri, Mar 01, 2024 at 01:20:29PM +0800, Wenwu Ma wrote: > > This patch fixes two null pointer dereferences detected by coverity > > scan. > > > > Coverity issue: 414096 > > Fixes: 6ccef90ff5d3 ("net/ice: support VSI level bandwidth config") > > > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> > > --- > > drivers/net/ice/ice_tm.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ice/ice_tm.c b/drivers/net/ice/ice_tm.c index > > fbab0b8808..e10ac855f9 100644 > > --- a/drivers/net/ice/ice_tm.c > > +++ b/drivers/net/ice/ice_tm.c > > @@ -616,7 +616,10 @@ static int ice_set_node_rate(struct ice_hw *hw, > > ICE_MAX_BW, > > rate); > > if (status) { > > - PMD_DRV_LOG(ERR, "Failed to set max bandwidth for > node %u", tm_node->id); > > + if (tm_node != NULL) > > + PMD_DRV_LOG(ERR, "Failed to set max bandwidth for > node %u", tm_node->id); > > + else > > + PMD_DRV_LOG(ERR, "Failed to set max bandwidth"); > > return -EINVAL; > > } > > > > @@ -630,7 +633,10 @@ static int ice_set_node_rate(struct ice_hw *hw, > > ICE_MIN_BW, > > rate); > > if (status) { > > - PMD_DRV_LOG(ERR, "Failed to set min bandwidth for > node %u", tm_node->id); > > + if (tm_node != NULL) > > + PMD_DRV_LOG(ERR, "Failed to set min bandwidth for > node %u", tm_node->id); > > + else > > + PMD_DRV_LOG(ERR, "Failed to set min bandwidth"); > > return -EINVAL; > > } > > > Hi Wenwu, > > I'm not sure that this is the best fix here, since the error message doesn't seem > particularly useful without the node id. Looking at the code, this is a static > function, so non-public, and only called in three places in > rte_tm.c: from ice_cfg_hw_node, ice_do_hierarchy_commit and > ice_reset_nolead_nodes. In all three cases, failure of this function is > immediately followed by a more specific error message from the calling > function. Therefore, I think we can solve the coverity problem by just deleting > the error prints from here completely, and let the callers manage error > reporting. > > What do you think? > Ok, I will submit a new patch later. > /Bruce ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] net/ice: fix null pointer dereferences 2024-03-01 5:20 [PATCH] net/ice: fix null pointer dereferences Wenwu Ma 2024-03-01 10:34 ` Bruce Richardson @ 2024-03-04 5:37 ` Wenwu Ma 2024-03-04 10:00 ` Bruce Richardson 1 sibling, 1 reply; 5+ messages in thread From: Wenwu Ma @ 2024-03-04 5:37 UTC (permalink / raw) To: dev; +Cc: songx.jiale, Wenwu Ma, stable This patch fixes two null pointer dereferences detected by coverity scan. Coverity issue: 414096 Fixes: 6ccef90ff5d3 ("net/ice: support VSI level bandwidth config") Cc: stable@dpdk.org Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> --- v2: - deleting rather than modifying the prints that cause null pointer dereferences --- drivers/net/ice/ice_tm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/ice/ice_tm.c b/drivers/net/ice/ice_tm.c index fbab0b8808..17f0ca0ce0 100644 --- a/drivers/net/ice/ice_tm.c +++ b/drivers/net/ice/ice_tm.c @@ -615,10 +615,8 @@ static int ice_set_node_rate(struct ice_hw *hw, sched_node, ICE_MAX_BW, rate); - if (status) { - PMD_DRV_LOG(ERR, "Failed to set max bandwidth for node %u", tm_node->id); + if (status) return -EINVAL; - } if (reset || committed == 0) rate = ICE_SCHED_DFLT_BW; @@ -629,10 +627,8 @@ static int ice_set_node_rate(struct ice_hw *hw, sched_node, ICE_MIN_BW, rate); - if (status) { - PMD_DRV_LOG(ERR, "Failed to set min bandwidth for node %u", tm_node->id); + if (status) return -EINVAL; - } return 0; } -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net/ice: fix null pointer dereferences 2024-03-04 5:37 ` [PATCH v2] " Wenwu Ma @ 2024-03-04 10:00 ` Bruce Richardson 0 siblings, 0 replies; 5+ messages in thread From: Bruce Richardson @ 2024-03-04 10:00 UTC (permalink / raw) To: Wenwu Ma; +Cc: dev, songx.jiale, stable On Mon, Mar 04, 2024 at 01:37:51PM +0800, Wenwu Ma wrote: > This patch fixes two null pointer dereferences detected by > coverity scan. > > Coverity issue: 414096 > Fixes: 6ccef90ff5d3 ("net/ice: support VSI level bandwidth config") > Cc: stable@dpdk.org > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com> Applied to dpdk-next-net-intel with expanded commit log message. thanks, /Bruce ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-03-04 10:00 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-01 5:20 [PATCH] net/ice: fix null pointer dereferences Wenwu Ma 2024-03-01 10:34 ` Bruce Richardson 2024-03-04 1:43 ` Ma, WenwuX 2024-03-04 5:37 ` [PATCH v2] " Wenwu Ma 2024-03-04 10:00 ` Bruce Richardson
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).