This patch adds EXPORT_SYMBOL_GPL calls to the three arm iommu
functions - arm_iommu_create_mapping, arm_iommu_free_mapping
and arm_iommu_attach_device. These three functions are arm specific
wrapper functions for creating/freeing/using an iommu mapping and
they are called by various drivers. If any of these drivers need
to be built as dynamic modules, these functions need to be exported.
Changelog v2: using EXPORT_SYMBOL_GPL as suggested by Marek.
Signed-off-by: Prathyush K <prathyush.k(a)samsung.com>
---
arch/arm/mm/dma-mapping.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 6b2fb87..226ebcf 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1797,6 +1797,7 @@ err2:
err:
return ERR_PTR(err);
}
+EXPORT_SYMBOL_GPL(arm_iommu_create_mapping);
static void release_iommu_mapping(struct kref *kref)
{
@@ -1813,6 +1814,7 @@ void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping)
if (mapping)
kref_put(&mapping->kref, release_iommu_mapping);
}
+EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
/**
* arm_iommu_attach_device
@@ -1841,5 +1843,6 @@ int arm_iommu_attach_device(struct device *dev,
pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
return 0;
}
+EXPORT_SYMBOL_GPL(arm_iommu_attach_device);
#endif
--
1.8.0
From: Inki Dae <inki.dae(a)samsung.com>
This patch adds a new attribute, DMA_ATTR_SKIP_BUFFER_CLEAR
to skip buffer clearing. The buffer clearing also flushes CPU cache
so this operation has performance deterioration a little bit.
With this patch, allocated buffer region is cleared as default.
So if you want to skip the buffer clearing, just set this attribute.
But this flag should be used carefully because this use might get
access to some vulnerable content such as security data. So with this
patch, we make sure that all pages will be somehow cleared before
exposing to userspace.
For example, let's say that the security data had been stored
in some memory and freed without clearing it.
And then malicious process allocated the region though some buffer
allocator such as gem and ion without clearing it, and requested blit
operation with cleared another buffer though gpu or other drivers.
At this time, the malicious process could access the security data.
Signed-off-by: Inki Dae <inki.dae(a)samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park(a)samsung.com>
---
arch/arm/mm/dma-mapping.c | 6 ++++--
include/linux/dma-attrs.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 6b2fb87..fbe9dff 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1058,7 +1058,8 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
if (!page)
goto error;
- __dma_clear_buffer(page, size);
+ if (!dma_get_attr(DMA_ATTR_SKIP_BUFFER_CLEAR, attrs))
+ __dma_clear_buffer(page, size);
for (i = 0; i < count; i++)
pages[i] = page + i;
@@ -1082,7 +1083,8 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
pages[i + j] = pages[i] + j;
}
- __dma_clear_buffer(pages[i], PAGE_SIZE << order);
+ if (!dma_get_attr(DMA_ATTR_SKIP_BUFFER_CLEAR, attrs))
+ __dma_clear_buffer(pages[i], PAGE_SIZE << order);
i += 1 << order;
count -= 1 << order;
}
diff --git a/include/linux/dma-attrs.h b/include/linux/dma-attrs.h
index c8e1831..2592c05 100644
--- a/include/linux/dma-attrs.h
+++ b/include/linux/dma-attrs.h
@@ -18,6 +18,7 @@ enum dma_attr {
DMA_ATTR_NO_KERNEL_MAPPING,
DMA_ATTR_SKIP_CPU_SYNC,
DMA_ATTR_FORCE_CONTIGUOUS,
+ DMA_ATTR_SKIP_BUFFER_CLEAR,
DMA_ATTR_MAX,
};
--
1.7.4.1
All drivers which implement this need to have some sort of refcount to
allow concurrent vmap usage. Hence implement this in the dma-buf core.
To protect against concurrent calls we need a lock, which potentially
causes new funny locking inversions. But this shouldn't be a problem
for exporters with statically allocated backing storage, and more
dynamic drivers have decent issues already anyway.
Inspired by some refactoring patches from Aaron Plattner, who
implemented the same idea, but only for drm/prime drivers.
v2: Check in dma_buf_release that no dangling vmaps are left.
Suggested by Aaron Plattner. We might want to do similar checks for
attachments, but that's for another patch. Also fix up ERR_PTR return
for vmap.
Cc: Aaron Plattner <aplattner(a)nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter(a)ffwll.ch>
---
Compile-tested only - Aaron has been bugging me too a bit too often
about this on irc.
Cheers, Daniel
---
Documentation/dma-buf-sharing.txt | 6 +++++-
drivers/base/dma-buf.c | 42 ++++++++++++++++++++++++++++++++++-----
include/linux/dma-buf.h | 4 +++-
3 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt
index 0188903..4966b1b 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -302,7 +302,11 @@ Access to a dma_buf from the kernel context involves three steps:
void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
The vmap call can fail if there is no vmap support in the exporter, or if it
- runs out of vmalloc space. Fallback to kmap should be implemented.
+ runs out of vmalloc space. Fallback to kmap should be implemented. Note that
+ the dma-buf layer keeps a reference count for all vmap access and calls down
+ into the exporter's vmap function only when no vmapping exists, and only
+ unmaps it once. Protection against concurrent vmap/vunmap calls is provided
+ by taking the dma_buf->lock mutex.
3. Finish access
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index a3f79c4..36af5de 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -39,6 +39,8 @@ static int dma_buf_release(struct inode *inode, struct file *file)
dmabuf = file->private_data;
+ BUG_ON(dmabuf->vmapping_counter);
+
dmabuf->ops->release(dmabuf);
kfree(dmabuf);
return 0;
@@ -482,12 +484,34 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
*/
void *dma_buf_vmap(struct dma_buf *dmabuf)
{
+ void *ptr;
+
if (WARN_ON(!dmabuf))
return NULL;
- if (dmabuf->ops->vmap)
- return dmabuf->ops->vmap(dmabuf);
- return NULL;
+ if (!dmabuf->ops->vmap)
+ return NULL;
+
+ mutex_lock(&dmabuf->lock);
+ if (dmabuf->vmapping_counter) {
+ dmabuf->vmapping_counter++;
+ BUG_ON(!dmabuf->vmap_ptr);
+ ptr = dmabuf->vmap_ptr;
+ goto out_unlock;
+ }
+
+ BUG_ON(dmabuf->vmap_ptr);
+
+ ptr = dmabuf->ops->vmap(dmabuf);
+ if (IS_ERR_OR_NULL(ptr))
+ goto out_unlock;
+
+ dmabuf->vmap_ptr = ptr;
+ dmabuf->vmapping_counter = 1;
+
+out_unlock:
+ mutex_unlock(&dmabuf->lock);
+ return ptr;
}
EXPORT_SYMBOL_GPL(dma_buf_vmap);
@@ -501,7 +525,15 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
if (WARN_ON(!dmabuf))
return;
- if (dmabuf->ops->vunmap)
- dmabuf->ops->vunmap(dmabuf, vaddr);
+ BUG_ON(!dmabuf->vmap_ptr);
+ BUG_ON(dmabuf->vmapping_counter > 0);
+
+ mutex_lock(&dmabuf->lock);
+ if (--dmabuf->vmapping_counter == 0) {
+ if (dmabuf->ops->vunmap)
+ dmabuf->ops->vunmap(dmabuf, vaddr);
+ dmabuf->vmap_ptr = NULL;
+ }
+ mutex_unlock(&dmabuf->lock);
}
EXPORT_SYMBOL_GPL(dma_buf_vunmap);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index bd2e52c..e3bf2f6 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -119,8 +119,10 @@ struct dma_buf {
struct file *file;
struct list_head attachments;
const struct dma_buf_ops *ops;
- /* mutex to serialize list manipulation and attach/detach */
+ /* mutex to serialize list manipulation, attach/detach and vmap/unmap */
struct mutex lock;
+ unsigned vmapping_counter;
+ void *vmap_ptr;
void *priv;
};
--
1.7.11.7
Hi Linus,
A fairly small dma-buf pull request for 3.8 - only 2 patches. Could
you please pull?
Thanks!
~Sumit.
The following changes since commit f01af9f85855e38fbd601e033a8eac204cc4cc1c:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc
(2012-12-19 20:31:02 -0800)
are available in the git repository at:
git://git.linaro.org/people/sumitsemwal/linux-dma-buf.git
tags/tag-for-linus-3.8
for you to fetch changes up to ada65c74059f8c104f1b467c126205471634c435:
dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER (2012-12-20
12:05:06 +0530)
----------------------------------------------------------------
3.8: dma-buf minor updates
----------------------------------------------------------------
Maarten Lankhorst (1):
dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER
Rob Clark (1):
dma-buf: might_sleep() in dma_buf_unmap_attachment()
drivers/base/dma-buf.c | 2 +
include/linux/dma-buf.h | 99 -----------------------------------------------
2 files changed, 2 insertions(+), 99 deletions(-)
So I've gotten back to playing with prime for a day, and found some
old intel/radeon tests I had failing,
Tracked it down to a lifetime issue with the current code and can
think of two fixes,
The problem scenario is
1. i915: create gem object
2. i915: export gem object to prime
3. radeon: import gem object
4. close prime fd
5. radeon: unref object
6. i915: unref object
So we end up at this point, with a imported buffer record for the
dma_buf on the i915 file private.
Now if a subsequent test (without closing the drm fd) reallocates the
dma_buf with the same address, we can end up seeing that.
So why doesn't that reference get cleaned up?
So the reference gets added above at step 2, and when radeon unrefs
the object, i915 gets the dma-buf release callback, however at that
stage
we don't actually have the file priv to remove the pointer from, so it
dangles there.
Possible fixes:
a) take a reference on the dma_buf attached to the gem handle when we
export it, keep it until the gem handle goes away. I'm unsure if this
could create zombie objects, since the dma buf has a reference on the
gem object, but since the gem handle is separate to the object it
might work.
b) don't keep track of dma_buf, keep track of gem objects, when we get
a lookup, check inside the gem object, since currently we NULL out the
export_dma_buf when we hit the release path, apart from the fact I'm
sure the locking is foobar,
c) scan all the file privs for all the devices, no.
Anyone else any better plans?
Dave.