AMD General > -----Original Message----- > From: Kamal, Asad <[email protected]> > Sent: Friday, May 29, 2026 16:07 > To: Wang, Yang(Kevin) <[email protected]>; amd- > [email protected] > Cc: Lazar, Lijo <[email protected]>; Zhang, Hawking > <[email protected]>; Ma, Le <[email protected]>; Zhang, Morris > <[email protected]>; Deucher, Alexander <[email protected]> > Subject: RE: [PATCH] drm/amd/pm: fix off-by-one over-read in pp mode > > AMD General > > Good catch. count and i are unsigned, so the concern is underflow of count - > 1 - I, if i > + 1 > count, not signed overflow. On the sysfs path, kernfs NUL-terminates at > buf[count] and count >= 2, so the whitespace loop should keep i <= count - 1, > but > that isn't explicit today. I'll add if (i + 1 > count) return -EINVAL before > computing len > and keep buf_cpy[len] = '\0' for strsep(). > > Is it Ok?
Looks good, go ahead please. Best Regards, Kevin > > Thanks & Regards > Asad > > > > -----Original Message----- > From: Wang, Yang(Kevin) <[email protected]> > Sent: Friday, May 29, 2026 12:26 PM > To: Kamal, Asad <[email protected]>; [email protected] > Cc: Lazar, Lijo <[email protected]>; Zhang, Hawking > <[email protected]>; Ma, Le <[email protected]>; Zhang, Morris > <[email protected]>; Deucher, Alexander <[email protected]> > Subject: RE: [PATCH] drm/amd/pm: fix off-by-one over-read in pp mode > > AMD General > > > -----Original Message----- > > From: Kamal, Asad <[email protected]> > > Sent: Friday, May 29, 2026 14:18 > > To: [email protected] > > Cc: Lazar, Lijo <[email protected]>; Zhang, Hawking > > <[email protected]>; Ma, Le <[email protected]>; Zhang, Morris > > <[email protected]>; Deucher, Alexander > <[email protected]>; > > Wang, Yang(Kevin) <[email protected]>; Kamal, Asad > > <[email protected]> > > Subject: [PATCH] drm/amd/pm: fix off-by-one over-read in pp mode > > > > After consuming the leading profile digit in tmp[0] and skipping i > > bytes of whitespace via *++buf, buf points at original + 1 + i. The > > number of bytes still inside the sysfs buffer is count - (1 + i), not > > count - i; using the latter copied one byte past the store buffer. > > > > NUL-terminate buf_cpy before strsep() so parsing cannot run past the > > copied payload. > > > > Signed-off-by: Asad Kamal <[email protected]> > > --- > > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > index 60db9b66d08c..450ecb188aed 100644 > > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > @@ -1379,6 +1379,7 @@ static ssize_t > > amdgpu_set_pp_power_profile_mode(struct device *dev, > > char tmp[2]; > > long int profile_mode = 0; > > const char delimiter[3] = {' ', '\n', '\0'}; > > + size_t len; > > > > tmp[0] = *(buf); > > tmp[1] = '\0'; > > @@ -1391,7 +1392,9 @@ static ssize_t > > amdgpu_set_pp_power_profile_mode(struct device *dev, > > return -EINVAL; > > while (isspace(*++buf)) > > i++; > > - memcpy(buf_cpy, buf, count-i); > > + len = count - 1 - i; > There is still a risk of signed integer overflow here, please review this > part. > btw, the variable 'i' may be greater than 'count' ? > > while (isspace(*++buf)) > > i++; > > Best Regards, > Kevin > > + memcpy(buf_cpy, buf, len); > > + buf_cpy[len] = '\0'; > > tmp_str = buf_cpy; > > while ((sub_str = strsep(&tmp_str, delimiter)) != NULL) { > > if (strlen(sub_str) == 0) > > -- > > 2.46.0 > >
