[PATCH iwl-net] ice: Fix increasing MSI-X on VF

[PATCH iwl-net] ice: Fix increasing MSI-X on VF

[Marcin Szycik]

Increasing MSI-X value on a VF leads to invalid memory operations. This
is caused by not reallocating some arrays.

Reproducer:
  modprobe ice
  echo 0 > /sys/bus/pci/devices/$PF_PCI/sriov_drivers_autoprobe
  echo 1 > /sys/bus/pci/devices/$PF_PCI/sriov_numvfs
  echo 17 > /sys/bus/pci/devices/$VF0_PCI/sriov_vf_msix_count

Default MSI-X is 16, so 17 and above triggers this issue.

KASAN reports:

  BUG: KASAN: slab-out-of-bounds in ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
  Read of size 8 at addr ffff8888b937d180 by task bash/28433
  (...)

  Call Trace:
   (...)
   ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
   kasan_report+0xed/0x120
   ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
   ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
   ice_vsi_cfg_def+0x3360/0x4770 [ice]
   ? mutex_unlock+0x83/0xd0
   ? __pfx_ice_vsi_cfg_def+0x10/0x10 [ice]
   ? __pfx_ice_remove_vsi_lkup_fltr+0x10/0x10 [ice]
   ice_vsi_cfg+0x7f/0x3b0 [ice]
   ice_vf_reconfig_vsi+0x114/0x210 [ice]
   ice_sriov_set_msix_vec_count+0x3d0/0x960 [ice]
   sriov_vf_msix_count_store+0x21c/0x300
   (...)

  Allocated by task 28201:
   (...)
   ice_vsi_cfg_def+0x1c8e/0x4770 [ice]
   ice_vsi_cfg+0x7f/0x3b0 [ice]
   ice_vsi_setup+0x179/0xa30 [ice]
   ice_sriov_configure+0xcaa/0x1520 [ice]
   sriov_numvfs_store+0x212/0x390
   (...)

To fix it, use ice_vsi_rebuild() instead of ice_vf_reconfig_vsi(). This
causes the required arrays to be reallocated taking the new queue count
into account (ice_vsi_realloc_stat_arrays()). Set req_txq and req_rxq
before ice_vsi_rebuild(), so that realloc uses the newly set queue
count.

Additionally, ice_vsi_rebuild() does not remove VSI filters
(ice_fltr_remove_all()), so ice_vf_init_host_cfg() is no longer
necessary.

Reported-by: Jacob Keller <jacob.e.keller@intel.com>
Fixes: 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with ice_vf_reconfig_vsi()")
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
---
 drivers/net/ethernet/intel/ice/ice_sriov.c          | 11 ++++++++---
 drivers/net/ethernet/intel/ice/ice_vf_lib.c         |  2 +-
 drivers/net/ethernet/intel/ice/ice_vf_lib_private.h |  1 -
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index c2d6b2a144e9..91cb393f616f 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -1121,7 +1121,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count)
 	if (vf->first_vector_idx < 0)
 		goto unroll;
 
-	if (ice_vf_reconfig_vsi(vf) || ice_vf_init_host_cfg(vf, vsi)) {
+	vsi->req_txq = queues;
+	vsi->req_rxq = queues;
+
+	if (ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT)) {
 		/* Try to rebuild with previous values */
 		needs_rebuild = true;
 		goto unroll;
@@ -1150,8 +1153,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count)
 	}
 
 	if (needs_rebuild) {
-		ice_vf_reconfig_vsi(vf);
-		ice_vf_init_host_cfg(vf, vsi);
+		vsi->req_txq = prev_queues;
+		vsi->req_rxq = prev_queues;
+
+		ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT);
 	}
 
 	ice_ena_vf_mappings(vf);
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 749a08ccf267..8c434689e3f7 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -256,7 +256,7 @@ static void ice_vf_pre_vsi_rebuild(struct ice_vf *vf)
  *
  * It brings the VSI down and then reconfigures it with the hardware.
  */
-int ice_vf_reconfig_vsi(struct ice_vf *vf)
+static int ice_vf_reconfig_vsi(struct ice_vf *vf)
 {
 	struct ice_vsi *vsi = ice_get_vf_vsi(vf);
 	struct ice_pf *pf = vf->pf;
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
index 91ba7fe0eaee..0c7e77c0a09f 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
@@ -23,7 +23,6 @@
 #warning "Only include ice_vf_lib_private.h in CONFIG_PCI_IOV virtualization files"
 #endif
 
-int ice_vf_reconfig_vsi(struct ice_vf *vf);
 void ice_initialize_vf_entry(struct ice_vf *vf);
 void ice_dis_vf_qs(struct ice_vf *vf);
 int ice_check_vf_init(struct ice_vf *vf);
-- 
2.45.0

Re: [PATCH iwl-net] ice: Fix increasing MSI-X on VF

[Simon Horman]

On Fri, Sep 27, 2024 at 05:15:40PM +0200, Marcin Szycik wrote:
> Increasing MSI-X value on a VF leads to invalid memory operations. This
> is caused by not reallocating some arrays.
> 
> Reproducer:
>   modprobe ice
>   echo 0 > /sys/bus/pci/devices/$PF_PCI/sriov_drivers_autoprobe
>   echo 1 > /sys/bus/pci/devices/$PF_PCI/sriov_numvfs
>   echo 17 > /sys/bus/pci/devices/$VF0_PCI/sriov_vf_msix_count
> 
> Default MSI-X is 16, so 17 and above triggers this issue.
> 
> KASAN reports:
> 
>   BUG: KASAN: slab-out-of-bounds in ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
>   Read of size 8 at addr ffff8888b937d180 by task bash/28433
>   (...)
> 
>   Call Trace:
>    (...)
>    ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
>    kasan_report+0xed/0x120
>    ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
>    ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
>    ice_vsi_cfg_def+0x3360/0x4770 [ice]
>    ? mutex_unlock+0x83/0xd0
>    ? __pfx_ice_vsi_cfg_def+0x10/0x10 [ice]
>    ? __pfx_ice_remove_vsi_lkup_fltr+0x10/0x10 [ice]
>    ice_vsi_cfg+0x7f/0x3b0 [ice]
>    ice_vf_reconfig_vsi+0x114/0x210 [ice]
>    ice_sriov_set_msix_vec_count+0x3d0/0x960 [ice]
>    sriov_vf_msix_count_store+0x21c/0x300
>    (...)
> 
>   Allocated by task 28201:
>    (...)
>    ice_vsi_cfg_def+0x1c8e/0x4770 [ice]
>    ice_vsi_cfg+0x7f/0x3b0 [ice]
>    ice_vsi_setup+0x179/0xa30 [ice]
>    ice_sriov_configure+0xcaa/0x1520 [ice]
>    sriov_numvfs_store+0x212/0x390
>    (...)
> 
> To fix it, use ice_vsi_rebuild() instead of ice_vf_reconfig_vsi(). This
> causes the required arrays to be reallocated taking the new queue count
> into account (ice_vsi_realloc_stat_arrays()). Set req_txq and req_rxq
> before ice_vsi_rebuild(), so that realloc uses the newly set queue
> count.
> 
> Additionally, ice_vsi_rebuild() does not remove VSI filters
> (ice_fltr_remove_all()), so ice_vf_init_host_cfg() is no longer
> necessary.
> 
> Reported-by: Jacob Keller <jacob.e.keller@intel.com>
> Fixes: 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with ice_vf_reconfig_vsi()")
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

RE: [Intel-wired-lan] [PATCH iwl-net] ice: Fix increasing MSI-X on VF

[Romanowski, Rafal]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Simon
> Horman
> Sent: Monday, September 30, 2024 4:52 PM
> To: Marcin Szycik <marcin.szycik@linux.intel.com>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org; Michal
> Swiatkowski <michal.swiatkowski@linux.intel.com>; intel-wired-
> lan@lists.osuosl.org; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-net] ice: Fix increasing MSI-X on VF
> 
> On Fri, Sep 27, 2024 at 05:15:40PM +0200, Marcin Szycik wrote:
> > Increasing MSI-X value on a VF leads to invalid memory operations.
> > This is caused by not reallocating some arrays.
> >
> > Reproducer:
> >   modprobe ice
> >   echo 0 > /sys/bus/pci/devices/$PF_PCI/sriov_drivers_autoprobe
> >   echo 1 > /sys/bus/pci/devices/$PF_PCI/sriov_numvfs
> >   echo 17 > /sys/bus/pci/devices/$VF0_PCI/sriov_vf_msix_count
> >
> > Default MSI-X is 16, so 17 and above triggers this issue.
> >
> > KASAN reports:
> >
> >   BUG: KASAN: slab-out-of-bounds in ice_vsi_alloc_ring_stats+0x38d/0x4b0
> [ice]
> >   Read of size 8 at addr ffff8888b937d180 by task bash/28433
> >   (...)
> >
> >   Call Trace:
> >    (...)
> >    ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
> >    kasan_report+0xed/0x120
> >    ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
> >    ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice]
> >    ice_vsi_cfg_def+0x3360/0x4770 [ice]
> >    ? mutex_unlock+0x83/0xd0
> >    ? __pfx_ice_vsi_cfg_def+0x10/0x10 [ice]
> >    ? __pfx_ice_remove_vsi_lkup_fltr+0x10/0x10 [ice]
> >    ice_vsi_cfg+0x7f/0x3b0 [ice]
> >    ice_vf_reconfig_vsi+0x114/0x210 [ice]
> >    ice_sriov_set_msix_vec_count+0x3d0/0x960 [ice]
> >    sriov_vf_msix_count_store+0x21c/0x300
> >    (...)
> >
> >   Allocated by task 28201:
> >    (...)
> >    ice_vsi_cfg_def+0x1c8e/0x4770 [ice]
> >    ice_vsi_cfg+0x7f/0x3b0 [ice]
> >    ice_vsi_setup+0x179/0xa30 [ice]
> >    ice_sriov_configure+0xcaa/0x1520 [ice]
> >    sriov_numvfs_store+0x212/0x390
> >    (...)
> >
> > To fix it, use ice_vsi_rebuild() instead of ice_vf_reconfig_vsi().
> > This causes the required arrays to be reallocated taking the new queue
> > count into account (ice_vsi_realloc_stat_arrays()). Set req_txq and
> > req_rxq before ice_vsi_rebuild(), so that realloc uses the newly set
> > queue count.
> >
> > Additionally, ice_vsi_rebuild() does not remove VSI filters
> > (ice_fltr_remove_all()), so ice_vf_init_host_cfg() is no longer
> > necessary.
> >
> > Reported-by: Jacob Keller <jacob.e.keller@intel.com>
> > Fixes: 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with
> > ice_vf_reconfig_vsi()")
> > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>