RE: [PATCH v3] net/ice: fix crash on closing representor ports

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Wednesday, November 1, 2023 6:14 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: [PATCH v3] net/ice: fix crash on closing representor ports
> 
> The data resource in struct rte_eth_dev is cleared and points to NULL when
> the DCF port is closed.
> 
> If the DCF representor port is closed after the DCF port is closed, a
> segmentation fault occurs because the representor port accesses the data
> resource released by the DCF port.
> 
> This patch checks if the resource is present before accessing.
> 
> Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
> Fixes: da9cdcd1f372 ("net/ice: fix crash on representor port closing")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> v3: New solution.
> ---
>  drivers/net/ice/ice_dcf_vf_representor.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_dcf_vf_representor.c
> b/drivers/net/ice/ice_dcf_vf_representor.c
> index b9fcfc80ad..8c45e28f02 100644
> --- a/drivers/net/ice/ice_dcf_vf_representor.c
> +++ b/drivers/net/ice/ice_dcf_vf_representor.c
> @@ -111,14 +111,16 @@ ice_dcf_vf_repr_link_update(__rte_unused struct
> rte_eth_dev *ethdev,  static __rte_always_inline struct ice_dcf_hw *
> ice_dcf_vf_repr_hw(struct ice_dcf_vf_repr *repr)  {
> -	struct ice_dcf_adapter *dcf_adapter =
> -			repr->dcf_eth_dev->data->dev_private;
> +	struct rte_eth_dev_data *dcf_data = repr->dcf_eth_dev->data;

Seems this expose another issue, if dcf port already be closed, the dcf_eth_dev instance could already be reused by another driver.
So we can't assume dcf_eth_dev->data is NULL,  I think you can refine based on v2's method, but don't update dcf_valid flag in representor port's dev_stop.



> +	struct ice_dcf_adapter *dcf_adapter;
> 
> -	if (!dcf_adapter) {
> +	if (!dcf_data || !dcf_data->dev_private) {
>  		PMD_DRV_LOG(ERR, "DCF for VF representor has been
> released\n");
>  		return NULL;
>  	}
> 
> +	dcf_adapter = dcf_data->dev_private;
> +
>  	return &dcf_adapter->real_hw;
>  }
> 
> --
> 2.25.1