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