Re: [PATCH iwl-net] ice: clear the default forwarding VSI rule when releasing a VSI
From: Simon Horman <horms@kernel.org>
Date: 2026-06-23 10:30:01
Also in:
intel-wired-lan, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev --- ice: clear the default forwarding VSI rule when releasing a VSI This patch restores the cleanup of the default forwarding VSI rule in ice_vsi_release(), clearing it when the VSI owns it. This prevents a rule leak if a trusted VF enters promiscuous mode and is then destroyed.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c@@ -2871,6 +2871,9 @@ int ice_vsi_release(struct ice_vsi *vsi) return -ENODEV; pf = vsi->back; + if (ice_is_vsi_dflt_vsi(vsi)) + ice_clear_dflt_vsi(vsi); +
[Severity: High]
Is there a time-of-check to time-of-use race here?
ice_is_vsi_dflt_vsi() acquires and releases the filt_rule_lock to check if
the current VSI is the default. If true, ice_clear_dflt_vsi() is called,
which acquires the lock again during rule removal.
If the lock is dropped between these two calls, could another thread
establish a new default VSI rule before ice_clear_dflt_vsi() runs?
When ice_clear_dflt_vsi() executes, it appears the rule search only matches
by l_data and flag:
ice_switch.c:ice_find_rule_entry() {
if (!memcmp(&f_info->l_data, &list_itr->fltr_info.l_data,
sizeof(f_info->l_data)) &&
f_info->flag == list_itr->fltr_info.flag) {
...
}
For the default VSI rule, l_data is empty, so it might match any newly
established default rule.
And since ice_remove_rule_internal() does not appear to validate the
vsi_handle for non-list rules:
ice_switch.c:ice_remove_rule_internal() {
if (list_elem->fltr_info.fltr_act != ICE_FWD_TO_VSI_LIST) {
remove_rule = true;
...
}
Could this blindly remove the default forwarding configuration for a
completely unrelated VSI?
if (test_bit(ICE_FLAG_RSS_ENA, pf->flags)) ice_rss_clean(vsi);