netdev - Re: [PATCH iwl-next v3 3/8] ice: do not check for zero mac when creating mac filters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f41330d8-f686-4e04-844e-86c37351ad74@intel.com>
Date: Tue, 27 Jan 2026 11:31:19 +0100
From: Jakub Slepecki <jakub.slepecki@...el.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>, <przemyslaw.kitszel@...el.com>,
	<aleksandr.loktionov@...el.com>
CC: <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
	<intel-wired-lan@...ts.osuosl.org>, <michal.swiatkowski@...ux.intel.com>
Subject: Re: [PATCH iwl-next v3 3/8] ice: do not check for zero mac when
 creating mac filters

On 2026-01-27 0:21, Tony Nguyen wrote:
>> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
>> index 0275e2910c6b..04e5d653efce 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
>> @@ -3648,7 +3648,7 @@ int ice_add_mac(struct ice_hw *hw, struct list_head *m_list)
>>           u16 hw_vsi_id;
>>           err = ice_fltr_mac_address(addr, &m_list_itr->fltr_info);
>> -        if (err || is_zero_ether_addr(addr))
> 
> This is introduced in the previous patch; it would be better to remove it in the original patch.

Previous patch moves it from

diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 84848f0123e7..0275e2910c6b 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -3634,17 +3660,19 @@ int ice_add_mac(struct ice_hw *hw, struct list_head *m_list)
  		if (m_list_itr->fltr_info.src_id != ICE_SRC_ID_VSI)
  			return -EINVAL;
  		m_list_itr->fltr_info.src = hw_vsi_id;
-		if (m_list_itr->fltr_info.lkup_type != ICE_SW_LKUP_MAC ||
-		    is_zero_ether_addr(add))
  			return -EINVAL;
...

here, as a call to is_zero_ether_addr(), to the chunk right above, in

@@ -3614,16 +3637,19 @@ bool ice_vlan_fltr_exist(struct ice_hw *hw, u16 vlan_id, u16 vsi_handle)
  int ice_add_mac(struct ice_hw *hw, struct list_head *m_list)
  {
  	struct ice_fltr_list_entry *m_list_itr;
-	int status = 0;
+	int err;
  
  	if (!m_list || !hw)
  		return -EINVAL;
  
  	list_for_each_entry(m_list_itr, m_list, list_entry) {
-		u8 *add = &m_list_itr->fltr_info.l_data.mac.mac_addr[0];
+		u8 addr[ETH_ALEN];
  		u16 vsi_handle;
  		u16 hw_vsi_id;
  
+		err = ice_fltr_mac_address(addr, &m_list_itr->fltr_info);
+		if (err || is_zero_ether_addr(addr))
+			return -EINVAL;
...

here, same call.

The intention of the previous patch is to allow adding mac,vlan filters.
Check is removed separately to make it visible.  Alternative is hiding
it somewhere in two active chunks and in a long commit message.  I think
this fits well into "separate each logical change into a separate patch."

> Also, AI Review says:
> 
> The is_zero_ether_addr(addr) check was removed in line 3651, relying on the claim that ice_fltr_mac_address() performs this validation. However, the helper function only extracts the MAC address and validates the lookup type—it does NOT validate against zero addresses.

Removal is a result of internal discussion about ice_add_mac() and
ice_fltr_mac_address() using zero addresses to mark errors.  Reading
through the comments now does not make me convinced it's the best way.
As long as errors are reported via int returns, I think the zero address
check can act as a sanity check.  AFAIK, all calls that may result in
ice_add_mac() currently are guarded by is_valid_ether_addr().

As for the phrasing in the commit message.  That's my mistake and if the
patch remains, I'll correct this.  This version of this patch should
not say "previously assumed zero-address cases."

I'd prefer to remove this patch in v4.

> Thanks,
> Tony

Thank you!