linux-kernel - Re: [RFC] dma-buf: Implement test module

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, 12 Dec 2013 16:08:59 +0100
From:	Thierry Reding <thierry.reding@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sumit Semwal <sumit.semwal@...aro.org>,
	linaro-mm-sig@...ts.linaro.org, linux-kernel@...r.kernel.org,
	dri-devel@...ts.freedesktop.org
Subject: Re: [RFC] dma-buf: Implement test module

On Thu, Dec 12, 2013 at 03:53:05PM +0100, Daniel Vetter wrote:
> On Thu, Dec 12, 2013 at 03:36:29PM +0100, Thierry Reding wrote:
[...]
> > diff --git a/drivers/base/dma-buf-test.c b/drivers/base/dma-buf-test.c
[...]
> > +struct dmabuf_create {
> > +	__u32 flags;
> > +	__u32 size;
> > +};
> > +
> > +#define DMABUF_IOCTL_BASE	'D'
> > +#define DMABUF_IOCTL_CREATE	_IOWR(DMABUF_IOCTL_BASE, 0, struct dmabuf_create)
> > +#define DMABUF_IOCTL_DELETE	_IOWR(DMABUF_IOCTL_BASE, 1, int)
> > +#define DMABUF_IOCTL_EXPORT	_IOWR(DMABUF_IOCTL_BASE, 2, int)
> 
> Shouldn't we put this into an uapi header somewhere?

Yes, definitely. I just didn't want to go through all that trouble
before checking that anyone else actually thought this was a good
idea.

There are a few other things that probably need to be solved before this
can be merged, though. For instance, it currently doesn't compile on x86
because it uses dma_alloc_writecombine(). So there's ample room for
improvement.

> Also I think the ioctl interface is a bit convoluted - I'd just make
> one single interface which directly returns a new dma-buf fd. Removing
> it can be handled with close, no other ioctls required imo. The
> explicit export step is kinda only for subsystems that already have an
> existing buffer handle space, but we don't have this here.

I initially thought that this allowed more flexibility, but on the other
hand having to keep track of a number of buffers would add complexity to
the implementation and like you said it's probably not worth it. Also,
it's not like the code currently copes well with multiple buffers being
created. It just chickens out by returning -EBUSY.

The equivalent could be achieved by beefing up the DMABUF_IOCTL_CREATE
thusly:

	struct dmabuf_create {
		__u32 flags;
		__u32 size;
		__u32 fd;
		__u32 pad;
	};

Then use the size and flags parameters as before, but return the file
descriptor to the DMA-BUF in .fd. Then simply keep it open until the
descriptor is closed. That would also seem to be a more reliable
interface since closing the fd just decrements the reference count on
the DMA-BUF and therefore would keep it alive if somebody still kept
around a reference (the importer for instance).

Thierry

Content of type "application/pgp-signature" skipped