All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu/psp: prevent page fault by checking write_frame address
@ 2017-10-16  9:09 Evan Quan
       [not found] ` <1508144957-11529-1-git-send-email-evan.quan-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Evan Quan @ 2017-10-16  9:09 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Evan Quan

Change-Id: If3b79428b32ffab57b4e75f9c20c2b2d7e600223
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 31 ++++++++++++++++++++-----------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c  | 31 ++++++++++++++++++++-----------
 2 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 77cab1f..487e17b 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -255,10 +255,10 @@ int psp_v10_0_cmd_submit(struct psp_context *psp,
 		        int index)
 {
 	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 = psp->km_ring.ring_mem;
+	struct psp_gfx_rb_frame *write_frame_ptr;
 	struct amdgpu_device *adev = psp->adev;
-	uint32_t ring_size_dw = ring->ring_size / 4;
+	uint32_t ring_size_dw = psp->km_ring.ring_size / 4;
 	uint32_t rb_frame_size_dw = sizeof(struct psp_gfx_rb_frame) / 4;
 
 	/* KM (GPCOM) prepare write pointer */
@@ -266,19 +266,28 @@ 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_ptr = ring_buffer_start;
 	else
-		write_frame = ring->ring_mem + (psp_write_ptr_reg / rb_frame_size_dw);
+		write_frame_ptr = ring_buffer_start + (psp_write_ptr_reg / rb_frame_size_dw);
+	if ((write_frame_ptr < ring_buffer_start) ||
+		((ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame)) < write_frame_ptr)) {
+		DRM_ERROR("ring_buffer_start = %x; ring_buffer_end = %x; write_frame_ptr = %x\n",
+				ring_buffer_start,
+				ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame),
+				write_frame_ptr);
+		DRM_ERROR("write_frame_ptr is pointing to address out of bounds\n");
+		return -EINVAL;
+	}
 
 	/* Initialize KM RB frame */
-	memset(write_frame, 0, sizeof(struct psp_gfx_rb_frame));
+	memset(write_frame_ptr, 0, sizeof(struct psp_gfx_rb_frame));
 
 	/* Update KM RB frame */
-	write_frame->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
-	write_frame->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
-	write_frame->fence_addr_hi = upper_32_bits(fence_mc_addr);
-	write_frame->fence_addr_lo = lower_32_bits(fence_mc_addr);
-	write_frame->fence_value = index;
+	write_frame_ptr->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
+	write_frame_ptr->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
+	write_frame_ptr->fence_addr_hi = upper_32_bits(fence_mc_addr);
+	write_frame_ptr->fence_addr_lo = lower_32_bits(fence_mc_addr);
+	write_frame_ptr->fence_value = index;
 
 	/* Update the write Pointer in DWORDs */
 	psp_write_ptr_reg = (psp_write_ptr_reg + rb_frame_size_dw) % ring_size_dw;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index bcbe30d..ec472e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -365,10 +365,10 @@ int psp_v3_1_cmd_submit(struct psp_context *psp,
 		        int index)
 {
 	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 = psp->km_ring.ring_mem;
+	struct psp_gfx_rb_frame *write_frame_ptr;
 	struct amdgpu_device *adev = psp->adev;
-	uint32_t ring_size_dw = ring->ring_size / 4;
+	uint32_t ring_size_dw = psp->km_ring.ring_size / 4;
 	uint32_t rb_frame_size_dw = sizeof(struct psp_gfx_rb_frame) / 4;
 
 	/* KM (GPCOM) prepare write pointer */
@@ -378,19 +378,28 @@ 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_ptr = ring_buffer_start;
 	else
-		write_frame = ring->ring_mem + (psp_write_ptr_reg / rb_frame_size_dw);
+		write_frame_ptr = ring_buffer_start + (psp_write_ptr_reg / rb_frame_size_dw);
+	if ((write_frame_ptr < ring_buffer_start) ||
+		((ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame)) < write_frame_ptr)) {
+		DRM_ERROR("ring_buffer_start = %x; ring_buffer_end = %x; write_frame_ptr = %x\n",
+				ring_buffer_start,
+				ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame),
+				write_frame_ptr);
+		DRM_ERROR("write_frame_ptr is pointing to address out of bounds\n");
+		return -EINVAL;
+	}
 
 	/* Initialize KM RB frame */
-	memset(write_frame, 0, sizeof(struct psp_gfx_rb_frame));
+	memset(write_frame_ptr, 0, sizeof(struct psp_gfx_rb_frame));
 
 	/* Update KM RB frame */
-	write_frame->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
-	write_frame->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
-	write_frame->fence_addr_hi = upper_32_bits(fence_mc_addr);
-	write_frame->fence_addr_lo = lower_32_bits(fence_mc_addr);
-	write_frame->fence_value = index;
+	write_frame_ptr->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
+	write_frame_ptr->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
+	write_frame_ptr->fence_addr_hi = upper_32_bits(fence_mc_addr);
+	write_frame_ptr->fence_addr_lo = lower_32_bits(fence_mc_addr);
+	write_frame_ptr->fence_value = index;
 
 	/* Update the write Pointer in DWORDs */
 	psp_write_ptr_reg = (psp_write_ptr_reg + rb_frame_size_dw) % ring_size_dw;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/amdgpu/psp: prevent page fault by checking write_frame address
       [not found] ` <1508144957-11529-1-git-send-email-evan.quan-5C7GfCeVMHo@public.gmane.org>
@ 2017-10-16 15:10   ` Alex Deucher
  0 siblings, 0 replies; 2+ messages in thread
From: Alex Deucher @ 2017-10-16 15:10 UTC (permalink / raw)
  To: Evan Quan; +Cc: amd-gfx list

On Mon, Oct 16, 2017 at 5:09 AM, Evan Quan <evan.quan@amd.com> wrote:
> Change-Id: If3b79428b32ffab57b4e75f9c20c2b2d7e600223
> Signed-off-by: Evan Quan <evan.quan@amd.com>

Please provide a more detailed commit message.  Seems like we should
look at converting the psp to use the common ring helpers to avoid
issues like this is the future.  At the very least a ring_alloc/commit
sort of interface.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 31 ++++++++++++++++++++-----------
>  drivers/gpu/drm/amd/amdgpu/psp_v3_1.c  | 31 ++++++++++++++++++++-----------
>  2 files changed, 40 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index 77cab1f..487e17b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -255,10 +255,10 @@ int psp_v10_0_cmd_submit(struct psp_context *psp,
>                         int index)
>  {
>         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 = psp->km_ring.ring_mem;
> +       struct psp_gfx_rb_frame *write_frame_ptr;
>         struct amdgpu_device *adev = psp->adev;
> -       uint32_t ring_size_dw = ring->ring_size / 4;
> +       uint32_t ring_size_dw = psp->km_ring.ring_size / 4;
>         uint32_t rb_frame_size_dw = sizeof(struct psp_gfx_rb_frame) / 4;
>
>         /* KM (GPCOM) prepare write pointer */
> @@ -266,19 +266,28 @@ 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_ptr = ring_buffer_start;
>         else
> -               write_frame = ring->ring_mem + (psp_write_ptr_reg / rb_frame_size_dw);
> +               write_frame_ptr = ring_buffer_start + (psp_write_ptr_reg / rb_frame_size_dw);
> +       if ((write_frame_ptr < ring_buffer_start) ||
> +               ((ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame)) < write_frame_ptr)) {
> +               DRM_ERROR("ring_buffer_start = %x; ring_buffer_end = %x; write_frame_ptr = %x\n",
> +                               ring_buffer_start,
> +                               ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame),
> +                               write_frame_ptr);
> +               DRM_ERROR("write_frame_ptr is pointing to address out of bounds\n");
> +               return -EINVAL;
> +       }
>
>         /* Initialize KM RB frame */
> -       memset(write_frame, 0, sizeof(struct psp_gfx_rb_frame));
> +       memset(write_frame_ptr, 0, sizeof(struct psp_gfx_rb_frame));
>
>         /* Update KM RB frame */
> -       write_frame->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
> -       write_frame->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
> -       write_frame->fence_addr_hi = upper_32_bits(fence_mc_addr);
> -       write_frame->fence_addr_lo = lower_32_bits(fence_mc_addr);
> -       write_frame->fence_value = index;
> +       write_frame_ptr->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
> +       write_frame_ptr->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
> +       write_frame_ptr->fence_addr_hi = upper_32_bits(fence_mc_addr);
> +       write_frame_ptr->fence_addr_lo = lower_32_bits(fence_mc_addr);
> +       write_frame_ptr->fence_value = index;
>
>         /* Update the write Pointer in DWORDs */
>         psp_write_ptr_reg = (psp_write_ptr_reg + rb_frame_size_dw) % ring_size_dw;
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> index bcbe30d..ec472e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> @@ -365,10 +365,10 @@ int psp_v3_1_cmd_submit(struct psp_context *psp,
>                         int index)
>  {
>         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 = psp->km_ring.ring_mem;
> +       struct psp_gfx_rb_frame *write_frame_ptr;
>         struct amdgpu_device *adev = psp->adev;
> -       uint32_t ring_size_dw = ring->ring_size / 4;
> +       uint32_t ring_size_dw = psp->km_ring.ring_size / 4;
>         uint32_t rb_frame_size_dw = sizeof(struct psp_gfx_rb_frame) / 4;
>
>         /* KM (GPCOM) prepare write pointer */
> @@ -378,19 +378,28 @@ 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_ptr = ring_buffer_start;
>         else
> -               write_frame = ring->ring_mem + (psp_write_ptr_reg / rb_frame_size_dw);
> +               write_frame_ptr = ring_buffer_start + (psp_write_ptr_reg / rb_frame_size_dw);
> +       if ((write_frame_ptr < ring_buffer_start) ||
> +               ((ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame)) < write_frame_ptr)) {
> +               DRM_ERROR("ring_buffer_start = %x; ring_buffer_end = %x; write_frame_ptr = %x\n",
> +                               ring_buffer_start,
> +                               ring_buffer_start + psp->km_ring.ring_size / sizeof(struct psp_gfx_rb_frame),
> +                               write_frame_ptr);
> +               DRM_ERROR("write_frame_ptr is pointing to address out of bounds\n");
> +               return -EINVAL;
> +       }
>
>         /* Initialize KM RB frame */
> -       memset(write_frame, 0, sizeof(struct psp_gfx_rb_frame));
> +       memset(write_frame_ptr, 0, sizeof(struct psp_gfx_rb_frame));
>
>         /* Update KM RB frame */
> -       write_frame->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
> -       write_frame->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
> -       write_frame->fence_addr_hi = upper_32_bits(fence_mc_addr);
> -       write_frame->fence_addr_lo = lower_32_bits(fence_mc_addr);
> -       write_frame->fence_value = index;
> +       write_frame_ptr->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
> +       write_frame_ptr->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
> +       write_frame_ptr->fence_addr_hi = upper_32_bits(fence_mc_addr);
> +       write_frame_ptr->fence_addr_lo = lower_32_bits(fence_mc_addr);
> +       write_frame_ptr->fence_value = index;
>
>         /* Update the write Pointer in DWORDs */
>         psp_write_ptr_reg = (psp_write_ptr_reg + rb_frame_size_dw) % ring_size_dw;
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-10-16 15:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16  9:09 [PATCH] drm/amdgpu/psp: prevent page fault by checking write_frame address Evan Quan
     [not found] ` <1508144957-11529-1-git-send-email-evan.quan-5C7GfCeVMHo@public.gmane.org>
2017-10-16 15:10   ` Alex Deucher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.