netdev - RE: [PATCH] hv_netvsc:Register VF in netvsc_probe if NET_DEVICE

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: Tue, 30 Jan 2024 22:30:57 +0000
From: Dexuan Cui <decui@...rosoft.com>
To: Haiyang Zhang <haiyangz@...rosoft.com>, Shradha Gupta
	<shradhagupta@...ux.microsoft.com>, KY Srinivasan <kys@...rosoft.com>, Wei
 Liu <wei.liu@...nel.org>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Wojciech Drewek <wojciech.drewek@...el.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Shradha Gupta <shradhagupta@...rosoft.com>, "stable@...r.kernel.org"
	<stable@...r.kernel.org>
Subject: RE: [PATCH] hv_netvsc:Register VF in netvsc_probe if
 NET_DEVICE_REGISTER missed

> From: Haiyang Zhang <haiyangz@...rosoft.com>
> Sent: Tuesday, January 30, 2024 2:05 PM
> [...]
> > > @@ -2205,8 +2209,11 @@ static int netvsc_vf_join(struct net_device
> > > *vf_netdev,
> > >  			   ndev->name, ret);
> > >  		goto upper_link_failed;
> > >  	}
> > > -
> > > -	schedule_delayed_work(&ndev_ctx->vf_takeover,
> > > VF_TAKEOVER_INT);
> > > +	/* If this registration is called from probe context vf_takeover
> > > +	 * is taken care of later in probe itself.
> > I suspect "later in probe itself" is not accurate.
> > If 'context' is VF_REG_IN_PROBE, I suppose what happens here is:
> > after netvsc_probe() finishes, the netvsc interface becomes available,
> > so the user space will ifup it, and netvsc_open() will UP the VF
> > interface,
> > and netvsc_netdev_event() is called for the VF with event ==
> > NETDEV_POST_INIT (?) and NETDEV_CHANGE, and the data path is
> > switched to the VF.
> 
> In register_netdevice(), NETDEV_POST_INIT is earlier than
> NETDEV_REGISTER.
> This case: netvsc_open >> dev_open(vf) >> NETDEV_UP >>
>  netvsc_vf_changed(event_dev, event);

I see. So there should be no issue here. Thanks for the clarification!

> > If my understanding is correct, I think in the case of 'context' ==
> > VF_REG_IN_PROBE, I suspect the "Align MTU of VF with master"
> > and the "sync address list from ndev to VF" in __netvsc_vf_setup() are
> > omitted? If so, should this be fixed? e.g. Not sure if the below is an
> > issue or not:
> > 1) a VF is bound to a NetVSC interface, and a user sets the MTUs to 1024.
> > 2) rmmod hv_netvsc
> > 3) modprobe hv_netvsc
> > 4) the netvsc interface uses MTU=1500 (the default), and the VF still
> > uses 1024.
> 
> __netvsc_vf_setup() is skipped from the netvsc_register_vf >>
> netvsc_vf_join(),
> but called from netvsc_probe(), so the VF mtu is sync-ed to 1500.
> I verified mtu sync in test.
You're correct. Sorry, I didn't notice that in the patch __netvsc_vf_setup()
now is also called from netvsc_probe().