All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu/pm: Remove VLA usage
@ 2018-06-20 18:26 Kees Cook
  2018-07-17  3:59 ` Kees Cook
  0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2018-06-20 18:26 UTC (permalink / raw)
  To: Alex Deucher
  Cc: linux-kernel, Christian König, David (ChunMing) Zhou,
	David Airlie, Rex Zhu, Huang Rui, Felix Kuehling, amd-gfx,
	dri-devel

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@mail.gmail.com

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
  2018-06-20 18:26 [PATCH] drm/amdgpu/pm: Remove VLA usage Kees Cook
@ 2018-07-17  3:59 ` Kees Cook
       [not found]   ` <CAGXu5jK74ew7iTw2_OqJSj4wYjBggPpkN-ENEFRpuq0C-eGRDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2018-07-17  3:59 UTC (permalink / raw)
  To: Alex Deucher
  Cc: LKML, Christian König, David (ChunMing) Zhou, David Airlie,
	Rex Zhu, Huang Rui, Felix Kuehling, amd-gfx list,
	Maling list - DRI developers

On Wed, Jun 20, 2018 at 11:26 AM, Kees Cook <keescook@chromium.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@mail.gmail.com
>
> Signed-off-by: Kees Cook <keescook@chromium.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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
       [not found]   ` <CAGXu5jK74ew7iTw2_OqJSj4wYjBggPpkN-ENEFRpuq0C-eGRDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-07-17  6:23     ` Zhu, Rex
       [not found]       ` <CY4PR12MB16874F4DB841A96E065E94EEFB5C0-rpdhrqHFk06Y0SjTqZDccQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Zhu, Rex @ 2018-07-17  6:23 UTC (permalink / raw)
  To: Kees Cook, Deucher, Alexander
  Cc: Zhou, David(ChunMing), David Airlie, Kuehling, Felix, LKML,
	amd-gfx list, Huang, Ray, Maling list - DRI developers,
	Koenig, Christian


[-- Attachment #1.1: Type: text/plain, Size: 6688 bytes --]

Patch is:
Reviewed-by: Rex Zhu<rezhu-5C7GfCeVMHo@public.gmane.org<mailto: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

[-- Attachment #1.2: Type: text/html, Size: 16688 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
       [not found]       ` <CY4PR12MB16874F4DB841A96E065E94EEFB5C0-rpdhrqHFk06Y0SjTqZDccQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-07-17  6:58         ` Christian König
       [not found]           ` <42ba5486-34a2-02bd-50a1-12314e6e2755-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2018-07-17  6:58 UTC (permalink / raw)
  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


[-- Attachment #1.1: Type: text/plain, Size: 7423 bytes --]

> 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 <mailto: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


[-- Attachment #1.2: Type: text/html, Size: 15959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
       [not found]           ` <42ba5486-34a2-02bd-50a1-12314e6e2755-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-07-18 10:45             ` Zhu, Rex
  0 siblings, 0 replies; 5+ messages in thread
From: Zhu, Rex @ 2018-07-18 10:45 UTC (permalink / raw)
  To: Kees Cook, Deucher, Alexander, Koenig, Christian
  Cc: Zhou, David(ChunMing), David Airlie, Kuehling, Felix, LKML,
	amd-gfx list, Huang, Ray, Maling list - DRI developers


[-- Attachment #1.1: Type: text/plain, Size: 8043 bytes --]

Patch has been applied to our internal amd-staging-drm-next tree.


Thanks.


Best Regards

Rex

________________________________
From: Christian K?nig <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Tuesday, July 17, 2018 2:58:21 PM
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
Subject: Re: [PATCH] drm/amdgpu/pm: Remove VLA usage

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<mailto:rezhu-5C7GfCeVMHo@public.gmane.org>>



Best Regards
Rex


________________________________
From: keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org<mailto:keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> <keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org><mailto:keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> on behalf of Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org><mailto: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><mailto: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><mailto:keescook@chromium.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<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[-- Attachment #1.2: Type: text/html, Size: 19456 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-07-18 10:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-20 18:26 [PATCH] drm/amdgpu/pm: Remove VLA usage Kees Cook
2018-07-17  3:59 ` Kees Cook
     [not found]   ` <CAGXu5jK74ew7iTw2_OqJSj4wYjBggPpkN-ENEFRpuq0C-eGRDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-07-17  6:23     ` Zhu, Rex
     [not found]       ` <CY4PR12MB16874F4DB841A96E065E94EEFB5C0-rpdhrqHFk06Y0SjTqZDccQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-07-17  6:58         ` Christian König
     [not found]           ` <42ba5486-34a2-02bd-50a1-12314e6e2755-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-07-18 10:45             ` Zhu, Rex

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.