accel/amdxdna: Fix leak when pinning ubuf pages

[PATCH V1] accel/amdxdna: Fix leak when pinning ubuf pages

Posted by Lizhi Hou 1 week ago

From: Max Zhen <max.zhen@amd.com>

When pin_user_pages_fast() returns fewer pages than requested, the pages
that were successfully pinned are not released, leading to a leak.

Fix this by unpinning any partially pinned pages before returning failure.

Fixes: bd72d4acda10 ("accel/amdxdna: Support user space allocated buffer")
Signed-off-by: Max Zhen <max.zhen@amd.com>
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_ubuf.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c
index fb999aa25318..4c0647057759 100644
--- a/drivers/accel/amdxdna/amdxdna_ubuf.c
+++ b/drivers/accel/amdxdna/amdxdna_ubuf.c
@@ -196,13 +196,17 @@ struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev,
 		ret = pin_user_pages_fast(va_ent[i].vaddr, npages,
 					  FOLL_WRITE | FOLL_LONGTERM,
 					  &ubuf->pages[start]);
-		if (ret < 0 || ret != npages) {
-			ret = -ENOMEM;
+		if (ret >= 0) {
+			start += ret;
+			if (ret != npages) {
+				XDNA_ERR(xdna, "Partially pinned pages %d/%u", ret, npages);
+				ret = -ENOMEM;
+				goto destroy_pages;
+			}
+		} else {
 			XDNA_ERR(xdna, "Failed to pin pages ret %d", ret);
 			goto destroy_pages;
 		}
-
-		start += ret;
 	}
 
 	exp_info.ops = &amdxdna_ubuf_dmabuf_ops;
-- 
2.34.1

Re: [PATCH V1] accel/amdxdna: Fix leak when pinning ubuf pages

Posted by Mario Limonciello 1 week ago


On 3/25/26 20:06, Lizhi Hou wrote:
> From: Max Zhen <max.zhen@amd.com>
> 
> When pin_user_pages_fast() returns fewer pages than requested, the pages
> that were successfully pinned are not released, leading to a leak.
> 
> Fix this by unpinning any partially pinned pages before returning failure.
> 
> Fixes: bd72d4acda10 ("accel/amdxdna: Support user space allocated buffer")
> Signed-off-by: Max Zhen <max.zhen@amd.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   drivers/accel/amdxdna/amdxdna_ubuf.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c
> index fb999aa25318..4c0647057759 100644
> --- a/drivers/accel/amdxdna/amdxdna_ubuf.c
> +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c
> @@ -196,13 +196,17 @@ struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev,
>   		ret = pin_user_pages_fast(va_ent[i].vaddr, npages,
>   					  FOLL_WRITE | FOLL_LONGTERM,
>   					  &ubuf->pages[start]);
> -		if (ret < 0 || ret != npages) {
> -			ret = -ENOMEM;
> +		if (ret >= 0) {
> +			start += ret;
> +			if (ret != npages) {
> +				XDNA_ERR(xdna, "Partially pinned pages %d/%u", ret, npages);
> +				ret = -ENOMEM;
> +				goto destroy_pages;
> +			}
> +		} else {
>   			XDNA_ERR(xdna, "Failed to pin pages ret %d", ret);
>   			goto destroy_pages;
>   		}
> -
> -		start += ret;
>   	}
>   
>   	exp_info.ops = &amdxdna_ubuf_dmabuf_ops;

Re: [PATCH V1] accel/amdxdna: Fix leak when pinning ubuf pages

Posted by Lizhi Hou 1 week ago

Applied to drm-misc-next

On 3/25/26 19:31, Mario Limonciello wrote:
>
>
> On 3/25/26 20:06, Lizhi Hou wrote:
>> From: Max Zhen <max.zhen@amd.com>
>>
>> When pin_user_pages_fast() returns fewer pages than requested, the pages
>> that were successfully pinned are not released, leading to a leak.
>>
>> Fix this by unpinning any partially pinned pages before returning 
>> failure.
>>
>> Fixes: bd72d4acda10 ("accel/amdxdna: Support user space allocated 
>> buffer")
>> Signed-off-by: Max Zhen <max.zhen@amd.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>>   drivers/accel/amdxdna/amdxdna_ubuf.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c 
>> b/drivers/accel/amdxdna/amdxdna_ubuf.c
>> index fb999aa25318..4c0647057759 100644
>> --- a/drivers/accel/amdxdna/amdxdna_ubuf.c
>> +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c
>> @@ -196,13 +196,17 @@ struct dma_buf *amdxdna_get_ubuf(struct 
>> drm_device *dev,
>>           ret = pin_user_pages_fast(va_ent[i].vaddr, npages,
>>                         FOLL_WRITE | FOLL_LONGTERM,
>>                         &ubuf->pages[start]);
>> -        if (ret < 0 || ret != npages) {
>> -            ret = -ENOMEM;
>> +        if (ret >= 0) {
>> +            start += ret;
>> +            if (ret != npages) {
>> +                XDNA_ERR(xdna, "Partially pinned pages %d/%u", ret, 
>> npages);
>> +                ret = -ENOMEM;
>> +                goto destroy_pages;
>> +            }
>> +        } else {
>>               XDNA_ERR(xdna, "Failed to pin pages ret %d", ret);
>>               goto destroy_pages;
>>           }
>> -
>> -        start += ret;
>>       }
>>         exp_info.ops = &amdxdna_ubuf_dmabuf_ops;
>