Re: Question about 'dma_resv_get_fences'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Am 30.09.24 um 21:38 schrieb Zichen Xie:
Dear Linux Developers for DMA BUFFER SHARING FRAMEWORK,

We are curious about the function 'dma_resv_get_fences' here:
https://elixir.bootlin.com/linux/v6.11/source/drivers/dma-buf/dma-resv.c#L568,
and the logic below:
```
dma_resv_for_each_fence_unlocked(&cursor, fence) {

if (dma_resv_iter_is_restarted(&cursor)) {
struct dma_fence **new_fences;
unsigned int count;

while (*num_fences)
dma_fence_put((*fences)[--(*num_fences)]);

count = cursor.num_fences + 1;

/* Eventually re-allocate the array */
new_fences = krealloc_array(*fences, count,
     sizeof(void *),
     GFP_KERNEL);
if (count && !new_fences) {
kfree(*fences);
*fences = NULL;
*num_fences = 0;
dma_resv_iter_end(&cursor);
return -ENOMEM;
}
*fences = new_fences;
}

(*fences)[(*num_fences)++] = dma_fence_get(fence);
}
```
The existing check 'if (count && !new_fences)' may fail if count==0,
and 'krealloc_array' with count==0 is an undefined behavior. The
realloc may fail and return a NULL pointer, leading to a NULL Pointer
Dereference in '(*fences)[(*num_fences)++] = dma_fence_get(fence);'

You already answered the question yourself "count = cursor.num_fences + 1;". So count can never be 0.

What could theoretically be possible is that num_fences overflows, but this value isn't userspace controllable and we would run into memory allocation failures long before that happened.

But we could potentially remove this whole handling since if there are no fences in the dma_resv object we don't enter the loop in the first place.

Regards,
Christian.


Please correct us if we miss some key prerequisites for this function!
Thank you very much!





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux