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:   Thu, 8 Dec 2016 16:56:04 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     kys@...rosoft.com
Cc:     linux-kernel@...r.kernel.org, devel@...uxdriverproject.org,
        olaf@...fle.de, apw@...onical.com, vkuznets@...hat.com,
        jasowang@...hat.com, leann.ogasawara@...onical.com,
        bjorn.helgaas@...il.com, Haiyang Zhang <haiyangz@...rosoft.com>
Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
 numbers

On Thu, Dec 08, 2016 at 12:33:43AM -0800, kys@...hange.microsoft.com wrote:
> From: Haiyang Zhang <haiyangz@...rosoft.com>
> 
> We currently use MAC address to match VF and synthetic NICs. Hyper-V
> provides a serial number to both devices for this purpose. This patch
> implements the matching based on VF serial numbers. This is the way
> specified by the protocol and more reliable.
> 
> Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@...rosoft.com>
> ---
>  drivers/net/hyperv/netvsc_drv.c |   55 ++++++++++++++++++++++++++++++++++++---
>  1 files changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 9522763..c5778cf 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct net_device *netdev)
>  	free_netdev(netdev);
>  }
>  
> -static struct net_device *get_netvsc_bymac(const u8 *mac)
> +static struct net_device *get_netvsc_byvfser(u32 vfser)
>  {
>  	struct net_device *dev;
> +	struct net_device_context *ndev_ctx;
>  
>  	ASSERT_RTNL();
>  
> @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device *netdev)
>  		if (dev->netdev_ops != &device_ops)
>  			continue;	/* not a netvsc device */
>  
> -		if (ether_addr_equal(mac, dev->perm_addr))
> +		ndev_ctx = netdev_priv(dev);
> +		if (ndev_ctx->vf_serial == vfser)
>  			return dev;
>  	}
>  
> @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct net_device *netdev)
>  	return NULL;
>  }
>  
> +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> +{
> +	struct device *dev;
> +	struct hv_device *hdev;
> +	struct hv_pcibus_device *hbus = NULL;
> +	struct list_head *iter;
> +	struct hv_pci_dev *hpdev;
> +	unsigned long flags;
> +	u32 vfser = 0;
> +	u32 count = 0;
> +
> +	for (dev = &vf_netdev->dev; dev; dev = dev->parent) {

You are going to walk the whole device tree backwards?  That's crazy.
And foolish.  And racy and broken (what happens if the tree changes
while you do this?)  Where is the lock being grabbed while this happens?
What about reference counts?  Do you see other drivers ever doing this
(if you do, point them out and I'll go yell at them too...)

> +		if (!dev_is_vmbus(dev))
> +			continue;

Ick.

Why isn't your parent pointer a vmbus device all the time?  How could
you get burried down in the device hierarchy when you are the driver for
a specific bus type in the first place?  How could this function ever be
called for a device that is NOT of this type?

thanks,

greg k-h

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ