Re: [PATCH] drm/amd/pm: Fix memory some memory corruption

2023-06-06 Thread Alex Deucher 
Applied.  Thanks!

Alex

On Tue, Jun 6, 2023 at 6:27 AM Quan, Evan  wrote:
>
> [AMD Official Use Only - General]
>
> Thanks for catching this.
> Reviewed-by: Evan Quan 
>
> > -Original Message-
> > From: Dan Carpenter 
> > Sent: Tuesday, June 6, 2023 4:34 PM
> > To: Quan, Evan 
> > Cc: Deucher, Alexander ; Koenig, Christian
> > ; Pan, Xinhui ; David
> > Airlie ; Daniel Vetter ; Lazar, Lijo
> > ; Zhang, Hawking ; Feng,
> > Kenneth ; Li, Candice ;
> > Chai, Thomas ; Wang, Yang(Kevin)
> > ; Zhang, Horatio ;
> > amd-gfx@lists.freedesktop.org; kernel-janit...@vger.kernel.org
> > Subject: [PATCH] drm/amd/pm: Fix memory some memory corruption
> >
> > The "od_table" is a pointer to a large struct, but this code is doing 
> > pointer
> > math as if it were pointing to bytes.  It results in writing far outside 
> > the struct.
> >
> > Fixes: f0a0c659fb96 ("drm/amd/pm: fulfill the OD support for SMU13.0.0")
> > Fixes: e3afa4f988b3 ("drm/amd/pm: fulfill the OD support for SMU13.0.7")
> > Signed-off-by: Dan Carpenter 
> > ---
> >  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++--
> > drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> > index 5ac5ea770c1c..413e592f0ed6 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> > @@ -1535,7 +1535,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct
> > smu_context *smu,
> >* settings. Thus we do not cache it.
> >*/
> >   offset_of_featurectrlmask = offsetof(OverDriveTable_t,
> > FeatureCtrlMask);
> > - if (memcmp(od_table + offset_of_featurectrlmask,
> > + if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
> >  table_context->user_overdrive_table +
> > offset_of_featurectrlmask,
> >  sizeof(OverDriveTableExternal_t) -
> > offset_of_featurectrlmask)) {
> >   smu_v13_0_0_dump_od_table(smu, od_table); @@ -
> > 1548,7 +1548,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct
> > smu_context *smu,
> >
> >   od_table->OverDriveTable.FeatureCtrlMask = 0;
> >   memcpy(table_context->user_overdrive_table +
> > offset_of_featurectrlmask,
> > -od_table + offset_of_featurectrlmask,
> > +(u8 *)od_table + offset_of_featurectrlmask,
> >  sizeof(OverDriveTableExternal_t) -
> > offset_of_featurectrlmask);
> >
> >   if (!memcmp(table_context->user_overdrive_table,
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > index 0bd086360efa..cda4e818aab7 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > @@ -1524,7 +1524,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct
> > smu_context *smu,
> >* settings. Thus we do not cache it.
> >*/
> >   offset_of_featurectrlmask = offsetof(OverDriveTable_t,
> > FeatureCtrlMask);
> > - if (memcmp(od_table + offset_of_featurectrlmask,
> > + if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
> >  table_context->user_overdrive_table +
> > offset_of_featurectrlmask,
> >  sizeof(OverDriveTableExternal_t) -
> > offset_of_featurectrlmask)) {
> >   smu_v13_0_7_dump_od_table(smu, od_table); @@ -
> > 1537,7 +1537,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct
> > smu_context *smu,
> >
> >   od_table->OverDriveTable.FeatureCtrlMask = 0;
> >   memcpy(table_context->user_overdrive_table +
> > offset_of_featurectrlmask,
> > -od_table + offset_of_featurectrlmask,
> > +(u8 *)od_table + offset_of_featurectrlmask,
> >  sizeof(OverDriveTableExternal_t) -
> > offset_of_featurectrlmask);
> >
> >   if (!memcmp(table_context->user_overdrive_table,
> > --
> > 2.39.2
>

[PATCH] drm/amd/pm: Fix memory some memory corruption

2023-06-06 Thread Dan Carpenter 
The "od_table" is a pointer to a large struct, but this code is doing
pointer math as if it were pointing to bytes.  It results in writing
far outside the struct.

Fixes: f0a0c659fb96 ("drm/amd/pm: fulfill the OD support for SMU13.0.0")
Fixes: e3afa4f988b3 ("drm/amd/pm: fulfill the OD support for SMU13.0.7")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++--
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
index 5ac5ea770c1c..413e592f0ed6 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
@@ -1535,7 +1535,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct 
smu_context *smu,
 * settings. Thus we do not cache it.
 */
offset_of_featurectrlmask = offsetof(OverDriveTable_t, 
FeatureCtrlMask);
-   if (memcmp(od_table + offset_of_featurectrlmask,
+   if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
   table_context->user_overdrive_table + 
offset_of_featurectrlmask,
   sizeof(OverDriveTableExternal_t) - 
offset_of_featurectrlmask)) {
smu_v13_0_0_dump_od_table(smu, od_table);
@@ -1548,7 +1548,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct 
smu_context *smu,
 
od_table->OverDriveTable.FeatureCtrlMask = 0;
memcpy(table_context->user_overdrive_table + 
offset_of_featurectrlmask,
-  od_table + offset_of_featurectrlmask,
+  (u8 *)od_table + offset_of_featurectrlmask,
   sizeof(OverDriveTableExternal_t) - 
offset_of_featurectrlmask);
 
if (!memcmp(table_context->user_overdrive_table,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
index 0bd086360efa..cda4e818aab7 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
@@ -1524,7 +1524,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct 
smu_context *smu,
 * settings. Thus we do not cache it.
 */
offset_of_featurectrlmask = offsetof(OverDriveTable_t, 
FeatureCtrlMask);
-   if (memcmp(od_table + offset_of_featurectrlmask,
+   if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
   table_context->user_overdrive_table + 
offset_of_featurectrlmask,
   sizeof(OverDriveTableExternal_t) - 
offset_of_featurectrlmask)) {
smu_v13_0_7_dump_od_table(smu, od_table);
@@ -1537,7 +1537,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct 
smu_context *smu,
 
od_table->OverDriveTable.FeatureCtrlMask = 0;
memcpy(table_context->user_overdrive_table + 
offset_of_featurectrlmask,
-  od_table + offset_of_featurectrlmask,
+  (u8 *)od_table + offset_of_featurectrlmask,
   sizeof(OverDriveTableExternal_t) - 
offset_of_featurectrlmask);
 
if (!memcmp(table_context->user_overdrive_table,
-- 
2.39.2

RE: [PATCH] drm/amd/pm: Fix memory some memory corruption

2023-06-06 Thread Quan, Evan 
[AMD Official Use Only - General]

Thanks for catching this.
Reviewed-by: Evan Quan 

> -Original Message-
> From: Dan Carpenter 
> Sent: Tuesday, June 6, 2023 4:34 PM
> To: Quan, Evan 
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Pan, Xinhui ; David
> Airlie ; Daniel Vetter ; Lazar, Lijo
> ; Zhang, Hawking ; Feng,
> Kenneth ; Li, Candice ;
> Chai, Thomas ; Wang, Yang(Kevin)
> ; Zhang, Horatio ;
> amd-gfx@lists.freedesktop.org; kernel-janit...@vger.kernel.org
> Subject: [PATCH] drm/amd/pm: Fix memory some memory corruption
>
> The "od_table" is a pointer to a large struct, but this code is doing pointer
> math as if it were pointing to bytes.  It results in writing far outside the 
> struct.
>
> Fixes: f0a0c659fb96 ("drm/amd/pm: fulfill the OD support for SMU13.0.0")
> Fixes: e3afa4f988b3 ("drm/amd/pm: fulfill the OD support for SMU13.0.7")
> Signed-off-by: Dan Carpenter 
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++--
> drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index 5ac5ea770c1c..413e592f0ed6 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -1535,7 +1535,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct
> smu_context *smu,
>* settings. Thus we do not cache it.
>*/
>   offset_of_featurectrlmask = offsetof(OverDriveTable_t,
> FeatureCtrlMask);
> - if (memcmp(od_table + offset_of_featurectrlmask,
> + if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
>  table_context->user_overdrive_table +
> offset_of_featurectrlmask,
>  sizeof(OverDriveTableExternal_t) -
> offset_of_featurectrlmask)) {
>   smu_v13_0_0_dump_od_table(smu, od_table); @@ -
> 1548,7 +1548,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct
> smu_context *smu,
>
>   od_table->OverDriveTable.FeatureCtrlMask = 0;
>   memcpy(table_context->user_overdrive_table +
> offset_of_featurectrlmask,
> -od_table + offset_of_featurectrlmask,
> +(u8 *)od_table + offset_of_featurectrlmask,
>  sizeof(OverDriveTableExternal_t) -
> offset_of_featurectrlmask);
>
>   if (!memcmp(table_context->user_overdrive_table,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> index 0bd086360efa..cda4e818aab7 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> @@ -1524,7 +1524,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct
> smu_context *smu,
>* settings. Thus we do not cache it.
>*/
>   offset_of_featurectrlmask = offsetof(OverDriveTable_t,
> FeatureCtrlMask);
> - if (memcmp(od_table + offset_of_featurectrlmask,
> + if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
>  table_context->user_overdrive_table +
> offset_of_featurectrlmask,
>  sizeof(OverDriveTableExternal_t) -
> offset_of_featurectrlmask)) {
>   smu_v13_0_7_dump_od_table(smu, od_table); @@ -
> 1537,7 +1537,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct
> smu_context *smu,
>
>   od_table->OverDriveTable.FeatureCtrlMask = 0;
>   memcpy(table_context->user_overdrive_table +
> offset_of_featurectrlmask,
> -od_table + offset_of_featurectrlmask,
> +(u8 *)od_table + offset_of_featurectrlmask,
>  sizeof(OverDriveTableExternal_t) -
> offset_of_featurectrlmask);
>
>   if (!memcmp(table_context->user_overdrive_table,
> --
> 2.39.2