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]
Date: Wed, 27 Mar 2024 08:29:35 +0100
From: Ivan Vecera <ivecera@...hat.com>
To: netdev@...r.kernel.org, Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: aleksandr.loktionov@...el.com, mschmidt@...hat.com, horms@...nel.org,
 Jesse Brandeburg <jesse.brandeburg@...el.com>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 "moderated list:INTEL ETHERNET DRIVERS" <intel-wired-lan@...ts.osuosl.org>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] i40e: Fix VF MAC filter removal

On 13. 03. 24 14:56, Ivan Vecera wrote:
> Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> administratively set MAC") fixed an issue where untrusted VF was
> allowed to remove its own MAC address although this was assigned
> administratively from PF. Unfortunately the introduced check
> is wrong because it causes that MAC filters for other MAC addresses
> including multi-cast ones are not removed.
> 
> <snip>
> 	if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> 	    i40e_can_vf_change_mac(vf))
> 		was_unimac_deleted = true;
> 	else
> 		continue;
> 
> 	if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> 	...
> </snip>
> 
> The else path with `continue` effectively skips any MAC filter
> removal except one for primary MAC addr when VF is allowed to do so.
> Fix the check condition so the `continue` is only done for primary
> MAC address.
> 
> Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
> Signed-off-by: Ivan Vecera <ivecera@...hat.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index b34c71770887..10267a300770 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>   		/* Allow to delete VF primary MAC only if it was not set
>   		 * administratively by PF or if VF is trusted.
>   		 */
> -		if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> -		    i40e_can_vf_change_mac(vf))
> -			was_unimac_deleted = true;
> -		else
> -			continue;
> +		if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> +			if (i40e_can_vf_change_mac(vf))
> +				was_unimac_deleted = true;
> +			else
> +				continue;
> +		}
>   
>   		if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>   			ret = -EINVAL;

Hi Tony,
the fix is not part of your recent pull series for i40e... I have 
submitted it to 'net' instead of 'iwl-net' as it fixes recent commit 
that causes MAC filter resource leaks that should be fixed as soon as 
possible. But its status in patchwork is 'Awaiting upstream' so it has 
to be resubmitted by yourself... Or should this be picked directly by 
netdev maintainers?

Thanks,
Ivan


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ