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: <DM5PR2101MB101355087924D6C8442BDA4ACAFF0@DM5PR2101MB1013.namprd21.prod.outlook.com>
Date:   Wed, 17 Oct 2018 14:15:30 +0000
From:   Haiyang Zhang <haiyangz@...rosoft.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "David S. Miller" <davem@...emloft.net>,
        Sasha Levin <Alexander.Levin@...rosoft.com>
Subject: RE: [PATCH 4.18 101/135] hv_netvsc: pair VF based on serial number



> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Sent: Wednesday, October 17, 2018 9:17 AM
> To: Haiyang Zhang <haiyangz@...rosoft.com>
> Cc: linux-kernel@...r.kernel.org; stable@...r.kernel.org; Stephen Hemminger
> <sthemmin@...rosoft.com>; David S. Miller <davem@...emloft.net>; Sasha
> Levin <Alexander.Levin@...rosoft.com>
> Subject: Re: [PATCH 4.18 101/135] hv_netvsc: pair VF based on serial number
> 
> On Wed, Oct 17, 2018 at 01:02:17PM +0000, Haiyang Zhang wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > Sent: Wednesday, October 17, 2018 3:46 AM
> > > To: Haiyang Zhang <haiyangz@...rosoft.com>
> > > Cc: linux-kernel@...r.kernel.org; stable@...r.kernel.org; Stephen
> > > Hemminger <sthemmin@...rosoft.com>; David S. Miller
> > > <davem@...emloft.net>; Sasha Levin <Alexander.Levin@...rosoft.com>
> > > Subject: Re: [PATCH 4.18 101/135] hv_netvsc: pair VF based on serial
> > > number
> > >
> > > On Tue, Oct 16, 2018 at 07:13:12PM +0000, Haiyang Zhang wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: linux-kernel-owner@...r.kernel.org <linux-kernel-
> > > > > owner@...r.kernel.org> On Behalf Of Greg Kroah-Hartman
> > > > > Sent: Tuesday, October 16, 2018 1:06 PM
> > > > > To: linux-kernel@...r.kernel.org
> > > > > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>;
> > > > > stable@...r.kernel.org; Stephen Hemminger
> > > > > <sthemmin@...rosoft.com>; David S. Miller <davem@...emloft.net>;
> > > > > Sasha Levin <Alexander.Levin@...rosoft.com>
> > > > > Subject: [PATCH 4.18 101/135] hv_netvsc: pair VF based on serial
> > > > > number
> > > > >
> > > > > 4.18-stable review patch.  If anyone has any objections, please let me
> know.
> > > > >
> > > > > ------------------
> > > > >
> > > > > From: Stephen Hemminger <stephen@...workplumber.org>
> > > > >
> > > > > [ Upstream commit 00d7ddba1143623b31bc2c15d18216e2da031b14 ]
> > > > >
> > > > > Matching network device based on MAC address is problematic
> > > > > since a non VF network device can be creted with a duplicate MAC
> > > > > address causing confusion and problems.  The VMBus API does
> > > > > provide a serial number that is a better matching method.
> > > > >
> > > > > Signed-off-by: Stephen Hemminger <sthemmin@...rosoft.com>
> > > > > Signed-off-by: David S. Miller <davem@...emloft.net>
> > > > > Signed-off-by: Sasha Levin <alexander.levin@...rosoft.com>
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > > > ---
> > > > >  drivers/net/hyperv/netvsc.c     |    3 ++
> > > > >  drivers/net/hyperv/netvsc_drv.c |   58 ++++++++++++++++++++++-------
> -----
> > > -----
> > > > > -
> > > > >  2 files changed, 36 insertions(+), 25 deletions(-)
> > > > >
> > > > > --- a/drivers/net/hyperv/netvsc.c
> > > > > +++ b/drivers/net/hyperv/netvsc.c
> > > > > @@ -1203,6 +1203,9 @@ static void netvsc_send_vf(struct net_de
> > > > >
> > > > >  	net_device_ctx->vf_alloc = nvmsg-
> >msg.v4_msg.vf_assoc.allocated;
> > > > >  	net_device_ctx->vf_serial = nvmsg-
> >msg.v4_msg.vf_assoc.serial;
> > > > > +	netdev_info(ndev, "VF slot %u %s\n",
> > > > > +		    net_device_ctx->vf_serial,
> > > > > +		    net_device_ctx->vf_alloc ? "added" : "removed");
> > > > >  }
> > > > >
> > > > >  static  void netvsc_receive_inband(struct net_device *ndev,
> > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > @@ -1794,20 +1794,6 @@ out_unlock:
> > > > >  	rtnl_unlock();
> > > > >  }
> > > > >
> > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac) -{
> > > > > -	struct net_device_context *ndev_ctx;
> > > > > -
> > > > > -	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
> > > > > -		struct net_device *dev = hv_get_drvdata(ndev_ctx-
> > > > > >device_ctx);
> > > > > -
> > > > > -		if (ether_addr_equal(mac, dev->perm_addr))
> > > > > -			return dev;
> > > > > -	}
> > > > > -
> > > > > -	return NULL;
> > > > > -}
> > > > > -
> > > > >  static struct net_device *get_netvsc_byref(struct net_device
> *vf_netdev)  {
> > > > >  	struct net_device_context *net_device_ctx; @@ -1936,26
> > > > > +1922,48
> > > @@
> > > > > static void netvsc_vf_setup(struct work_
> > > > >  	rtnl_unlock();
> > > > >  }
> > > > >
> > > > > +/* Find netvsc by VMBus serial number.
> > > > > + * The PCI hyperv controller records the serial number as the slot.
> > > > > + */
> > > > > +static struct net_device *get_netvsc_byslot(const struct
> > > > > +net_device
> > > > > +*vf_netdev) {
> > > > > +	struct device *parent = vf_netdev->dev.parent;
> > > > > +	struct net_device_context *ndev_ctx;
> > > > > +	struct pci_dev *pdev;
> > > > > +
> > > > > +	if (!parent || !dev_is_pci(parent))
> > > > > +		return NULL; /* not a PCI device */
> > > > > +
> > > > > +	pdev = to_pci_dev(parent);
> > > > > +	if (!pdev->slot) {
> > > > > +		netdev_notice(vf_netdev, "no PCI slot information\n");
> > > > > +		return NULL;
> > > > > +	}
> > > > > +
> > > > > +	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
> > > > > +		if (!ndev_ctx->vf_alloc)
> > > > > +			continue;
> > > > > +
> > > > > +		if (ndev_ctx->vf_serial == pdev->slot->number)
> > > > > +			return hv_get_drvdata(ndev_ctx->device_ctx);
> > > >
> > > > Greg,
> > > >
> > > > I had a fix to this patch. If not already, Could you include my
> > > > following fix
> > > together?
> > > > hv_netvsc: fix vf serial matching with pci slot info
> > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> > > > it.k
> > > > ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fdavem%2Fnet-
> > > next.git%2F
> > > >
> > >
> commit%2F%3Fid%3D005479556197f80139771960dda0dfdcd2d2aad5&amp;da
> > > ta=02%
> > > >
> > >
> 7C01%7Chaiyangz%40microsoft.com%7C6ce194a47449435fe82a08d63404a94e
> > > %7C7
> > > >
> > >
> 2f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753591959679786&amp
> > > ;sdata
> > > >
> > >
> =ReWYMZW%2FYtoFSbFSxny9VGw6V8v9J4sGanfJjbVd34A%3D&amp;reserved=
> > > 0
> > >
> > > I can not take a patch that is not "upstream" yet, sorry.
> > >
> > > Also, this really feels like a "new feature" to me.  Can someone
> > > explain what bug this is fixing?
> >
> > Patch #A: hv_netvsc: pair VF based on serial number
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> > ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-
> next.git%2
> >
> Fcommit%2F%3Fid%3D00d7ddba1143623b31bc2c15d18216e2da031b14&amp;
> data=02
> > %7C01%7Chaiyangz%40microsoft.com%7C2e07aa3f8a9d4f32526008d63432d
> f14%7C
> >
> 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753790439794786&am
> p;sdat
> > a=0ZyQ7jJdgsEZbh8TqA0eypvPUJVQaxSJcSNYuhLSl%2Fc%3D&amp;reserved=0
> >
> > Patch #B: hv_netvsc: fix vf serial matching with pci slot info
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> > ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-
> next.git%2
> >
> Fcommit%2F%3Fid%3D005479556197f80139771960dda0dfdcd2d2aad5&amp;d
> ata=02
> > %7C01%7Chaiyangz%40microsoft.com%7C2e07aa3f8a9d4f32526008d63432d
> f14%7C
> >
> 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753790439794786&am
> p;sdat
> >
> a=JPfFlatrkt1K6zMLHmZDRG51cCLwVk9pXqHVtLm26hA%3D&amp;reserved=0
> >
> > The patch #A is a feature that use VF serial number for matching. But #A has
> a bug causing mismatch.
> >
> > Patch #B, upstream now (see the link to Linux-next), is a fix to #A.
> 
> You didn't answer my original question here, what bug is this fixing?

Patch #A: hv_netvsc: pair VF based on serial number
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=00d7ddba1143623b31bc2c15d18216e2da031b14

Patch #B: hv_netvsc: fix vf serial matching with pci slot info
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=005479556197f80139771960dda0dfdcd2d2aad5

The VF NIC needs to be paired with synthetic NIC on HyperV  --  to do that we 
use MAC address matching before the patch #A. But a non VF NIC can also 
have the same MAC, which shouldn't be paired with synthetic NIC. So a better 
method is implemented by #A to use VF serial number for matching.

But, #A has a bug, which causes matching to fail. Patch #B fixed it by extracting 
the VF serial number correctly from slot info.

Thanks,
- Haiyang



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ