[PATCH libdrm 1/2] amdgpu: prevent an integer wraparound of cpu_map_count

[Zhang, Jerry(Junwei)]

On 10/24/18 3:07 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>

We need commit log and sign-off here.

BTW, have you encounter any issue about that?

>
> ---
>   amdgpu/amdgpu_bo.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index c0f42e81..81f8a5f7 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -22,20 +22,21 @@
>    *
>    */
>   
>   #include <stdlib.h>
>   #include <stdio.h>
>   #include <stdint.h>
>   #include <string.h>
>   #include <errno.h>
>   #include <fcntl.h>
>   #include <unistd.h>
> +#include <limits.h>
>   #include <sys/ioctl.h>
>   #include <sys/mman.h>
>   #include <sys/time.h>
>   
>   #include "libdrm_macros.h"
>   #include "xf86drm.h"
>   #include "amdgpu_drm.h"
>   #include "amdgpu_internal.h"
>   #include "util_math.h"
>   
> @@ -442,21 +443,29 @@ drm_public int amdgpu_bo_cpu_map(amdgpu_bo_handle bo, void **cpu)
>   {
>   	union drm_amdgpu_gem_mmap args;
>   	void *ptr;
>   	int r;
>   
>   	pthread_mutex_lock(&bo->cpu_access_mutex);
>   
>   	if (bo->cpu_ptr) {
>   		/* already mapped */
>   		assert(bo->cpu_map_count > 0);
> -		bo->cpu_map_count++;
> +
> +		/* If the counter has already reached INT_MAX, don't increment
> +		 * it and assume that the buffer will be mapped indefinitely.
> +		 * The buffer is pretty unlikely to get unmapped by the user
> +		 * at this point.
> +		 */
> +		if (bo->cpu_map_count != INT_MAX)
> +			bo->cpu_map_count++;

If so, shall we print some error here to notice that indefinite mappings 
come up.

Regards,
Jerry
> +
>   		*cpu = bo->cpu_ptr;
>   		pthread_mutex_unlock(&bo->cpu_access_mutex);
>   		return 0;
>   	}
>   
>   	assert(bo->cpu_map_count == 0);
>   
>   	memset(&args, 0, sizeof(args));
>   
>   	/* Query the buffer address (args.addr_ptr).
> @@ -492,21 +501,27 @@ drm_public int amdgpu_bo_cpu_unmap(amdgpu_bo_handle bo)
>   
>   	pthread_mutex_lock(&bo->cpu_access_mutex);
>   	assert(bo->cpu_map_count >= 0);
>   
>   	if (bo->cpu_map_count == 0) {
>   		/* not mapped */
>   		pthread_mutex_unlock(&bo->cpu_access_mutex);
>   		return -EINVAL;
>   	}
>   
> -	bo->cpu_map_count--;
> +	/* If the counter has already reached INT_MAX, don't decrement it.
> +	 * This is because amdgpu_bo_cpu_map doesn't increment it past
> +	 * INT_MAX.
> +	 */
> +	if (bo->cpu_map_count != INT_MAX)
> +		bo->cpu_map_count--;
> +
>   	if (bo->cpu_map_count > 0) {
>   		/* mapped multiple times */
>   		pthread_mutex_unlock(&bo->cpu_access_mutex);
>   		return 0;
>   	}
>   
>   	r = drm_munmap(bo->cpu_ptr, bo->alloc_size) == 0 ? 0 : -errno;
>   	bo->cpu_ptr = NULL;
>   	pthread_mutex_unlock(&bo->cpu_access_mutex);
>   	return r;