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

[Alex Deucher]

On Wed, Mar 29, 2017 at 9:46 AM, Christian König
<deathsimple at vodafone.de> wrote:
> 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.

But the register is read from the cached copy which is zeroed.

>
>>
>>> 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?

Acked-by: Alex Deucher <alexander.deucher at amd.com>

>
> 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
>
>
>