linux-kernel - Re: [PATCH net] hv_netvsc: Fix VF namespace also in netvsc

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: <20241003084838.32c3b03b@hermes.local>
Date: Thu, 3 Oct 2024 08:48:38 -0700
From: Stephen Hemminger <stephen@...workplumber.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Haiyang Zhang <haiyangz@...rosoft.com>, linux-hyperv@...r.kernel.org,
 netdev@...r.kernel.org, kys@...rosoft.com, wei.liu@...nel.org,
 decui@...rosoft.com, edumazet@...gle.com, kuba@...nel.org,
 davem@...emloft.net, linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH net] hv_netvsc: Fix VF namespace also in netvsc_open

On Thu, 3 Oct 2024 11:34:49 +0200
Paolo Abeni <pabeni@...hat.com> wrote:

> On 9/27/24 22:54, Haiyang Zhang wrote:
> > The existing code moves VF to the same namespace as the synthetic device
> > during netvsc_register_vf(). But, if the synthetic device is moved to a
> > new namespace after the VF registration, the VF won't be moved together.
> > 
> > To make the behavior more consistent, add a namespace check to netvsc_open(),
> > and move the VF if it is not in the same namespace.
> > 
> > Cc: stable@...r.kernel.org
> > Fixes: c0a41b887ce6 ("hv_netvsc: move VF to same namespace as netvsc device")
> > Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>  
> 
> This looks strange to me. Skimming over the code it looks like that with 
> VF you really don't mean a Virtual Function...

In Hyper-V/Azure, there is a feature called "Accelerated Networking" where
a Virtual Function (VF) is associated with the synthetic network interface.
The VF may be added/removed by hypervisor while network is running and driver
needs to follow and track that.

> 
> Looking at the blamed commit, it looks like that having both the 
> synthetic and the "VF" device in different namespaces is an intended 
> use-case. This change would make such scenario more difficult and could 
> possibly break existing use-cases.

That commit was trying to solve the case where a network interface
was isolated at boot. The VF device shows up after the
synthetic device has been registered.

> Why do you think it will be more consistent? If the user moved the 
> synthetic device in another netns, possibly/likely the user intended to 
> keep both devices separated.

Splitting the two across namespaces is not useful. The VF is a secondary
device and doing anything directly on the VF will not give good results.
Linux does not have a way to hide or lock out network devices, if it did
the VF would be so marked.

This patch is trying to handle the case where userspace moves
the synthetic network device and the VF is left in wrong namespace.

Moving the device when brought up is not the best solution. Probably better to
do it when the network device is moved to another namespace.
Which is visible in driver as NETDEV_REGISTER event.
The driver already handles this (for VF events) in netvsc_netdev_event()
it would just have to look at events on the netvsc device as well.