[PATCH] drm/amd/pm: And destination bounds checking to struct copy

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Monday, August 23, 2021 3:02 PM
> To: Kees Cook <keescook at chromium.org>; Lazar, Lijo
> <Lijo.Lazar at amd.com>
> Cc: Pan, Xinhui <Xinhui.Pan at amd.com>; David Airlie <airlied at linux.ie>;
> Daniel Vetter <daniel at ffwll.ch>; Zhang, Hawking
> <Hawking.Zhang at amd.com>; Xu, Feifei <Feifei.Xu at amd.com>; Gao, Likun
> <Likun.Gao at amd.com>; Gu, JiaWei (Will) <JiaWei.Gu at amd.com>; Quan,
> Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org; dri-
> devel at lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Tuikov, Luben
> <Luben.Tuikov at amd.com>; Grodzovsky, Andrey
> <Andrey.Grodzovsky at amd.com>; Li, Dennis <Dennis.Li at amd.com>;
> Sundararaju, Sathishkumar <Sathishkumar.Sundararaju at amd.com>; Kim,
> Jonathan <Jonathan.Kim at amd.com>; Wang, Kevin(Yang)
> <Kevin1.Wang at amd.com>; Nieto, David M <David.Nieto at amd.com>; Feng,
> Kenneth <Kenneth.Feng at amd.com>; Lee Jones <lee.jones at linaro.org>;
> Clements, John <John.Clements at amd.com>; linux-kernel at vger.kernel.org;
> linux-hardening at vger.kernel.org
> Subject: Re: [PATCH] drm/amd/pm: And destination bounds checking to
> struct copy
> 
> Am 23.08.21 um 16:23 schrieb Kees Cook:
> >
> > On August 22, 2021 11:28:54 PM PDT, "Christian König"
> <christian.koenig at amd.com> wrote:
> >>
> >> Am 19.08.21 um 22:14 schrieb Kees Cook:
> >>> [...]
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> index 96e895d6be35..4605934a4fb7 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> @@ -1446,4 +1446,29 @@ static inline int amdgpu_in_reset(struct
> amdgpu_device *adev)
> >>>    {
> >>>    	return atomic_read(&adev->in_gpu_reset);
> >>>    }
> >>> +
> >>> +/**
> >>> + * memcpy_trailing - Copy the end of one structure into the middle
> >>> +of another
> >>> + *
> >>> + * @dst: Pointer to destination struct
> >>> + * @first_dst_member: The member name in @dst where the
> overwrite
> >>> +begins
> >>> + * @last_dst_member: The member name in @dst where the
> overwrite
> >>> +ends after
> >>> + * @src: Pointer to the source struct
> >>> + * @first_src_member: The member name in @src where the copy
> begins
> >>> + *
> >>> + */
> >>> +#define memcpy_trailing(dst, first_dst_member, last_dst_member,
> 		   \
> >>> +		        src, first_src_member)				   \
> >> Please don't add a function like this into amdgpu.h, especially when
> >> it is only used by the SMU code.
> > Sure, I'm happy to move it. It wasn't clear to me which headers were
> considered "immutable". Which header should I put this in?
> 
> I think amdgpu_smuio.h, but I'm not 100% sure. Alex do you have a better
> idea?
> 

No, that's for the SMUIO callbacks for the IP block.  Please use drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h

Alex

> We don't want to put anything new into amdgpu.h any more since this is
> basically only a legacy leftover.
> 
> Thanks,
> Christian.
> 
> >
> >> And please give it an amdgpu_ prefix so that we are not confusing it
> >> with a core function.
> > Sure, I will include that.
> >
> >> Apart from that looks good to me.
> > Thanks!
> >
> > -Kees