linux-kernel - RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

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:   Wed, 14 Dec 2016 23:18:59 +0000
From:   Haiyang Zhang <haiyangz@...rosoft.com>
To:     Greg KH <gregkh@...uxfoundation.org>,
        Stephen Hemminger <stephen@...workplumber.org>
CC:     "olaf@...fle.de" <olaf@...fle.de>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "bjorn.helgaas@...il.com" <bjorn.helgaas@...il.com>,
        "apw@...onical.com" <apw@...onical.com>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "leann.ogasawara@...onical.com" <leann.ogasawara@...onical.com>
Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
 numbers



> -----Original Message-----
> From: Greg KH [mailto:gregkh@...uxfoundation.org]
> Sent: Saturday, December 10, 2016 7:21 AM
> To: Stephen Hemminger <stephen@...workplumber.org>
> Cc: Haiyang Zhang <haiyangz@...rosoft.com>; olaf@...fle.de;
> jasowang@...hat.com; linux-kernel@...r.kernel.org;
> bjorn.helgaas@...il.com; apw@...onical.com; devel@...uxdriverproject.org;
> leann.ogasawara@...onical.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, Dec 09, 2016 at 04:21:48PM -0800, Stephen Hemminger wrote:
> > On Fri, 9 Dec 2016 22:35:05 +0000
> > Haiyang Zhang <haiyangz@...rosoft.com> wrote:
> >
> > > > > >
> > > > > > Emulated NIC is already excluded in start of netvc notifier
> handler.
> > > > > >
> > > > > > static int netvsc_netdev_event(struct notifier_block *this,
> > > > > > 			       unsigned long event, void *ptr)
> > > > > > {
> > > > > > 	struct net_device *event_dev =
> netdev_notifier_info_to_dev(ptr);
> > > > > >
> > > > > > 	/* Skip our own events */
> > > > > > 	if (event_dev->netdev_ops == &device_ops)
> > > > > > 		return NOTIFY_DONE;
> > > > > >
> > > > >
> > > > > Emulated device is not based on netvsc. It's the native Linux
> > > > (dec100M?)
> > > > > Driver. So this line doesn't exclude it. And how about other NIC
> type
> > > > > may be added in the future?
> > > >
> > > > Sorry, forgot about that haven't used emulated device in years.
> > > > The emulated device should appear to be on a PCI bus, but the
> serial
> > > > would not match??
> > >
> > > It's not a vmbus device, not a hv_pci device either. Hv_PCI is a
> subset
> > > of vmbus devices. So emulated NIC won't have hv_pci serial number.
> > >
> > > In my patch, the following code ensure, we only try to get serial
> number
> > > after confirming it's vmbus and hv_pci device:
> > >
> > > +               if (!dev_is_vmbus(dev))
> > > +                       continue;
> > > +
> > > +               hdev = device_to_hv_device(dev);
> > > +               if (hdev->device_id != HV_PCIE)
> > > +                       continue;
> >
> > Ok, the walk back up the device tree is logically ok, but I don't
> > know enough about PCI device tree to be assured that it is safe.
> > Also, you could short circuit away most of the unwanted devices
> > by making sure the vf_netdev->dev.parent is a PCI device.
> 
> Ugh, this seems really really messy.  Can't we just have the
> netdev_event interface pass back a pointer to something that we "know"
> what it is?  This walking the device tree is a mess, and not good.
> 
> I'd even argue that dev_is_pci() needs to be removed from the tree too,
> as it shouldn't be needed either.  We did a lot of work on the driver
> model to prevent the need for having to declare the "type" of 'struct
> device' at all, and by doing this type of thing it goes against the
> basic design of the model.
> 
> Yes, it makes things a bit "tougher" in places, but you don't do crazy
> things like walk device trees to try to find random devices and then
> think it's safe to actually use them :(
> 

We register a notifier_block with:
	register_netdevice_notifier(struct notifier_block *nb)

The "struct notifier_block" basically contains a callback function:
struct notifier_block {
        notifier_fn_t notifier_call;
        struct notifier_block __rcu *next;
        int priority;
};

It doesn't specify which device we want, so all net devices can trigger
this event. Seems we can't have this notifier return VF device only.

Thanks,
- Haiyang