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? 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
