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

From: "Deucher, Alexander" <Alexander.Deucher@amd.com>
To: "Koenig, Christian" <Christian.Koenig@amd.com>,
	Kees Cook <keescook@chromium.org>,
	"Lazar, Lijo" <Lijo.Lazar@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	"Zhang, Hawking" <Hawking.Zhang@amd.com>,
	"Xu, Feifei" <Feifei.Xu@amd.com>,
	"Gao, Likun" <Likun.Gao@amd.com>,
	"Gu, JiaWei (Will)" <JiaWei.Gu@amd.com>,
	"Quan, Evan" <Evan.Quan@amd.com>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"Tuikov, Luben" <Luben.Tuikov@amd.com>,
	"Grodzovsky, Andrey" <Andrey.Grodzovsky@amd.com>,
	"Li, Dennis" <Dennis.Li@amd.com>,
	"Sundararaju, Sathishkumar" <Sathishkumar.Sundararaju@amd.com>,
	"Kim, Jonathan" <Jonathan.Kim@amd.com>,
	"Wang, Kevin(Yang)" <Kevin1.Wang@amd.com>,
	"Nieto, David M" <David.Nieto@amd.com>,
	"Feng, Kenneth" <Kenneth.Feng@amd.com>,
	Lee Jones <lee.jones@linaro.org>,
	"Clements, John" <John.Clements@amd.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-hardening@vger.kernel.org"
	<linux-hardening@vger.kernel.org>
Subject: RE: [PATCH] drm/amd/pm: And destination bounds checking to struct copy
Date: Mon, 23 Aug 2021 19:13:43 +0000	[thread overview]
Message-ID: <BL1PR12MB514453053481613D46D64470F7C49@BL1PR12MB5144.namprd12.prod.outlook.com> (raw)
In-Reply-To: <07a77a6c-f754-c676-5063-72ad418351d5@amd.com>

[Public]

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