linux-kernel - RE: [PATCH 1/1] Staging: hv: Move the mouse driver out of staging

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: <6E21E5352C11B742B20C142EB499E0481AA6A1DF@TK5EX14MBXC128.redmond.corp.microsoft.com>
Date:	Wed, 9 Nov 2011 14:43:50 +0000
From:	KY Srinivasan <kys@...rosoft.com>
To:	Greg KH <gregkh@...e.de>
CC:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"ohering@...e.com" <ohering@...e.com>,
	"joe@...ches.com" <joe@...ches.com>,
	"jkosina@...e.cz" <jkosina@...e.cz>
Subject: RE: [PATCH 1/1] Staging: hv: Move the mouse driver out of staging



> -----Original Message-----
> From: Greg KH [mailto:gregkh@...e.de]
> Sent: Tuesday, November 08, 2011 7:49 PM
> To: KY Srinivasan
> Cc: Dmitry Torokhov; linux-kernel@...r.kernel.org;
> devel@...uxdriverproject.org; ohering@...e.com; joe@...ches.com;
> jkosina@...e.cz
> Subject: Re: [PATCH 1/1] Staging: hv: Move the mouse driver out of staging
> 
> On Wed, Nov 09, 2011 at 12:45:30AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dmitry Torokhov [mailto:dmitry.torokhov@...il.com]
> > > Sent: Monday, November 07, 2011 12:51 AM
> > > To: KY Srinivasan
> > > Cc: gregkh@...e.de; linux-kernel@...r.kernel.org;
> > > devel@...uxdriverproject.org; virtualization@...ts.osdl.org;
> ohering@...e.com;
> > > joe@...ches.com; jkosina@...e.cz
> > > Subject: Re: [PATCH 1/1] Staging: hv: Move the mouse driver out of staging
> > >
> > > Hi K. Y,
> > >
> > > On Mon, Nov 07, 2011 at 01:04:53AM +0000, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Dmitry Torokhov [mailto:dmitry.torokhov@...il.com]
> > > > > Sent: Saturday, November 05, 2011 2:48 AM
> > > > > To: KY Srinivasan
> > > > > Cc: gregkh@...e.de; linux-kernel@...r.kernel.org;
> > > > > devel@...uxdriverproject.org; virtualization@...ts.osdl.org;
> > > ohering@...e.com;
> > > > > joe@...ches.com; jkosina@...e.cz
> > > > > Subject: Re: [PATCH 1/1] Staging: hv: Move the mouse driver out of
> staging
> > > > >
> > > > > Hi KY,
> > > >
> > > > Dimitry,
> > > >
> > > > Let me begin by thanking you for taking the time to review. I have
> incorporated
> > > > pretty much all your suggestions.
> > >
> > > Thank you very much for considering my suggestions.
> >
> > >
> > > > >
> > > > > Instead of potentially ever-increasing buffer that you also allocate
> > > > > (and it looks like leaking on every callback invocation) can you just
> > > > > repeat the read if you know that there are more data and use single
> > > > > pre-allocated buffer?
> > > >
> > > > The ring-buffer protocol is such that we need to consume the full message.
> > > > Also, why do you say we are leaking memory?
> > >
> > > Ah, OK, I see, we keep reading until read returns 0-sized reply and then
> > > we free the buffer... Never mind then.
> > >
> > > > > > +
> > > > > > +	hid_dev->ll_driver = &mousevsc_ll_driver;
> > > > > > +	hid_dev->driver = &mousevsc_hid_driver;
> > > > >
> > > > > You are not really hid driver; you are more of a "provider" so why do
> > > > > you need to set hid_dev->driver in addition to hid_dev->ll_driver?
> > > > >
> > > > True, but hid_parse_report() expects that the driver field be set; so I
> > > > need to fake this.
> > >
> > > If you supply .parse() method for your mousevsc_ll_driver structure and
> > > call hid_parse_report() from it then HID core will set the default
> > > driver and call parse at appropriate time.
> >
> > Would it be ok if I were to make this change after Jiri accepts the driver out of
> staging?
> 
> Why not fix this now?  No need to take a driver that is known to have
> issues, right?

The issue I am running into is that the mouse_vsc driver is both a low level driver as 
well as the higher level driver all rolled into one. This is based on my understanding of the
way the way the hid drivers are structured under driver/hid. Furthermore, there is going to
be only one driver using this hyperv low level driver and there is really nothing much to do in 
that driver either. Thus, in my mind it makes sense to structure the driver as I have currently structured.
In any event, I am going to send the patches out as I have them. If there is a consensus that further
changes are needed, I will obviously make them independent of where the driver is (either in staging 
or not). 

Regards,

K. Y 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/