From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BBF12370AF3 for ; Tue, 23 Jun 2026 10:57:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782212271; cv=none; b=CEHGDraBfskb7OXrYMqrl7dJUS/ixAjK3bwYnspNTf+vfJtJfqYwZ7UvhYehIOtG8owXC6VW3CU7UvsQfH00bU/IXcj8Lfsy3AIt3qVOvMst8xRiuPjFtrGBMiKq8kuL1knTvGjBHszDDjo6Thkb1e2nyENxoEgvtnhSPtQ5jfo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782212271; c=relaxed/simple; bh=w284zWyGlA8zSLYL9YiCV7XpcJPGn/1WTChcWaTE1XY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=uJCKB7jZ9pHBdSsj4Jp6J8JzII8T+hplMhPj2cBDjT4D8m2iiL8mySWmYFRzN8HeaP0bdYA9q8jR7u6JWAWNxUDeQwANrmvXnzMWVI5IUTJbU3fio07URwWrhjqOajBubspIXCBh6I49i9gbXHaATZUWuemAGNy8w6BNkrfb1fc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=ZcoxHJVR; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=Kh4Nlm7H; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="ZcoxHJVR"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="Kh4Nlm7H" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1782212268; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Qdcx8pE/nw7N4Xhb05KzxoHXjXPkYTO5AuxwEAinnqw=; b=ZcoxHJVRCXvL+TWV8te7Lr1mTIemN95s75oQ3zkWHypixduVxVLSeeuuyiI8OpRhfsniQn 88drQX7OtxIfGodhwxe15MzIUdulvoK79nsIh68IY6nQNkVcBaixQFPyIthrcgwoGWz3AH aAflku2J04qsdpbzl1PxFpSZJ4KC/q4= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-498-fLGOUB9NO0a75RtvmDF2GA-1; Tue, 23 Jun 2026 06:57:46 -0400 X-MC-Unique: fLGOUB9NO0a75RtvmDF2GA-1 X-Mimecast-MFC-AGG-ID: fLGOUB9NO0a75RtvmDF2GA_1782212265 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-490a767b782so38050195e9.2 for ; Tue, 23 Jun 2026 03:57:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1782212265; x=1782817065; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=Qdcx8pE/nw7N4Xhb05KzxoHXjXPkYTO5AuxwEAinnqw=; b=Kh4Nlm7Hghx0KI6TRRGzjvzXH5y5SE8H04TGA5r0TdrqCnFnpumK1eD7GZdscVyfQV RCEj9kleREP37KVHq6vKFBTh06eIoY6nLPjeIKRP9UPASok3J3SDgAojutu2MgAfKxmX U6eIMPLWGct+x3WISt87APtgmDpwIvJnAECoJA0BsIS+LkqtxhaiguinyfWdmNsIrdPZ kae6wJRMdqvqsLDUjfSC8Mq1P079MSsTs72ZXJCELrB8nitsUWtKYj4iyDdGhtrUxguT nfMHCPIq+jfKvHEyf5uQdKbqmie+qi/FV/s9CPAK0mKR9UxW4IbSLxYyPUK6h4+7jtUx znwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782212265; x=1782817065; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Qdcx8pE/nw7N4Xhb05KzxoHXjXPkYTO5AuxwEAinnqw=; b=LsZfSH0Zw/VmOt6SSorowVPVYU/oZPeHDLDPzzPfRDBFysBHX7SV3iWCdxtkoIf/Oq 2Bks7g35MBTXE2z6CeX4ZJT8KEFxQO36dr+uCj1DpaXifP8aEM7MrGHgs9A2/iT1P91z xCRFjue9EXCNxcke7vF/aX7q+3fgBw5BC75kFsciv1c/RtoJjDjIhY73NIVHDQHdxJ0l mSek/X/nOurNaWcjjW19Fajn6crSaJhTkMT+rSRNTPzeA+f4eijuV/7iNdv6Sa4Bu1Bq yQ8EJsh0h2NBypA0iSJC5OIldG0VNcIXfh25/b3usc6vffhPFYv8Dwean8CTEvoMlFIJ YkPg== X-Forwarded-Encrypted: i=1; AFNElJ/1FJsnc/rjt+alTbkP3niN+e/hPO4dycDwc9gbbxV2BS6NLJnybhRzHsT0YPj8D570a2iLDH3oUOxMjOk=@vger.kernel.org X-Gm-Message-State: AOJu0YyDpj3FMaUGAb/+60j3/BN5YyWrSxpORQ8by33L3YKo3ixWXvhQ 0RJVMsmt32/aaibhJNTipMgCaswBooBclN9Taati39nIFFdYWKGUSum7nb565yAsXPVEYOY5TAD +1egznBia22sVGhZb+tNex9sX80q5DmAdl4ukGaY6FjGjULf5unyOd4/tYZg854eNWA== X-Gm-Gg: AfdE7cmZLCAIpQH9mXaOJnIG4b6jEsHLxhADbFEo4NPwICpdvFPWeUSBIHWxgeTGj2H 3m49G4wHEQ3dy8LGWV5i9Es7krJJ3s33RhKx1jzBYZWFf9+vH91Xw5LVJ6LLPgtSEgB6hhnQm9F Rl+BOLPltnMlUS3UTHFHdIWkt9V8T3qHQhxhNbQZpvQ/8p7FKk/VBsRATHuu6UdmQiSzt7rU15n tH5+RluGiVN/efUmloN1umxi0Mw/wddENZ0k8LvdV98P+qCMGNtMrXDkPaRjpkxfNkiEp2TeRai UDXIvCp9HA7Vcj+GHJqFNa22Rw2J6miSRr/D3Foh3HYlaBWhw9mAaCqMIpfTimLYgr4CW9Rktw= = X-Received: by 2002:a05:600c:e558:10b0:490:bcc1:4edb with SMTP id 5b1f17b1804b1-49240e95442mr204023885e9.27.1782212265005; Tue, 23 Jun 2026 03:57:45 -0700 (PDT) X-Received: by 2002:a05:600c:e558:10b0:490:bcc1:4edb with SMTP id 5b1f17b1804b1-49240e95442mr204023585e9.27.1782212264580; Tue, 23 Jun 2026 03:57:44 -0700 (PDT) Received: from [10.43.3.161] ([213.175.37.14]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-492494496cdsm260372885e9.10.2026.06.23.03.57.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Jun 2026 03:57:43 -0700 (PDT) Message-ID: <987aa5d0-9ca8-4038-b77f-94adba497e40@redhat.com> Date: Tue, 23 Jun 2026 12:57:42 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH iwl-net] ice: clear the default forwarding VSI rule when releasing a VSI To: Simon Horman Cc: netdev@vger.kernel.org, anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, jacob.e.keller@intel.com, michal.swiatkowski@linux.intel.com, intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org References: <20260622081030.2312129-1-poros@redhat.com> <20260623102934.1072800-1-horms@kernel.org> Content-Language: en-US From: Petr Oros In-Reply-To: <20260623102934.1072800-1-horms@kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 6/23/26 12:29, Simon Horman wrote: > 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. > >> 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?  The dropped lock isn't new here. The whole dflt_vsi API is check-then-act.  ice_vsi_sync_fltr() does the same if (ice_is_vsi_dflt_vsi(vsi))  ice_clear_dflt_vsi(vsi), and this path runs under vf->cfg_lock, the same  domain as the ice_vf_clear_all_promisc_modes() cleanup it restores. There  is at most one DFLT rule per direction, because a second default VSI folds  both into one ICE_FWD_TO_VSI_LIST, which is the leak this fixes, so the  empty l_data match is unambiguous. In that list case removal honors the  handle via ice_rem_update_vsi_list() and drops only the requested VSI. The  unvalidated whole rule branch is only the single VSI case where that VSI is  the sole default, so removing it is intended. An unrelated removal would  require another context to clear this VSI and install a different sole  default in the gap, but those flows are serialized per context with rtnl,  vf->cfg_lock and ICE_CFG_BUSY. Regards, Petr >> if (test_bit(ICE_FLAG_RSS_ENA, pf->flags)) >> ice_rss_clean(vsi);