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: <aBB0NNOg47XHIjpq@phenom.ffwll.local>
Date: Tue, 29 Apr 2025 08:39:48 +0200
From: Simona Vetter <simona.vetter@...ll.ch>
To: Nicolas Dufresne <nicolas@...fresne.ca>
Cc: Bastien Curutchet <bastien.curutchet@...tlin.com>,
	Sumit Semwal <sumit.semwal@...aro.org>,
	Christian König <christian.koenig@....com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	linaro-mm-sig@...ts.linaro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] dma-buf: Add DMA_BUF_IOCTL_GET_DMA_ADDR

Catching up after spring break, hence the late reply ...

On Fri, Apr 11, 2025 at 02:34:37PM -0400, Nicolas Dufresne wrote:
> Le jeudi 10 avril 2025 à 16:53 +0200, Bastien Curutchet a écrit :
> > There is no way to transmit the DMA address of a buffer to userspace.
> > Some UIO users need this to handle DMA from userspace.
> 
> To me this API is against all safe practice we've been pushing forward
> and has no place in DMA_BUF API.
> 
> If this is fine for the UIO subsystem to pass around physicial
> addresses, then make this part of the UIO device ioctl.

Yeah, this has no business in dma-buf since the entire point of dma-buf
was to stop all the nasty "just pass raw dma addr in userspace" hacks that
preceeded it.

And over the years since dma-buf landed, we've removed a lot of these,
like dri1 drivers. Or where that's not possible like with fbdev, hid the
raw dma addr uapi behind a Kconfig.

I concur with the overall sentiment that this should be done in
vfio/iommufd interfaces, maybe with some support added to map dma-buf. I
think patches for that have been floating around for a while, but I lost a
bit the status of where exactly they are.

Cheers, Sima

> 
> regards,
> Nicolas
> 
> > 
> > Add a new dma_buf_ops operation that returns the DMA address.
> > Add a new ioctl to transmit this DMA address to userspace.
> > 
> > Signed-off-by: Bastien Curutchet <bastien.curutchet@...tlin.com>
> > ---
> >  drivers/dma-buf/dma-buf.c    | 21 +++++++++++++++++++++
> >  include/linux/dma-buf.h      |  1 +
> >  include/uapi/linux/dma-buf.h |  1 +
> >  3 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index
> > 398418bd9731ad7a3a1f12eaea6a155fa77a22fe..cbbb518981e54e50f479c3d1fcf
> > 6da6971f639c1 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -454,6 +454,24 @@ static long dma_buf_import_sync_file(struct
> > dma_buf *dmabuf,
> >  }
> >  #endif
> >  
> > +static int dma_buf_get_dma_addr(struct dma_buf *dmabuf, u64 __user
> > *arg)
> > +{
> > +	u64 addr;
> > +	int ret;
> > +
> > +	if (!dmabuf->ops->get_dma_addr)
> > +		return -EINVAL;
> > +
> > +	ret = dmabuf->ops->get_dma_addr(dmabuf, &addr);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (copy_to_user(arg, &addr, sizeof(u64)))
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> >  static long dma_buf_ioctl(struct file *file,
> >  			  unsigned int cmd, unsigned long arg)
> >  {
> > @@ -504,6 +522,9 @@ static long dma_buf_ioctl(struct file *file,
> >  		return dma_buf_import_sync_file(dmabuf, (const void
> > __user *)arg);
> >  #endif
> >  
> > +	case DMA_BUF_IOCTL_GET_DMA_ADDR:
> > +		return dma_buf_get_dma_addr(dmabuf, (u64 __user
> > *)arg);
> > +
> >  	default:
> >  		return -ENOTTY;
> >  	}
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index
> > 36216d28d8bdc01a9c9c47e27c392413f7f6c5fb..ed4bf15d3ce82e7a86323fff459
> > 699a9bc8baa3b 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -285,6 +285,7 @@ struct dma_buf_ops {
> >  
> >  	int (*vmap)(struct dma_buf *dmabuf, struct iosys_map *map);
> >  	void (*vunmap)(struct dma_buf *dmabuf, struct iosys_map
> > *map);
> > +	int (*get_dma_addr)(struct dma_buf *dmabuf, u64 *addr);
> >  };
> >  
> >  /**
> > diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-
> > buf.h
> > index
> > 5a6fda66d9adf01438619e7e67fa69f0fec2d88d..f3aba46942042de6a2e3a4cca3e
> > b3f87175e29c9 100644
> > --- a/include/uapi/linux/dma-buf.h
> > +++ b/include/uapi/linux/dma-buf.h
> > @@ -178,5 +178,6 @@ struct dma_buf_import_sync_file {
> >  #define DMA_BUF_SET_NAME_B	_IOW(DMA_BUF_BASE, 1, __u64)
> >  #define DMA_BUF_IOCTL_EXPORT_SYNC_FILE	_IOWR(DMA_BUF_BASE, 2,
> > struct dma_buf_export_sync_file)
> >  #define DMA_BUF_IOCTL_IMPORT_SYNC_FILE	_IOW(DMA_BUF_BASE, 3, struct
> > dma_buf_import_sync_file)
> > +#define DMA_BUF_IOCTL_GET_DMA_ADDR	_IOR(DMA_BUF_BASE, 4, __u64
> > *)
> >  
> >  #endif

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ