Am 22.12.21 um 22:43 schrieb Daniel Vetter:
On Tue, Dec 07, 2021 at 01:34:00PM +0100, Christian König wrote:
So far we had the approach of using a directed acyclic
graph with the dma_resv obj.

This turned out to have many downsides, especially it means
that every single driver and user of this interface needs
to be aware of this restriction when adding fences. If the
rules for the DAG are not followed then we end up with
potential hard to debug memory corruption, information
leaks or even elephant big security holes because we allow
userspace to access freed up memory.

Since we already took a step back from that by always
looking at all fences we now go a step further and stop
dropping the shared fences when a new exclusive one is
added.

Signed-off-by: Christian König <christian.koe...@amd.com>
---
  drivers/dma-buf/dma-resv.c | 13 -------------
  1 file changed, 13 deletions(-)

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 9acceabc9399..ecb2ff606bac 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
No doc update at all!

Scratching my head I'm pretty sure I've updated at least the kerneldoc for dma_resv_add_excl_fence(). Must have gone lost in some rebase.


I checked, we're not that shitty with docs,

Well I wouldn't say shitty, but they are not perfect either.

  Minimally the DOC: section
header and also the struct dma_resv kerneldoc. Also there's maybe more
references and stuff I've missed on a quick look, please check for them
(e.g. dma_buf.resv kerneldoc is rather important to keep correct too).

Code itself does what it says in the commit message, but we really need
the most accurate docs we can get for this stuff, or the confusion will
persist :-/

Yeah completely agree, going to fix that.

Thanks,
Christian.


Cheers, Daniel

@@ -383,29 +383,16 @@ EXPORT_SYMBOL(dma_resv_replace_fences);
  void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
  {
        struct dma_fence *old_fence = dma_resv_excl_fence(obj);
-       struct dma_resv_list *old;
-       u32 i = 0;
dma_resv_assert_held(obj); - old = dma_resv_shared_list(obj);
-       if (old)
-               i = old->shared_count;
-
        dma_fence_get(fence);
write_seqcount_begin(&obj->seq);
        /* write_seqcount_begin provides the necessary memory barrier */
        RCU_INIT_POINTER(obj->fence_excl, fence);
-       if (old)
-               old->shared_count = 0;
        write_seqcount_end(&obj->seq);
- /* inplace update, no shared fences */
-       while (i--)
-               dma_fence_put(rcu_dereference_protected(old->shared[i],
-                                               dma_resv_held(obj)));
-
        dma_fence_put(old_fence);
  }
  EXPORT_SYMBOL(dma_resv_add_excl_fence);
--
2.25.1


Reply via email to