On 10/29/2026 1:01 AM, Loktionov, Aleksandr wrote: > > >> -----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?
Agreed. Even if the race is real (it hasn't been confirmed on actual hardware), this is not the right fix. The driver should maintain a lock on the relevant data structure and this is clearly a hack of a fix. I'm going to reject this from IWL patchwork. Can someone from the FNIC team make a note about this potential issue so we can investigate and develop a proper fix? Thanks, Jake
