From: Ville Syrjälä ville.syrjala@linux.intel.com
Currently dma_resv_get_fences() will leak the previously allocated array if the fence iteration got restarted and the krealloc_array() fails.
Free the old array by hand, and make sure we still clear the returned *fences so the caller won't end up accessing freed memory. Some (but not all) of the callers of dma_resv_get_fences() seem to still trawl through the array even when dma_resv_get_fences() failed. And let's zero out *num_fences as well for good measure.
Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Christian König christian.koenig@amd.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/dma-buf/dma-resv.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index b6f71eb00866..38b4110378de 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -571,6 +571,7 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage, 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) @@ -579,13 +580,17 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage, count = cursor.num_fences + 1;
/* Eventually re-allocate the array */ - *fences = krealloc_array(*fences, count, - sizeof(void *), - GFP_KERNEL); - if (count && !*fences) { + 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);
Am 13.07.23 um 21:47 schrieb Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Currently dma_resv_get_fences() will leak the previously allocated array if the fence iteration got restarted and the krealloc_array() fails.
Free the old array by hand, and make sure we still clear the returned *fences so the caller won't end up accessing freed memory. Some (but not all) of the callers of dma_resv_get_fences() seem to still trawl through the array even when dma_resv_get_fences() failed. And let's zero out *num_fences as well for good measure.
Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Christian König christian.koenig@amd.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Good catch, Reviewed-by: Christian König christian.koenig@amd.com
Should I add a CC: stable and push to drm-misc-fixes?
Thanks, Christian.
drivers/dma-buf/dma-resv.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index b6f71eb00866..38b4110378de 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -571,6 +571,7 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage, 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) @@ -579,13 +580,17 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage, count = cursor.num_fences + 1; /* Eventually re-allocate the array */
*fences = krealloc_array(*fences, count,sizeof(void *),GFP_KERNEL);if (count && !*fences) {
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);
On Fri, Jul 14, 2023 at 08:56:15AM +0200, Christian König wrote:
Am 13.07.23 um 21:47 schrieb Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Currently dma_resv_get_fences() will leak the previously allocated array if the fence iteration got restarted and the krealloc_array() fails.
Free the old array by hand, and make sure we still clear the returned *fences so the caller won't end up accessing freed memory. Some (but not all) of the callers of dma_resv_get_fences() seem to still trawl through the array even when dma_resv_get_fences() failed. And let's zero out *num_fences as well for good measure.
Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Christian König christian.koenig@amd.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Good catch, Reviewed-by: Christian König christian.koenig@amd.com
Should I add a CC: stable and push to drm-misc-fixes?
Sure, if you don't mind. Thanks.
Thanks, Christian.
drivers/dma-buf/dma-resv.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index b6f71eb00866..38b4110378de 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -571,6 +571,7 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage, 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) @@ -579,13 +580,17 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage, count = cursor.num_fences + 1; /* Eventually re-allocate the array */
*fences = krealloc_array(*fences, count,sizeof(void *),GFP_KERNEL);if (count && !*fences) {
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);
linaro-mm-sig@lists.linaro.org
-
Christian König -
Ville Syrjala -
Ville Syrjälä