All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH intel-net] ice: fix NULL pointer dereference in ice_update_vsi_tx_ring_stats()
@ 2022-03-07 17:47 ` Maciej Fijalkowski
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej Fijalkowski @ 2022-03-07 17:47 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, bpf, anthony.l.nguyen, kuba, davem, magnus.karlsson,
	alexandr.lobakin, dan.carpenter

It is possible to do NULL pointer dereference in routine that updates
Tx ring stats. Currently only stats and bytes are updated when ring
pointer is valid, but later on ring is accessed to propagate gathered Tx
stats onto VSI stats.

Change the existing logic to move to next ring when ring is NULL.

Fixes: e72bba21355d ("ice: split ice_ring onto Tx/Rx separate structs")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 289e5c99e313..d3f8b6468b92 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -6145,8 +6145,9 @@ ice_update_vsi_tx_ring_stats(struct ice_vsi *vsi,
 		u64 pkts = 0, bytes = 0;
 
 		ring = READ_ONCE(rings[i]);
-		if (ring)
-			ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);
+		if (!ring)
+			continue;
+		ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);
 		vsi_stats->tx_packets += pkts;
 		vsi_stats->tx_bytes += bytes;
 		vsi->tx_restart += ring->tx_stats.restart_q;
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Intel-wired-lan] [PATCH intel-net] ice: fix NULL pointer dereference in ice_update_vsi_tx_ring_stats()
@ 2022-03-07 17:47 ` Maciej Fijalkowski
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej Fijalkowski @ 2022-03-07 17:47 UTC (permalink / raw)
  To: intel-wired-lan

It is possible to do NULL pointer dereference in routine that updates
Tx ring stats. Currently only stats and bytes are updated when ring
pointer is valid, but later on ring is accessed to propagate gathered Tx
stats onto VSI stats.

Change the existing logic to move to next ring when ring is NULL.

Fixes: e72bba21355d ("ice: split ice_ring onto Tx/Rx separate structs")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 289e5c99e313..d3f8b6468b92 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -6145,8 +6145,9 @@ ice_update_vsi_tx_ring_stats(struct ice_vsi *vsi,
 		u64 pkts = 0, bytes = 0;
 
 		ring = READ_ONCE(rings[i]);
-		if (ring)
-			ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);
+		if (!ring)
+			continue;
+		ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);
 		vsi_stats->tx_packets += pkts;
 		vsi_stats->tx_bytes += bytes;
 		vsi->tx_restart += ring->tx_stats.restart_q;
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH intel-net] ice: fix NULL pointer dereference in ice_update_vsi_tx_ring_stats()
  2022-03-07 17:47 ` [Intel-wired-lan] " Maciej Fijalkowski
@ 2022-03-08 12:51   ` Alexander Lobakin
  -1 siblings, 0 replies; 6+ messages in thread
From: Alexander Lobakin @ 2022-03-08 12:51 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: Alexander Lobakin, intel-wired-lan, netdev, bpf,
	anthony.l.nguyen, kuba, davem, magnus.karlsson, dan.carpenter

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Mon, 7 Mar 2022 18:47:39 +0100

> It is possible to do NULL pointer dereference in routine that updates
> Tx ring stats. Currently only stats and bytes are updated when ring
> pointer is valid, but later on ring is accessed to propagate gathered Tx
> stats onto VSI stats.
> 
> Change the existing logic to move to next ring when ring is NULL.
> 
> Fixes: e72bba21355d ("ice: split ice_ring onto Tx/Rx separate structs")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 289e5c99e313..d3f8b6468b92 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -6145,8 +6145,9 @@ ice_update_vsi_tx_ring_stats(struct ice_vsi *vsi,
>  		u64 pkts = 0, bytes = 0;
>  
>  		ring = READ_ONCE(rings[i]);
> -		if (ring)
> -			ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);
> +		if (!ring)
> +			continue;
> +		ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);

Nice catch, thanks!

Acked-by: Alexander Lobakin <alexandr.lobakin@intel.com>

>  		vsi_stats->tx_packets += pkts;
>  		vsi_stats->tx_bytes += bytes;
>  		vsi->tx_restart += ring->tx_stats.restart_q;

                                   ^^^^
                                   lol

> -- 
> 2.33.1

Al

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Intel-wired-lan] [PATCH intel-net] ice: fix NULL pointer dereference in ice_update_vsi_tx_ring_stats()
@ 2022-03-08 12:51   ` Alexander Lobakin
  0 siblings, 0 replies; 6+ messages in thread
From: Alexander Lobakin @ 2022-03-08 12:51 UTC (permalink / raw)
  To: intel-wired-lan

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Mon, 7 Mar 2022 18:47:39 +0100

> It is possible to do NULL pointer dereference in routine that updates
> Tx ring stats. Currently only stats and bytes are updated when ring
> pointer is valid, but later on ring is accessed to propagate gathered Tx
> stats onto VSI stats.
> 
> Change the existing logic to move to next ring when ring is NULL.
> 
> Fixes: e72bba21355d ("ice: split ice_ring onto Tx/Rx separate structs")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 289e5c99e313..d3f8b6468b92 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -6145,8 +6145,9 @@ ice_update_vsi_tx_ring_stats(struct ice_vsi *vsi,
>  		u64 pkts = 0, bytes = 0;
>  
>  		ring = READ_ONCE(rings[i]);
> -		if (ring)
> -			ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);
> +		if (!ring)
> +			continue;
> +		ice_fetch_u64_stats_per_ring(&ring->syncp, ring->stats, &pkts, &bytes);

Nice catch, thanks!

Acked-by: Alexander Lobakin <alexandr.lobakin@intel.com>

>  		vsi_stats->tx_packets += pkts;
>  		vsi_stats->tx_bytes += bytes;
>  		vsi->tx_restart += ring->tx_stats.restart_q;

                                   ^^^^
                                   lol

> -- 
> 2.33.1

Al

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [Intel-wired-lan] [PATCH intel-net] ice: fix NULL pointer dereference in ice_update_vsi_tx_ring_stats()
  2022-03-07 17:47 ` [Intel-wired-lan] " Maciej Fijalkowski
@ 2022-03-14  4:46   ` G, GurucharanX
  -1 siblings, 0 replies; 6+ messages in thread
From: G, GurucharanX @ 2022-03-14  4:46 UTC (permalink / raw)
  To: Fijalkowski, Maciej, intel-wired-lan
  Cc: netdev, dan.carpenter, kuba, bpf, davem, Karlsson, Magnus



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Maciej Fijalkowski
> Sent: Monday, March 7, 2022 11:18 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; dan.carpenter@oracle.com; kuba@kernel.org;
> bpf@vger.kernel.org; davem@davemloft.net; Karlsson, Magnus
> <magnus.karlsson@intel.com>
> Subject: [Intel-wired-lan] [PATCH intel-net] ice: fix NULL pointer dereference
> in ice_update_vsi_tx_ring_stats()
> 
> It is possible to do NULL pointer dereference in routine that updates Tx ring
> stats. Currently only stats and bytes are updated when ring pointer is valid,
> but later on ring is accessed to propagate gathered Tx stats onto VSI stats.
> 
> Change the existing logic to move to next ring when ring is NULL.
> 
> Fixes: e72bba21355d ("ice: split ice_ring onto Tx/Rx separate structs")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Intel-wired-lan] [PATCH intel-net] ice: fix NULL pointer dereference in ice_update_vsi_tx_ring_stats()
@ 2022-03-14  4:46   ` G, GurucharanX
  0 siblings, 0 replies; 6+ messages in thread
From: G, GurucharanX @ 2022-03-14  4:46 UTC (permalink / raw)
  To: intel-wired-lan



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Maciej Fijalkowski
> Sent: Monday, March 7, 2022 11:18 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; dan.carpenter at oracle.com; kuba at kernel.org;
> bpf at vger.kernel.org; davem at davemloft.net; Karlsson, Magnus
> <magnus.karlsson@intel.com>
> Subject: [Intel-wired-lan] [PATCH intel-net] ice: fix NULL pointer dereference
> in ice_update_vsi_tx_ring_stats()
> 
> It is possible to do NULL pointer dereference in routine that updates Tx ring
> stats. Currently only stats and bytes are updated when ring pointer is valid,
> but later on ring is accessed to propagate gathered Tx stats onto VSI stats.
> 
> Change the existing logic to move to next ring when ring is NULL.
> 
> Fixes: e72bba21355d ("ice: split ice_ring onto Tx/Rx separate structs")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-03-14  4:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 17:47 [PATCH intel-net] ice: fix NULL pointer dereference in ice_update_vsi_tx_ring_stats() Maciej Fijalkowski
2022-03-07 17:47 ` [Intel-wired-lan] " Maciej Fijalkowski
2022-03-08 12:51 ` Alexander Lobakin
2022-03-08 12:51   ` [Intel-wired-lan] " Alexander Lobakin
2022-03-14  4:46 ` G, GurucharanX
2022-03-14  4:46   ` G, GurucharanX

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.