linux-kernel - Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code

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: <d8f6ed37-11dc-1103-8908-ad79482a4694@linux.microsoft.com>
Date:   Thu, 3 Sep 2020 11:55:16 -0700
From:   Iouri Tarassov <iourit@...ux.microsoft.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Sasha Levin <sashal@...nel.org>, kys@...rosoft.com,
        haiyangz@...rosoft.com, sthemmin@...rosoft.com, wei.liu@...nel.org,
        iourit@...rosoft.com, linux-kernel@...r.kernel.org,
        linux-hyperv@...r.kernel.org, spronovo@...rosoft.com
Subject: Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code

Hi Greg,

I appreciate your comments and working to address them.

On 8/27/2020 11:12 PM, Greg KH wrote:
> As for "matching names", why does that matter?  Who sees both names at
> the same time?
>
> > > 
> > > endian issues?
> > > 
> > > If not, why are these bit fields?
> > This matches the definition on the Windows side. Windows only works on
> > little endian platforms.
>
> But Linux works on both, so you need to properly document/handle this somehow.
This driver works only in a Linux container in conjunction with the 
Windows host. The structure definitions are  the same on the host and 
the container. The driver will not be enabled or work on platforms, 
where Windows does not run.
>
> > > 
> > > > +struct d3dkmt_destroydevice {
> > > > +	struct d3dkmthandle		device;
> > > > +};
> > > 
> > > Again, single entity structures?
> > > 
> > > Are you trying to pass around "handles" and cast them backwards?
> > > 
> > > If so, great, but then use the real kernel structures for that like
> > > 'struct device' if these are actually devices.
> > > 
> > Again. The structure matches the definition on the Windows side to avoid
> > confusion.
>
> Who is confused here?  We accept naming conventions that do not match
> the normal Linux style when they are referring to external sources of
> the data.  Examples of this are USB device field names, and other
> hardware specifications that are public.  You aren't sharing code with a
> Windows system, so please follow the Linux coding style rules, as you
> want Linux developers to be helping you maintain this code, not
> developers who have ever read code from other operating systems.
>
> So please follow the rule of, "unless these fields and structures are
> publically defined somewhere, use Linux naming rules", like all of the
> rest of us do.
>
The d3dkmt* structures, like d3dkmt_destroydevice are publicly 
documented on MSDN 
(https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/d3dkmthk/ns-d3dkmthk-_d3dkmt_destroydevice). 
I am using the same definitions in the driver, so it is easy to find the 
corresponding definition and description of the structure. I have no 
problem to change the names, but I think using the same public 
definition will help the driver maintainers.

Thanks

Iouri