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

[Christian König]

Am 29.03.2017 um 15:29 schrieb Alex Deucher:
> On Wed, Mar 29, 2017 at 8:36 AM, Christian König
> <deathsimple at vodafone.de> wrote:
>> 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.
> Well, it's zeroed memory, so it's not random.  They just return 0.

No, the IOCTL is using kmalloc_array, not kcalloc or kzalloc.

That's why I stumbled over it in the first place.

>
>> That is strange at best, but I can't figure out the background here.
> No idea why they were added separately.  AFAIK, those indices are not used.

Dare to put an rb or ack-by on it?

Christian.

>
> Alex
>
>> 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
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx