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

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@xxxxxxx>
> Sent: Monday, August 23, 2021 3:02 PM
> To: Kees Cook <keescook@xxxxxxxxxxxx>; Lazar, Lijo
> <Lijo.Lazar@xxxxxxx>
> Cc: Pan, Xinhui <Xinhui.Pan@xxxxxxx>; David Airlie <airlied@xxxxxxxx>;
> Daniel Vetter <daniel@xxxxxxxx>; Zhang, Hawking
> <Hawking.Zhang@xxxxxxx>; Xu, Feifei <Feifei.Xu@xxxxxxx>; Gao, Likun
> <Likun.Gao@xxxxxxx>; Gu, JiaWei (Will) <JiaWei.Gu@xxxxxxx>; Quan,
> Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-
> devel@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Tuikov, Luben
> <Luben.Tuikov@xxxxxxx>; Grodzovsky, Andrey
> <Andrey.Grodzovsky@xxxxxxx>; Li, Dennis <Dennis.Li@xxxxxxx>;
> Sundararaju, Sathishkumar <Sathishkumar.Sundararaju@xxxxxxx>; Kim,
> Jonathan <Jonathan.Kim@xxxxxxx>; Wang, Kevin(Yang)
> <Kevin1.Wang@xxxxxxx>; Nieto, David M <David.Nieto@xxxxxxx>; Feng,
> Kenneth <Kenneth.Feng@xxxxxxx>; Lee Jones <lee.jones@xxxxxxxxxx>;
> Clements, John <John.Clements@xxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> linux-hardening@xxxxxxxxxxxxxxx
> 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@xxxxxxx> 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