[PATCH] drm/amdgpu: just disallow reading untouched registers

[Christian König]

Am 29.03.2017 um 14:28 schrieb Tom St Denis:
> On 29/03/17 08:18 AM, Christian König wrote:
>> Ping!
>>
>> Does anybody of you guys know what the background of those "untouched"
>> registers is?
>>
>> That we leak uninitialized memory to userspace is a bit bad.
>
> I don't know the history but wouldn't reading these just read the MMIO 
> registers?  Or these are cached and we're reading kernel mem?

The crux is we actually don't read them, but just ignore them.

In other words we copy back random kernel memory to userspace because we 
didn't initialize the returned value.

That is strange at best, but I can't figure out the background here.

Christian.

>
> It seems sensible to remove them though and the patch looks clean so 
> I'll give an ack-by.
>
> Tom
>
>>
>> Thanks,
>> Christian.
>>
>> Am 28.03.2017 um 13:24 schrieb Christian König:
>>> From: Christian König <christian.koenig at amd.com>
>>>
>>> Not sure what the original intention was here, but returning a random
>>> piece of
>>> kernel memory to userspace because we didn't set the value at all is
>>> clearly
>>> not a good idea.
>>>
>>> This patch disallows reading the register and returns
>>> a proper error code instead.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/vi.c | 6 ------
>>>   1 file changed, 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
>>> b/drivers/gpu/drm/amd/amdgpu/vi.c
>>> index 5c02ec4..f1c2bff 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
>>> @@ -465,15 +465,9 @@ static void vi_detect_hw_virtualization(struct
>>> amdgpu_device *adev)
>>>   }
>>>     static const struct amdgpu_allowed_register_entry
>>> tonga_allowed_read_registers[] = {
>>> -    {mmGB_MACROTILE_MODE7, true},
>>>   };
>>>     static const struct amdgpu_allowed_register_entry
>>> cz_allowed_read_registers[] = {
>>> -    {mmGB_TILE_MODE7, true},
>>> -    {mmGB_TILE_MODE12, true},
>>> -    {mmGB_TILE_MODE17, true},
>>> -    {mmGB_TILE_MODE23, true},
>>> -    {mmGB_MACROTILE_MODE7, true},
>>>   };
>>>     static const struct amdgpu_allowed_register_entry
>>> vi_allowed_read_registers[] = {
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx