> -----Original Message-----
> From: Binbin Deng <[email protected]>
> Sent: Tuesday, October 20, 2026 4:59 AM
> To: Nguyen, Anthony L <[email protected]>; Kitszel, Przemyslaw
> <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; Binbin Deng <[email protected]>
> Subject: [PATCH v1] i40e: Lock VF cloud filter list operations
>
> A VF's cloud-filter hlist is modified by the virtchnl add and delete
> handlers and by i40e_del_all_cloud_filters(), which runs when ADq is
> disabled or when the VF loses trusted status. These paths do not share
> a lock. In particular, the PF's async VF-message handler can process
> VIRTCHNL_OP_DEL_CLOUD_FILTER while host-side ndo_set_vf_trust() removes
> every cloud filter for that VF.
>
> The race is reachable from a guest by issuing cloud-filter virtchnl
> requests while the host revokes VF trust or disables ADq. The VF-
> message handler can traverse nodes that the bulk-delete path has already
> freed, or both paths can remove the same node. The result can be a
> KASAN use-after-free, hlist corruption, incorrect filter accounting, or
> a panic.
> The virtchnl operation-pending bit does not serialize this host-side
> trust path.
>
> Serialize cloud-filter control operations with a mutex. Hold it across
> the firmware add or delete, hlist update, and free, so bulk deletion and
> individual add/delete operations cannot observe a partially removed
> list.
> The lock is file-local and slow-path only.
>
> Found during a static-analysis-assisted audit: candidate sites were
> surfaced by a Coccinelle rule, and the reader/writer paths were
> confirmed by LLM-assisted (GLM-5.3) code inspection. Due to a lack of
> hardware infrastructureThe issue was not observed at runtime.
> It is identified by code inspection only.
>
> Fixes: e284fc280473 ("i40e: Add and delete cloud filter")
> Assisted-by: GLM-5.3 coccinelle
> Signed-off-by: Binbin Deng <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index a26c3d47ec15..f73a710f42f1 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3740,6 +3740,8 @@ static struct i40e_vsi
> *i40e_find_vsi_from_seid(struct i40e_vf *vf, u16 seid)
> *
> * This function deletes all cloud filters
> **/
> +static DEFINE_MUTEX(i40e_vf_cloud_filter_lock);
> +
> static void i40e_del_all_cloud_filters(struct i40e_vf *vf) {
> struct i40e_cloud_filter *cfilter = NULL; @@ -3748,6 +3750,7 @@
> static void i40e_del_all_cloud_filters(struct i40e_vf *vf)
> struct hlist_node *node;
> int ret;
>
> + mutex_lock(&i40e_vf_cloud_filter_lock);
> hlist_for_each_entry_safe(cfilter, node,
> &vf->cloud_filter_list, cloud_node) {
> vsi = i40e_find_vsi_from_seid(vf, cfilter->seid); @@ -
> 3773,6 +3776,7 @@ static void i40e_del_all_cloud_filters(struct i40e_vf
> *vf)
> kfree(cfilter);
> vf->num_cloud_filters--;
> }
> + mutex_unlock(&i40e_vf_cloud_filter_lock);
> }
>
> /**
> @@ -3856,6 +3860,7 @@ static int i40e_vc_del_cloud_filter(struct i40e_vf
> *vf, u8 *msg)
> }
>
> /* get the vsi to which the tc belongs to */
> + mutex_lock(&i40e_vf_cloud_filter_lock);
> vsi = pf->vsi[vf->ch[vcf->action_meta].vsi_idx];
> cfilter.seid = vsi->seid;
> cfilter.flags = vcf->field_flags;
> @@ -3870,6 +3875,7 @@ static int i40e_vc_del_cloud_filter(struct i40e_vf
> *vf, u8 *msg)
> "VF %d: Failed to delete cloud filter, err %pe
> aq_err %s\n",
> vf->vf_id, ERR_PTR(ret),
> libie_aq_str(pf->hw.aq.asq_last_status));
> + mutex_unlock(&i40e_vf_cloud_filter_lock);
> goto err;
> }
>
> @@ -3901,6 +3907,7 @@ static int i40e_vc_del_cloud_filter(struct i40e_vf
> *vf, u8 *msg)
> kfree(cf);
> vf->num_cloud_filters--;
> }
> + mutex_unlock(&i40e_vf_cloud_filter_lock);
>
> err:
> return i40e_vc_send_resp_to_vf(vf, VIRTCHNL_OP_DEL_CLOUD_FILTER,
> @@ -4002,6 +4009,7 @@ static int i40e_vc_add_cloud_filter(struct i40e_vf
> *vf, u8 *msg)
> }
>
> /* get the VSI to which the TC belongs to */
> + mutex_lock(&i40e_vf_cloud_filter_lock);
> vsi = pf->vsi[vf->ch[vcf->action_meta].vsi_idx];
> cfilter->seid = vsi->seid;
> cfilter->flags = vcf->field_flags;
> @@ -4016,6 +4024,7 @@ static int i40e_vc_add_cloud_filter(struct i40e_vf
> *vf, u8 *msg)
> "VF %d: Failed to add cloud filter, err %pe aq_err
> %s\n",
> vf->vf_id, ERR_PTR(aq_ret),
> libie_aq_str(pf->hw.aq.asq_last_status));
> + mutex_unlock(&i40e_vf_cloud_filter_lock);
> goto err_free;
> }
>
> @@ -4024,6 +4033,7 @@ static int i40e_vc_add_cloud_filter(struct i40e_vf
> *vf, u8 *msg)
> /* release the pointer passing it to the collection */
> cfilter = NULL;
> vf->num_cloud_filters++;
> + mutex_unlock(&i40e_vf_cloud_filter_lock);
> err_free:
> kfree(cfilter);
> err_out:
> --
> 2.43.0
I'm afraid one more global lock shared among all cards on server will make
things worse.
@Jakub Kicinski can you comment?
