netdev - RE: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal

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] [day] [month] [year] [list]

Date: Thu, 28 Mar 2024 16:10:52 +0000
From: "Romanowski, Rafal" <rafal.romanowski@...el.com>
To: Brett Creeley <bcreeley@....com>, ivecera <ivecera@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "moderated list:INTEL ETHERNET DRIVERS"
	<intel-wired-lan@...ts.osuosl.org>, open list <linux-kernel@...r.kernel.org>,
	"Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>, Eric Dumazet
	<edumazet@...gle.com>, "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
	"horms@...nel.org" <horms@...nel.org>, Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>
Subject: RE: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of
> Brett Creeley
> Sent: Thursday, March 14, 2024 4:54 PM
> To: ivecera <ivecera@...hat.com>; netdev@...r.kernel.org
> Cc: moderated list:INTEL ETHERNET DRIVERS <intel-wired-
> lan@...ts.osuosl.org>; open list <linux-kernel@...r.kernel.org>; Loktionov,
> Aleksandr <aleksandr.loktionov@...el.com>; Eric Dumazet
> <edumazet@...gle.com>; Nguyen, Anthony L
> <anthony.l.nguyen@...el.com>; horms@...nel.org; Jakub Kicinski
> <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; David S. Miller
> <davem@...emloft.net>
> Subject: Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
> 
> On 3/13/2024 6:56 AM, Ivan Vecera wrote:
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > 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;
> > +               }
> 
> Seems okay to me.
> 
> Reviewed-by: Brett Creeley <brett.creeley@....com>
> 
> >
> >                  if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> >                          ret = -EINVAL;
> > --
> > 2.43.0
> >
> >

Tested-by: Rafal Romanowski <rafal.romanowski@...el.com>