[PATCH 13/24] dma-buf: drop the DAG approach for the dma_resv object

[Christian König]

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.koenig at 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
>>