<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> <div dir="auto" style="direction: ltr; margin: 0px; padding: 0px; font-family: sans-serif, serif, "EmojiFont"; font-size: 11pt; color: black;"> Patch is: Reviewed-by: Rex Zhu<<a href="mailto:rezhu@amd.com" target="_blank" rel="noopener noreferrer" data-auth="NotApplicable" id="LPlnk600685" class="OWAAutoLink" previewremoved="true">rezhu@amd.com</a>> </div> <div align="left"> <div dir="auto" style="direction: ltr; margin: 0px; padding: 0px; font-family: sans-serif, serif, "EmojiFont"; font-size: 11pt; color: black;"> Best Regards </div> </div> Rex <div style="color: rgb(0, 0, 0);"> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: keescook@google.com <keescook@google.com> on behalf of Kees Cook <keescook@chromium.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 <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">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] <a href="https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com" id="LPlnk141741" class="OWAAutoLink" previewremoved="true"> https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com</a> > > 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 </div> </div> </div> </div> </body> </html>