All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs
@ 2020-03-10 12:53 Tom St Denis
  2020-03-10 12:54 ` Christian König
  2020-03-11 15:16 ` Alex Deucher
  0 siblings, 2 replies; 5+ messages in thread
From: Tom St Denis @ 2020-03-10 12:53 UTC (permalink / raw)
  To: amd-gfx; +Cc: Tom St Denis

The offset into the array was specified in bytes but should
be in terms of 32-bit words.  Also prevent large reads that
would also cause a buffer overread.

Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index c573edf02afc..e0f4ccd91fd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -783,11 +783,11 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
 	ssize_t result = 0;
 	uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
 
-	if (size & 3 || *pos & 3)
+	if (size > 4096 || size & 3 || *pos & 3)
 		return -EINVAL;
 
 	/* decode offset */
-	offset = *pos & GENMASK_ULL(11, 0);
+	offset = (*pos & GENMASK_ULL(11, 0)) / 4;
 	se = (*pos & GENMASK_ULL(19, 12)) >> 12;
 	sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
 	cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
-- 
2.24.1

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs
  2020-03-10 12:53 [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs Tom St Denis
@ 2020-03-10 12:54 ` Christian König
  2020-03-11 15:16 ` Alex Deucher
  1 sibling, 0 replies; 5+ messages in thread
From: Christian König @ 2020-03-10 12:54 UTC (permalink / raw)
  To: Tom St Denis, amd-gfx

Am 10.03.20 um 13:53 schrieb Tom St Denis:
> The offset into the array was specified in bytes but should
> be in terms of 32-bit words.  Also prevent large reads that
> would also cause a buffer overread.
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index c573edf02afc..e0f4ccd91fd4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -783,11 +783,11 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
>   	ssize_t result = 0;
>   	uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
>   
> -	if (size & 3 || *pos & 3)
> +	if (size > 4096 || size & 3 || *pos & 3)
>   		return -EINVAL;
>   
>   	/* decode offset */
> -	offset = *pos & GENMASK_ULL(11, 0);
> +	offset = (*pos & GENMASK_ULL(11, 0)) / 4;
>   	se = (*pos & GENMASK_ULL(19, 12)) >> 12;
>   	sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
>   	cu = (*pos & GENMASK_ULL(35, 28)) >> 28;

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs
  2020-03-10 12:53 [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs Tom St Denis
  2020-03-10 12:54 ` Christian König
@ 2020-03-11 15:16 ` Alex Deucher
  2020-03-11 16:58   ` Tom St Denis
  2020-03-11 20:34   ` Tom St Denis
  1 sibling, 2 replies; 5+ messages in thread
From: Alex Deucher @ 2020-03-11 15:16 UTC (permalink / raw)
  To: Tom St Denis; +Cc: amd-gfx list

On Tue, Mar 10, 2020 at 8:53 AM Tom St Denis <tom.stdenis@amd.com> wrote:
>
> The offset into the array was specified in bytes but should
> be in terms of 32-bit words.  Also prevent large reads that
> would also cause a buffer overread.
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index c573edf02afc..e0f4ccd91fd4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -783,11 +783,11 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
>         ssize_t result = 0;
>         uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
>
> -       if (size & 3 || *pos & 3)
> +       if (size > 4096 || size & 3 || *pos & 3)

Is size in dwords as well?

Alex

>                 return -EINVAL;
>
>         /* decode offset */
> -       offset = *pos & GENMASK_ULL(11, 0);
> +       offset = (*pos & GENMASK_ULL(11, 0)) / 4;
>         se = (*pos & GENMASK_ULL(19, 12)) >> 12;
>         sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
>         cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
> --
> 2.24.1
>
> _______________________________________________
> 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] 5+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs
  2020-03-11 15:16 ` Alex Deucher
@ 2020-03-11 16:58   ` Tom St Denis
  2020-03-11 20:34   ` Tom St Denis
  1 sibling, 0 replies; 5+ messages in thread
From: Tom St Denis @ 2020-03-11 16:58 UTC (permalink / raw)
  To: Alex Deucher; +Cc: amd-gfx list


On 2020-03-11 11:16 a.m., Alex Deucher wrote:
> On Tue, Mar 10, 2020 at 8:53 AM Tom St Denis <tom.stdenis@amd.com> wrote:
>> The offset into the array was specified in bytes but should
>> be in terms of 32-bit words.  Also prevent large reads that
>> would also cause a buffer overread.
>>
>> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> index c573edf02afc..e0f4ccd91fd4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> @@ -783,11 +783,11 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
>>          ssize_t result = 0;
>>          uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
>>
>> -       if (size & 3 || *pos & 3)
>> +       if (size > 4096 || size & 3 || *pos & 3)
> Is size in dwords as well?

Nope it's in bytes (as per the calling convention standards as well as 
later in the function we subtract 4 from it repeatedly).


Tom



>
> Alex
>
>>                  return -EINVAL;
>>
>>          /* decode offset */
>> -       offset = *pos & GENMASK_ULL(11, 0);
>> +       offset = (*pos & GENMASK_ULL(11, 0)) / 4;
>>          se = (*pos & GENMASK_ULL(19, 12)) >> 12;
>>          sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
>>          cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
>> --
>> 2.24.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Ctom.stdenis%40amd.com%7C550ca4aaaf084c3d7a9f08d7c5cf2c65%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637195365897948298&amp;sdata=YTb2YGBlAlDS%2FffaVOo2Yrej21N%2FJYVpFVIc1rQERWg%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs
  2020-03-11 15:16 ` Alex Deucher
  2020-03-11 16:58   ` Tom St Denis
@ 2020-03-11 20:34   ` Tom St Denis
  1 sibling, 0 replies; 5+ messages in thread
From: Tom St Denis @ 2020-03-11 20:34 UTC (permalink / raw)
  To: Alex Deucher; +Cc: amd-gfx list

Hi Alex,

I sent out a v2 of the patch to the list that also addresses the fact we 
were reading from the wrong offset from the internal buffer.

This entry was really only tested with offset==0 which is why this 
didn't come up until now that people want those trap registers :-)

Tom

On 2020-03-11 11:16 a.m., Alex Deucher wrote:
> On Tue, Mar 10, 2020 at 8:53 AM Tom St Denis <tom.stdenis@amd.com> wrote:
>> The offset into the array was specified in bytes but should
>> be in terms of 32-bit words.  Also prevent large reads that
>> would also cause a buffer overread.
>>
>> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> index c573edf02afc..e0f4ccd91fd4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> @@ -783,11 +783,11 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
>>          ssize_t result = 0;
>>          uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
>>
>> -       if (size & 3 || *pos & 3)
>> +       if (size > 4096 || size & 3 || *pos & 3)
> Is size in dwords as well?
>
> Alex
>
>>                  return -EINVAL;
>>
>>          /* decode offset */
>> -       offset = *pos & GENMASK_ULL(11, 0);
>> +       offset = (*pos & GENMASK_ULL(11, 0)) / 4;
>>          se = (*pos & GENMASK_ULL(19, 12)) >> 12;
>>          sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
>>          cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
>> --
>> 2.24.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Ctom.stdenis%40amd.com%7C550ca4aaaf084c3d7a9f08d7c5cf2c65%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637195365897948298&amp;sdata=YTb2YGBlAlDS%2FffaVOo2Yrej21N%2FJYVpFVIc1rQERWg%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-11 20:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-10 12:53 [PATCH] drm/amd/amdgpu: Fix GPR read from debugfs Tom St Denis
2020-03-10 12:54 ` Christian König
2020-03-11 15:16 ` Alex Deucher
2020-03-11 16:58   ` Tom St Denis
2020-03-11 20:34   ` Tom St Denis

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.