linux-kernel - Re: A proposal to check the device in generic way

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] [day] [month] [year] [list]

Message-ID: <20130311170902.GN4977@n2100.arm.linux.org.uk>
Date:	Mon, 11 Mar 2013 17:09:03 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Guennadi Liakhovetski <g.liakhovetski@....de>
Cc:	"Shevchenko, Andriy" <andriy.shevchenko@...el.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"Koul, Vinod" <vinod.koul@...el.com>,
	Arnd Bergmann <arnd.bergmann@...aro.org>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: Re: A proposal to check the device in generic way

On Mon, Mar 11, 2013 at 01:26:01PM +0100, Guennadi Liakhovetski wrote:
> Hi Andriy
> 
> (adding Russell to CC)
> 
> On Mon, 11 Mar 2013, Shevchenko, Andriy wrote:
> 
> > Hello.
> > 
> > Currently in linux-next we have the following things:
> > 
> > $ git grep -n 'chan->device->dev->driver' drivers/dma/
> > 
> > drivers/dma/amba-pl08x.c:1594:  if (chan->device->dev->driver !=
> > &pl08x_amba_driver.drv)
> > drivers/dma/dmaengine.c:190:    return chan->device->dev->driver->owner;
> > drivers/dma/edma.c:609: if (chan->device->dev->driver ==
> > &edma_driver.driver) {
> > drivers/dma/omap-dma.c:654:     if (chan->device->dev->driver ==
> > &omap_dma_driver.driver) {
> > drivers/dma/pl330.c:2374:       if (chan->device->dev->driver !=
> > &pl330_driver.drv)
> > drivers/dma/sa11x0-dma.c:1080:  if (chan->device->dev->driver ==
> > &sa11x0_dma_driver.driver) 
> > 
> > I think it's a non-generic way to check which driver provides a channel
> > into filter function. First of all, I don't get why that comparison goes
> > as deep as driver structure. Isn't clearer to check chan->device->dev
> > against the struct dev passed in the custom parameter structure? Like:
> > 
> > struct filter_params {
> >  struct dev *dev;
> >  void *param;
> > };
> 
> I don't think you always know which DMA device you want to use with this 
> DMA client - sometimes there are several DMA engine devices, that can be 
> used with your DMA client, or even if it's only one, you don't necessarily 
> have a pointer to it in your DMA client.

Correct - take a look at what gets passed - for example, sometimes the
filter parameter is just a string.  Sometimes, even in the static
platform code, we may not have an address for the DMA device structure.

So, how would you supply the struct device for the DMA?  You can't.

But, think about it a moment.  What we're actually really interested in
is: do we know the structure of the driver's channel specific data?
What determines that?  The device?  Or the driver?  It's the driver.

So for safe and correct type checking, we need to make sure that the
dma_chan is really associated with _this_ driver before we dereference
anything in the driver-private data structures surrounding dma_chan.

Hence, the struct device_driver is the right thing to use.  I would
however agree that it would be nice if the device model provided us a
way to do that without having to delve that deeply ourselves.
--
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/