From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Christian_K=c3=b6nig?= Subject: Re: [PATCH] drm/amdgpu/pm: Remove VLA usage Date: Tue, 17 Jul 2018 08:58:21 +0200 Message-ID: <42ba5486-34a2-02bd-50a1-12314e6e2755@gmail.com> References: <20180620182647.GA24405@beast> Reply-To: christian.koenig-5C7GfCeVMHo@public.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1584869283==" Return-path: In-Reply-To: Content-Language: en-US List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: "Zhu, Rex" , Kees Cook , "Deucher, Alexander" Cc: "Zhou, David(ChunMing)" , David Airlie , "Kuehling, Felix" , LKML , amd-gfx list , "Huang, Ray" , Maling list - DRI developers , "Koenig, Christian" This is a multi-part message in MIME format. --===============1584869283== Content-Type: multipart/alternative; boundary="------------EE817BF33AD9A922A5D68A8E" Content-Language: en-US This is a multi-part message in MIME format. --------------EE817BF33AD9A922A5D68A8E Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit > Who's tree should this go through? To answer the question: When Rex is ok with that he pushes it to our internal amd-staging-drm-next tree. Alex then pushes that tree to a public server and at some point sends a pull request for inclusion in drm-next. Regards, Christian. Am 17.07.2018 um 08:23 schrieb Zhu, Rex: > Patch is: > Reviewed-by: Rex Zhu> > > > > Best Regards > Rex > > > ------------------------------------------------------------------------ > *From:* keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org on behalf of Kees > Cook > *Sent:* Tuesday, July 17, 2018 11:59 AM > *To:* Deucher, Alexander > *Cc:* LKML; Koenig, Christian; Zhou, David(ChunMing); David Airlie; > Zhu, Rex; Huang, Ray; Kuehling, Felix; amd-gfx list; Maling list - DRI > developers > *Subject:* Re: [PATCH] drm/amdgpu/pm: Remove VLA usage > On Wed, Jun 20, 2018 at 11:26 AM, Kees Cook wrote: > > In the quest to remove all stack VLA usage from the kernel[1], this > > uses the maximum sane buffer size and removes copy/paste code. > > > > [1] > https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org > > > > Signed-off-by: Kees Cook > > Friendly ping! Who's tree should this go through? > > Thanks! > > -Kees > > > --- > >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 100 +++++++++++-------------- > >  1 file changed, 42 insertions(+), 58 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > > index b455da487782..5eb98cde22ed 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > > @@ -593,40 +593,59 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct > device *dev, > >                 return snprintf(buf, PAGE_SIZE, "\n"); > >  } > > > > -static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > > -               struct device_attribute *attr, > > -               const char *buf, > > -               size_t count) > > +/* > > + * Worst case: 32 bits individually specified, in octal at 12 > characters > > + * per line (+1 for \n). > > + */ > > +#define AMDGPU_MASK_BUF_MAX    (32 * 13) > > + > > +static ssize_t amdgpu_read_mask(const char *buf, size_t count, > uint32_t *mask) > >  { > > -       struct drm_device *ddev = dev_get_drvdata(dev); > > -       struct amdgpu_device *adev = ddev->dev_private; > >         int ret; > >         long level; > > -       uint32_t mask = 0; > >         char *sub_str = NULL; > >         char *tmp; > > -       char buf_cpy[count]; > > +       char buf_cpy[AMDGPU_MASK_BUF_MAX + 1]; > >         const char delimiter[3] = {' ', '\n', '\0'}; > > +       size_t bytes; > > > > -       memcpy(buf_cpy, buf, count+1); > > +       *mask = 0; > > + > > +       bytes = min(count, sizeof(buf_cpy) - 1); > > +       memcpy(buf_cpy, buf, bytes); > > +       buf_cpy[bytes] = '\0'; > >         tmp = buf_cpy; > >         while (tmp[0]) { > > -               sub_str =  strsep(&tmp, delimiter); > > +               sub_str = strsep(&tmp, delimiter); > >                 if (strlen(sub_str)) { > >                         ret = kstrtol(sub_str, 0, &level); > > - > > -                       if (ret) { > > -                               count = -EINVAL; > > -                               goto fail; > > -                       } > > -                       mask |= 1 << level; > > +                       if (ret) > > +                               return -EINVAL; > > +                       *mask |= 1 << level; > >                 } else > >                         break; > >         } > > + > > +       return 0; > > +} > > + > > +static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > > +               struct device_attribute *attr, > > +               const char *buf, > > +               size_t count) > > +{ > > +       struct drm_device *ddev = dev_get_drvdata(dev); > > +       struct amdgpu_device *adev = ddev->dev_private; > > +       int ret; > > +       uint32_t mask = 0; > > + > > +       ret = amdgpu_read_mask(buf, count, &mask); > > +       if (ret) > > +               return ret; > > + > >         if (adev->powerplay.pp_funcs->force_clock_level) > > amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask); > > > > -fail: > >         return count; > >  } > > > > @@ -651,32 +670,15 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct > device *dev, > >         struct drm_device *ddev = dev_get_drvdata(dev); > >         struct amdgpu_device *adev = ddev->dev_private; > >         int ret; > > -       long level; > >         uint32_t mask = 0; > > -       char *sub_str = NULL; > > -       char *tmp; > > -       char buf_cpy[count]; > > -       const char delimiter[3] = {' ', '\n', '\0'}; > > > > -       memcpy(buf_cpy, buf, count+1); > > -       tmp = buf_cpy; > > -       while (tmp[0]) { > > -               sub_str =  strsep(&tmp, delimiter); > > -               if (strlen(sub_str)) { > > -                       ret = kstrtol(sub_str, 0, &level); > > +       ret = amdgpu_read_mask(buf, count, &mask); > > +       if (ret) > > +               return ret; > > > > -                       if (ret) { > > -                               count = -EINVAL; > > -                               goto fail; > > -                       } > > -                       mask |= 1 << level; > > -               } else > > -                       break; > > -       } > >         if (adev->powerplay.pp_funcs->force_clock_level) > > amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask); > > > > -fail: > >         return count; > >  } > > > > @@ -701,33 +703,15 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct > device *dev, > >         struct drm_device *ddev = dev_get_drvdata(dev); > >         struct amdgpu_device *adev = ddev->dev_private; > >         int ret; > > -       long level; > >         uint32_t mask = 0; > > -       char *sub_str = NULL; > > -       char *tmp; > > -       char buf_cpy[count]; > > -       const char delimiter[3] = {' ', '\n', '\0'}; > > - > > -       memcpy(buf_cpy, buf, count+1); > > -       tmp = buf_cpy; > > > > -       while (tmp[0]) { > > -               sub_str =  strsep(&tmp, delimiter); > > -               if (strlen(sub_str)) { > > -                       ret = kstrtol(sub_str, 0, &level); > > +       ret = amdgpu_read_mask(buf, count, &mask); > > +       if (ret) > > +               return ret; > > > > -                       if (ret) { > > -                               count = -EINVAL; > > -                               goto fail; > > -                       } > > -                       mask |= 1 << level; > > -               } else > > -                       break; > > -       } > >         if (adev->powerplay.pp_funcs->force_clock_level) > > amdgpu_dpm_force_clock_level(adev, PP_PCIE, mask); > > > > -fail: > >         return count; > >  } > > > > -- > > 2.17.1 > > > > > > -- > > Kees Cook > > Pixel Security > > > > -- > Kees Cook > Pixel Security > > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx --------------EE817BF33AD9A922A5D68A8E Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: 8bit
Who's tree should this go through?
To answer the question: When Rex is ok with that he pushes it to our internal amd-staging-drm-next tree.

Alex then pushes that tree to a public server and at some point sends a pull request for inclusion in drm-next.

Regards,
Christian.

Am 17.07.2018 um 08:23 schrieb Zhu, Rex:
Patch is:
Reviewed-by: Rex Zhu<rezhu-5C7GfCeVMHo@public.gmane.org> 



Best Regards
Rex


From: keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org <keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> on behalf of Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Sent: Tuesday, July 17, 2018 11:59 AM
To: Deucher, Alexander
Cc: LKML; Koenig, Christian; Zhou, David(ChunMing); David Airlie; Zhu, Rex; Huang, Ray; Kuehling, Felix; amd-gfx list; Maling list - DRI developers
Subject: Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
 
On Wed, Jun 20, 2018 at 11:26 AM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> In the quest to remove all stack VLA usage from the kernel[1], this
> uses the maximum sane buffer size and removes copy/paste code.
>
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
>
> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Friendly ping! Who's tree should this go through?

Thanks!

-Kees

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 100 +++++++++++--------------
>  1 file changed, 42 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index b455da487782..5eb98cde22ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -593,40 +593,59 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev,
>                 return snprintf(buf, PAGE_SIZE, "\n");
>  }
>
> -static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
> -               struct device_attribute *attr,
> -               const char *buf,
> -               size_t count)
> +/*
> + * Worst case: 32 bits individually specified, in octal at 12 characters
> + * per line (+1 for \n).
> + */
> +#define AMDGPU_MASK_BUF_MAX    (32 * 13)
> +
> +static ssize_t amdgpu_read_mask(const char *buf, size_t count, uint32_t *mask)
>  {
> -       struct drm_device *ddev = dev_get_drvdata(dev);
> -       struct amdgpu_device *adev = ddev->dev_private;
>         int ret;
>         long level;
> -       uint32_t mask = 0;
>         char *sub_str = NULL;
>         char *tmp;
> -       char buf_cpy[count];
> +       char buf_cpy[AMDGPU_MASK_BUF_MAX + 1];
>         const char delimiter[3] = {' ', '\n', '\0'};
> +       size_t bytes;
>
> -       memcpy(buf_cpy, buf, count+1);
> +       *mask = 0;
> +
> +       bytes = min(count, sizeof(buf_cpy) - 1);
> +       memcpy(buf_cpy, buf, bytes);
> +       buf_cpy[bytes] = '\0';
>         tmp = buf_cpy;
>         while (tmp[0]) {
> -               sub_str =  strsep(&tmp, delimiter);
> +               sub_str = strsep(&tmp, delimiter);
>                 if (strlen(sub_str)) {
>                         ret = kstrtol(sub_str, 0, &level);
> -
> -                       if (ret) {
> -                               count = -EINVAL;
> -                               goto fail;
> -                       }
> -                       mask |= 1 << level;
> +                       if (ret)
> +                               return -EINVAL;
> +                       *mask |= 1 << level;
>                 } else
>                         break;
>         }
> +
> +       return 0;
> +}
> +
> +static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
> +               struct device_attribute *attr,
> +               const char *buf,
> +               size_t count)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = ddev->dev_private;
> +       int ret;
> +       uint32_t mask = 0;
> +
> +       ret = amdgpu_read_mask(buf, count, &mask);
> +       if (ret)
> +               return ret;
> +
>         if (adev->powerplay.pp_funcs->force_clock_level)
>                 amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
>
> -fail:
>         return count;
>  }
>
> @@ -651,32 +670,15 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
>         struct drm_device *ddev = dev_get_drvdata(dev);
>         struct amdgpu_device *adev = ddev->dev_private;
>         int ret;
> -       long level;
>         uint32_t mask = 0;
> -       char *sub_str = NULL;
> -       char *tmp;
> -       char buf_cpy[count];
> -       const char delimiter[3] = {' ', '\n', '\0'};
>
> -       memcpy(buf_cpy, buf, count+1);
> -       tmp = buf_cpy;
> -       while (tmp[0]) {
> -               sub_str =  strsep(&tmp, delimiter);
> -               if (strlen(sub_str)) {
> -                       ret = kstrtol(sub_str, 0, &level);
> +       ret = amdgpu_read_mask(buf, count, &mask);
> +       if (ret)
> +               return ret;
>
> -                       if (ret) {
> -                               count = -EINVAL;
> -                               goto fail;
> -                       }
> -                       mask |= 1 << level;
> -               } else
> -                       break;
> -       }
>         if (adev->powerplay.pp_funcs->force_clock_level)
>                 amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);
>
> -fail:
>         return count;
>  }
>
> @@ -701,33 +703,15 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
>         struct drm_device *ddev = dev_get_drvdata(dev);
>         struct amdgpu_device *adev = ddev->dev_private;
>         int ret;
> -       long level;
>         uint32_t mask = 0;
> -       char *sub_str = NULL;
> -       char *tmp;
> -       char buf_cpy[count];
> -       const char delimiter[3] = {' ', '\n', '\0'};
> -
> -       memcpy(buf_cpy, buf, count+1);
> -       tmp = buf_cpy;
>
> -       while (tmp[0]) {
> -               sub_str =  strsep(&tmp, delimiter);
> -               if (strlen(sub_str)) {
> -                       ret = kstrtol(sub_str, 0, &level);
> +       ret = amdgpu_read_mask(buf, count, &mask);
> +       if (ret)
> +               return ret;
>
> -                       if (ret) {
> -                               count = -EINVAL;
> -                               goto fail;
> -                       }
> -                       mask |= 1 << level;
> -               } else
> -                       break;
> -       }
>         if (adev->powerplay.pp_funcs->force_clock_level)
>                 amdgpu_dpm_force_clock_level(adev, PP_PCIE, mask);
>
> -fail:
>         return count;
>  }
>
> --
> 2.17.1
>
>
> --
> Kees Cook
> Pixel Security



--
Kees Cook
Pixel Security


_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

--------------EE817BF33AD9A922A5D68A8E-- --===============1584869283== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4Cg== --===============1584869283==--