Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
the CMA API is now used directly and therefore the allocated memory is no
longer automatically zeroed.
Explicitly zero CMA allocated memory to ensure that no data is exposed to
userspace.
Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
Signed-off-by: Liam Mark <lmark(a)codeaurora.org>
---
Changes in v2:
- Clean up the commit message.
- Add 'Fixes:'
drivers/staging/android/ion/ion_cma_heap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
index 86196ffd2faf..91a98785607a 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -51,6 +51,8 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
if (!pages)
return -ENOMEM;
+ memset(page_address(pages), 0, size);
+
table = kmalloc(sizeof(*table), GFP_KERNEL);
if (!table)
goto err;
--
1.8.5.2
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Since the CMA API is now used directly the allocated memory is no longer
automatically zeroed.
Explicitly zero CMA allocated memory to ensure that no data is exposed
to userspace.
Change-Id: I08e143707a0d31610821a7f16826c262bf3c1999
Signed-off-by: Liam Mark <lmark(a)codeaurora.org>
---
drivers/staging/android/ion/ion_cma_heap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
index 86196ff..91a9878 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -51,6 +51,8 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
if (!pages)
return -ENOMEM;
+ memset(page_address(pages), 0, size);
+
table = kmalloc(sizeof(*table), GFP_KERNEL);
if (!table)
goto err;
--
1.8.5.2
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Change reservation_object_get_fences_rcu to make the exclusive fence
pointer optional.
If not specified the exclusive fence is put into the fence array as
well.
This is helpful for a couple of cases where we need all fences in a
single array.
Signed-off-by: Christian König <christian.koenig(a)amd.com>
---
drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b759a569b7b8..461afa9febd4 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
* @pshared: the array of shared fence ptrs returned (array is krealloc'd to
* the required size, and must be freed by caller)
*
- * RETURNS
- * Zero or -errno
+ * Retrieve all fences from the reservation object. If the pointer for the
+ * exclusive fence is not specified the fence is put into the array of the
+ * shared fences as well. Returns either zero or -ENOMEM.
*/
int reservation_object_get_fences_rcu(struct reservation_object *obj,
struct dma_fence **pfence_excl,
@@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
do {
struct reservation_object_list *fobj;
- unsigned seq;
- unsigned int i;
+ unsigned int i, seq;
+ size_t sz = 0;
shared_count = i = 0;
@@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
goto unlock;
fobj = rcu_dereference(obj->fence);
- if (fobj) {
+ if (fobj)
+ sz += sizeof(*shared) * fobj->shared_max;
+
+ if (!pfence_excl && fence_excl)
+ sz += sizeof(*shared);
+
+ if (sz) {
struct dma_fence **nshared;
- size_t sz = sizeof(*shared) * fobj->shared_max;
nshared = krealloc(shared, sz,
GFP_NOWAIT | __GFP_NOWARN);
@@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
break;
}
shared = nshared;
- shared_count = fobj->shared_count;
-
+ shared_count = fobj ? fobj->shared_count : 0;
for (i = 0; i < shared_count; ++i) {
shared[i] = rcu_dereference(fobj->shared[i]);
if (!dma_fence_get_rcu(shared[i]))
break;
}
+
+ if (!pfence_excl && fence_excl) {
+ shared[i] = fence_excl;
+ fence_excl = NULL;
+ ++i;
+ ++shared_count;
+ }
}
if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
@@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
*pshared_count = shared_count;
*pshared = shared;
- *pfence_excl = fence_excl;
+ if (pfence_excl)
+ *pfence_excl = fence_excl;
return ret;
}
--
2.14.1
Change reservation_object_get_fences_rcu to make the exclusive fence
pointer optional.
If not specified the exclusive fence is put into the fence array as
well.
This is helpful for a couple of cases where we need all fences in a
single array.
Signed-off-by: Christian König <christian.koenig(a)amd.com>
---
drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b759a569b7b8..461afa9febd4 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
* @pshared: the array of shared fence ptrs returned (array is krealloc'd to
* the required size, and must be freed by caller)
*
- * RETURNS
- * Zero or -errno
+ * Retrieve all fences from the reservation object. If the pointer for the
+ * exclusive fence is not specified the fence is put into the array of the
+ * shared fences as well. Returns either zero or -ENOMEM.
*/
int reservation_object_get_fences_rcu(struct reservation_object *obj,
struct dma_fence **pfence_excl,
@@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
do {
struct reservation_object_list *fobj;
- unsigned seq;
- unsigned int i;
+ unsigned int i, seq;
+ size_t sz = 0;
shared_count = i = 0;
@@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
goto unlock;
fobj = rcu_dereference(obj->fence);
- if (fobj) {
+ if (fobj)
+ sz += sizeof(*shared) * fobj->shared_max;
+
+ if (!pfence_excl && fence_excl)
+ sz += sizeof(*shared);
+
+ if (sz) {
struct dma_fence **nshared;
- size_t sz = sizeof(*shared) * fobj->shared_max;
nshared = krealloc(shared, sz,
GFP_NOWAIT | __GFP_NOWARN);
@@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
break;
}
shared = nshared;
- shared_count = fobj->shared_count;
-
+ shared_count = fobj ? fobj->shared_count : 0;
for (i = 0; i < shared_count; ++i) {
shared[i] = rcu_dereference(fobj->shared[i]);
if (!dma_fence_get_rcu(shared[i]))
break;
}
+
+ if (!pfence_excl && fence_excl) {
+ shared[i] = fence_excl;
+ fence_excl = NULL;
+ ++i;
+ ++shared_count;
+ }
}
if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
@@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
*pshared_count = shared_count;
*pshared = shared;
- *pfence_excl = fence_excl;
+ if (pfence_excl)
+ *pfence_excl = fence_excl;
return ret;
}
--
2.14.1
Am 01.12.2017 um 01:23 schrieb Lyude Paul:
> I haven't gone to see where it started, but as of late a good number of
> pretty nasty deadlock issues have appeared with the kernel. Easy
> reproduction recipe on a laptop with i915/amdgpu prime with lockdep enabled:
>
> DRI_PRIME=1 glxinfo
Acked-by: Christian König <christian.koenig(a)amd.com>
Thanks for taking care of this,
Christian.
>
> Additionally, some more race conditions exist that I've managed to
> trigger with piglit and lockdep enabled after applying these patches:
>
> =============================
> WARNING: suspicious RCU usage
> 4.14.3Lyude-Test+ #2 Not tainted
> -----------------------------
> ./include/linux/reservation.h:216 suspicious rcu_dereference_protected() usage!
>
> other info that might help us debug this:
>
> rcu_scheduler_active = 2, debug_locks = 1
> 1 lock held by ext_image_dma_b/27451:
> #0: (reservation_ww_class_mutex){+.+.}, at: [<ffffffffa034f2ff>] ttm_bo_unref+0x9f/0x3c0 [ttm]
>
> stack backtrace:
> CPU: 0 PID: 27451 Comm: ext_image_dma_b Not tainted 4.14.3Lyude-Test+ #2
> Hardware name: HP HP ZBook 15 G4/8275, BIOS P70 Ver. 01.02 06/09/2017
> Call Trace:
> dump_stack+0x8e/0xce
> lockdep_rcu_suspicious+0xc5/0x100
> reservation_object_copy_fences+0x292/0x2b0
> ? ttm_bo_unref+0x9f/0x3c0 [ttm]
> ttm_bo_unref+0xbd/0x3c0 [ttm]
> amdgpu_bo_unref+0x2a/0x50 [amdgpu]
> amdgpu_gem_object_free+0x4b/0x50 [amdgpu]
> drm_gem_object_free+0x1f/0x40 [drm]
> drm_gem_object_put_unlocked+0x40/0xb0 [drm]
> drm_gem_object_handle_put_unlocked+0x6c/0xb0 [drm]
> drm_gem_object_release_handle+0x51/0x90 [drm]
> drm_gem_handle_delete+0x5e/0x90 [drm]
> ? drm_gem_handle_create+0x40/0x40 [drm]
> drm_gem_close_ioctl+0x20/0x30 [drm]
> drm_ioctl_kernel+0x5d/0xb0 [drm]
> drm_ioctl+0x2f7/0x3b0 [drm]
> ? drm_gem_handle_create+0x40/0x40 [drm]
> ? trace_hardirqs_on_caller+0xf4/0x190
> ? trace_hardirqs_on+0xd/0x10
> amdgpu_drm_ioctl+0x4f/0x90 [amdgpu]
> do_vfs_ioctl+0x93/0x670
> ? __fget+0x108/0x1f0
> SyS_ioctl+0x79/0x90
> entry_SYSCALL_64_fastpath+0x23/0xc2
>
> I've also added the relevant fixes for the issue mentioned above.
>
> Christian König (3):
> drm/ttm: fix ttm_bo_cleanup_refs_or_queue once more
> dma-buf: make reservation_object_copy_fences rcu save
> drm/amdgpu: reserve root PD while releasing it
>
> Michel Dänzer (1):
> drm/ttm: Always and only destroy bo->ttm_resv in ttm_bo_release_list
>
> drivers/dma-buf/reservation.c | 56 +++++++++++++++++++++++++---------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 ++++++--
> drivers/gpu/drm/ttm/ttm_bo.c | 43 +++++++++++++-------------
> 3 files changed, 74 insertions(+), 38 deletions(-)
>
> --
> 2.14.3
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx(a)lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
If we are testing if a reservation object's fences have been
signaled with timeout=0 (non-blocking), we need to pass 0 for
timeout to dma_fence_wait_timeout().
Plus bonus spelling correction.
Signed-off-by: Rob Clark <robdclark(a)gmail.com>
---
drivers/dma-buf/reservation.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index dec3a815455d..71f51140a9ad 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -420,7 +420,7 @@ EXPORT_SYMBOL_GPL(reservation_object_get_fences_rcu);
*
* RETURNS
* Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or
- * greater than zer on success.
+ * greater than zero on success.
*/
long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
bool wait_all, bool intr,
@@ -483,7 +483,14 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
goto retry;
}
- ret = dma_fence_wait_timeout(fence, intr, ret);
+ /*
+ * Note that dma_fence_wait_timeout() will return 1 if
+ * the fence is already signaled, so in the wait_all
+ * case when we go through the retry loop again, ret
+ * will be greater than 0 and we don't want this to
+ * cause _wait_timeout() to block
+ */
+ ret = dma_fence_wait_timeout(fence, intr, timeout ? ret : 0);
dma_fence_put(fence);
if (ret > 0 && wait_all && (i + 1 < shared_count))
goto retry;
--
2.13.6