[PATCH 2/2] drm/amdgpu/psp: prevent page fault by checking write_frame address(v3) - Prevent a possible buffer overflow when updating the ring buffer by bounds checking the command frame against the available space in the ring buffer.

[Deucher, Alexander]

> -----Original Message-----
> From: Evan Quan [mailto:evan.quan at amd.com]
> Sent: Friday, October 20, 2017 4:38 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: Zhu, Rex; Deucher, Alexander; Quan, Evan
> Subject: [PATCH 2/2] drm/amdgpu/psp: prevent page fault by checking
> write_frame address(v3) 

Need a new line between the subject and the description.  With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

- Prevent a possible buffer overflow when updating
> the ring buffer by bounds checking the command frame against the available
> space in the ring buffer.
> 
>  v2: update the ring_buffer_end address
>  v3: update the commit log
> 
> Change-Id: If3b79428b32ffab57b4e75f9c20c2b2d7e600223
> Signed-off-by: Evan Quan <evan.quan at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 16 ++++++++++++++--
>  drivers/gpu/drm/amd/amdgpu/psp_v3_1.c  | 16 ++++++++++++++--
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index dea7c90..26e10ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -257,6 +257,9 @@ int psp_v10_0_cmd_submit(struct psp_context *psp,
>  	unsigned int psp_write_ptr_reg = 0;
>  	struct psp_gfx_rb_frame * write_frame = psp->km_ring.ring_mem;
>  	struct psp_ring *ring = &psp->km_ring;
> +	struct psp_gfx_rb_frame *ring_buffer_start = ring->ring_mem;
> +	struct psp_gfx_rb_frame *ring_buffer_end = ring_buffer_start +
> +		ring->ring_size / sizeof(struct psp_gfx_rb_frame) - 1;
>  	struct amdgpu_device *adev = psp->adev;
>  	uint32_t ring_size_dw = ring->ring_size / 4;
>  	uint32_t rb_frame_size_dw = sizeof(struct psp_gfx_rb_frame) / 4;
> @@ -266,9 +269,18 @@ int psp_v10_0_cmd_submit(struct psp_context
> *psp,
> 
>  	/* Update KM RB frame pointer to new frame */
>  	if ((psp_write_ptr_reg % ring_size_dw) == 0)
> -		write_frame = ring->ring_mem;
> +		write_frame = ring_buffer_start;
>  	else
> -		write_frame = ring->ring_mem + (psp_write_ptr_reg /
> rb_frame_size_dw);
> +		write_frame = ring_buffer_start + (psp_write_ptr_reg /
> rb_frame_size_dw);
> +	/* Check invalid write_frame ptr address */
> +	if ((write_frame < ring_buffer_start) || (ring_buffer_end <
> write_frame)) {
> +		DRM_ERROR("ring_buffer_start = %x; ring_buffer_end = %x;
> write_frame = %x\n",
> +				ring_buffer_start,
> +				ring_buffer_end,
> +				write_frame);
> +		DRM_ERROR("write_frame is pointing to address out of
> bounds\n");
> +		return -EINVAL;
> +	}
> 
>  	/* Initialize KM RB frame */
>  	memset(write_frame, 0, sizeof(struct psp_gfx_rb_frame));
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> index cee5c39..d217050 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> @@ -367,6 +367,9 @@ int psp_v3_1_cmd_submit(struct psp_context *psp,
>  	unsigned int psp_write_ptr_reg = 0;
>  	struct psp_gfx_rb_frame * write_frame = psp->km_ring.ring_mem;
>  	struct psp_ring *ring = &psp->km_ring;
> +	struct psp_gfx_rb_frame *ring_buffer_start = ring->ring_mem;
> +	struct psp_gfx_rb_frame *ring_buffer_end = ring_buffer_start +
> +		ring->ring_size / sizeof(struct psp_gfx_rb_frame) - 1;
>  	struct amdgpu_device *adev = psp->adev;
>  	uint32_t ring_size_dw = ring->ring_size / 4;
>  	uint32_t rb_frame_size_dw = sizeof(struct psp_gfx_rb_frame) / 4;
> @@ -378,9 +381,18 @@ int psp_v3_1_cmd_submit(struct psp_context *psp,
>  	/* write_frame ptr increments by size of rb_frame in bytes */
>  	/* psp_write_ptr_reg increments by size of rb_frame in DWORDs */
>  	if ((psp_write_ptr_reg % ring_size_dw) == 0)
> -		write_frame = ring->ring_mem;
> +		write_frame = ring_buffer_start;
>  	else
> -		write_frame = ring->ring_mem + (psp_write_ptr_reg /
> rb_frame_size_dw);
> +		write_frame = ring_buffer_start + (psp_write_ptr_reg /
> rb_frame_size_dw);
> +	/* Check invalid write_frame ptr address */
> +	if ((write_frame < ring_buffer_start) || (ring_buffer_end <
> write_frame)) {
> +		DRM_ERROR("ring_buffer_start = %x; ring_buffer_end = %x;
> write_frame = %x\n",
> +				ring_buffer_start,
> +				ring_buffer_end,
> +				write_frame);
> +		DRM_ERROR("write_frame is pointing to address out of
> bounds\n");
> +		return -EINVAL;
> +	}
> 
>  	/* Initialize KM RB frame */
>  	memset(write_frame, 0, sizeof(struct psp_gfx_rb_frame));
> --
> 2.7.4