diff mbox series

[-next] cpufreq: Fix reserved space in cpufreq_show_cpus()

Message ID 20220521063534.138930-1-yiyang13@huawei.com (mailing list archive)
State Queued
Delegated to: Rafael Wysocki
Headers show
Series [-next] cpufreq: Fix reserved space in cpufreq_show_cpus() | expand

Commit Message

yiyang (D) May 21, 2022, 6:35 a.m. UTC
Function scnprintf() would reserve space for the trailing '\0' and return 
value is the number of characters written into buf not including the 
trailing '\0'. internally meaning the next scnprintf() would write begin 
the trailing '\0'. The code specifying "PAGE_SIZE - i - 2" here is trying 
to reserve space for "\n\0" which would cause scnprintf() to reserve an 
additional byte making the tail of the buf looks like this: "\n\0\0". 
Thus. we should reserve only the space for one '\0'. passing in 
"PAGE_SIZE - i - 1".

Additionally, each iteration would replace the trailing '\0' from the last 
iteration with a space, and append 4 additional bytes to the string making 
it a total of 5 additional bytes. That means we should stop printing into 
the buffer if the remaining size is less than 7 bytes(1 for the ' ', 4 for 
the %u and 2 for the tailing "\n\0")

Signed-off-by: Yi Yang <yiyang13@huawei.com>
---
 drivers/cpufreq/cpufreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Viresh Kumar May 24, 2022, 7:10 a.m. UTC | #1
On 21-05-22, 14:35, Yi Yang wrote:
> Function scnprintf() would reserve space for the trailing '\0' and return 
> value is the number of characters written into buf not including the 
> trailing '\0'. internally meaning the next scnprintf() would write begin 
> the trailing '\0'. The code specifying "PAGE_SIZE - i - 2" here is trying 
> to reserve space for "\n\0" which would cause scnprintf() to reserve an 
> additional byte making the tail of the buf looks like this: "\n\0\0". 
> Thus. we should reserve only the space for one '\0'. passing in 
> "PAGE_SIZE - i - 1".
> 
> Additionally, each iteration would replace the trailing '\0' from the last 
> iteration with a space, and append 4 additional bytes to the string making 
> it a total of 5 additional bytes. That means we should stop printing into 
> the buffer if the remaining size is less than 7 bytes(1 for the ' ', 4 for 
> the %u and 2 for the tailing "\n\0")
> 
> Signed-off-by: Yi Yang <yiyang13@huawei.com>
> ---
>  drivers/cpufreq/cpufreq.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 1f6667ce43bd..60c005c9961e 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -844,9 +844,9 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
>  
>  	for_each_cpu(cpu, mask) {
>  		if (i)
> -			i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), " ");
> -		i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), "%u", cpu);
> -		if (i >= (PAGE_SIZE - 5))
> +			i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), " ");
> +		i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), "%u", cpu);
> +		if (i >= (PAGE_SIZE - 6))
>  			break;
>  	}
>  	i += sprintf(&buf[i], "\n");

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Rafael J. Wysocki June 14, 2022, 1:37 p.m. UTC | #2
On Tue, May 24, 2022 at 9:10 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 21-05-22, 14:35, Yi Yang wrote:
> > Function scnprintf() would reserve space for the trailing '\0' and return
> > value is the number of characters written into buf not including the
> > trailing '\0'. internally meaning the next scnprintf() would write begin
> > the trailing '\0'. The code specifying "PAGE_SIZE - i - 2" here is trying
> > to reserve space for "\n\0" which would cause scnprintf() to reserve an
> > additional byte making the tail of the buf looks like this: "\n\0\0".
> > Thus. we should reserve only the space for one '\0'. passing in
> > "PAGE_SIZE - i - 1".
> >
> > Additionally, each iteration would replace the trailing '\0' from the last
> > iteration with a space, and append 4 additional bytes to the string making
> > it a total of 5 additional bytes. That means we should stop printing into
> > the buffer if the remaining size is less than 7 bytes(1 for the ' ', 4 for
> > the %u and 2 for the tailing "\n\0")
> >
> > Signed-off-by: Yi Yang <yiyang13@huawei.com>
> > ---
> >  drivers/cpufreq/cpufreq.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 1f6667ce43bd..60c005c9961e 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -844,9 +844,9 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
> >
> >       for_each_cpu(cpu, mask) {
> >               if (i)
> > -                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), " ");
> > -             i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), "%u", cpu);
> > -             if (i >= (PAGE_SIZE - 5))
> > +                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), " ");
> > +             i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), "%u", cpu);
> > +             if (i >= (PAGE_SIZE - 6))
> >                       break;
> >       }
> >       i += sprintf(&buf[i], "\n");
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied as 5.20 material, thanks!
Rafael J. Wysocki June 14, 2022, 1:48 p.m. UTC | #3
On Tue, Jun 14, 2022 at 3:37 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Tue, May 24, 2022 at 9:10 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 21-05-22, 14:35, Yi Yang wrote:
> > > Function scnprintf() would reserve space for the trailing '\0' and return
> > > value is the number of characters written into buf not including the
> > > trailing '\0'. internally meaning the next scnprintf() would write begin
> > > the trailing '\0'. The code specifying "PAGE_SIZE - i - 2" here is trying
> > > to reserve space for "\n\0" which would cause scnprintf() to reserve an
> > > additional byte making the tail of the buf looks like this: "\n\0\0".
> > > Thus. we should reserve only the space for one '\0'. passing in
> > > "PAGE_SIZE - i - 1".
> > >
> > > Additionally, each iteration would replace the trailing '\0' from the last
> > > iteration with a space, and append 4 additional bytes to the string making
> > > it a total of 5 additional bytes. That means we should stop printing into
> > > the buffer if the remaining size is less than 7 bytes(1 for the ' ', 4 for
> > > the %u and 2 for the tailing "\n\0")
> > >
> > > Signed-off-by: Yi Yang <yiyang13@huawei.com>
> > > ---
> > >  drivers/cpufreq/cpufreq.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index 1f6667ce43bd..60c005c9961e 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -844,9 +844,9 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
> > >
> > >       for_each_cpu(cpu, mask) {
> > >               if (i)
> > > -                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), " ");
> > > -             i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), "%u", cpu);
> > > -             if (i >= (PAGE_SIZE - 5))
> > > +                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), " ");
> > > +             i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), "%u", cpu);
> > > +             if (i >= (PAGE_SIZE - 6))
> > >                       break;
> > >       }
> > >       i += sprintf(&buf[i], "\n");
> >
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Applied as 5.20 material, thanks!

And dropped, because it has been superseded by this one:

https://patchwork.kernel.org/project/linux-pm/patch/b9fa08171c09343ace94a7343553a4bee4695c90.1653565641.git.viresh.kumar@linaro.org/
Viresh Kumar June 15, 2022, 4:56 a.m. UTC | #4
On 14-06-22, 15:48, Rafael J. Wysocki wrote:
> On Tue, Jun 14, 2022 at 3:37 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Tue, May 24, 2022 at 9:10 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > On 21-05-22, 14:35, Yi Yang wrote:
> > > > Function scnprintf() would reserve space for the trailing '\0' and return
> > > > value is the number of characters written into buf not including the
> > > > trailing '\0'. internally meaning the next scnprintf() would write begin
> > > > the trailing '\0'. The code specifying "PAGE_SIZE - i - 2" here is trying
> > > > to reserve space for "\n\0" which would cause scnprintf() to reserve an
> > > > additional byte making the tail of the buf looks like this: "\n\0\0".
> > > > Thus. we should reserve only the space for one '\0'. passing in
> > > > "PAGE_SIZE - i - 1".
> > > >
> > > > Additionally, each iteration would replace the trailing '\0' from the last
> > > > iteration with a space, and append 4 additional bytes to the string making
> > > > it a total of 5 additional bytes. That means we should stop printing into
> > > > the buffer if the remaining size is less than 7 bytes(1 for the ' ', 4 for
> > > > the %u and 2 for the tailing "\n\0")
> > > >
> > > > Signed-off-by: Yi Yang <yiyang13@huawei.com>
> > > > ---
> > > >  drivers/cpufreq/cpufreq.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > > index 1f6667ce43bd..60c005c9961e 100644
> > > > --- a/drivers/cpufreq/cpufreq.c
> > > > +++ b/drivers/cpufreq/cpufreq.c
> > > > @@ -844,9 +844,9 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
> > > >
> > > >       for_each_cpu(cpu, mask) {
> > > >               if (i)
> > > > -                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), " ");
> > > > -             i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), "%u", cpu);
> > > > -             if (i >= (PAGE_SIZE - 5))
> > > > +                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), " ");
> > > > +             i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), "%u", cpu);
> > > > +             if (i >= (PAGE_SIZE - 6))
> > > >                       break;
> > > >       }
> > > >       i += sprintf(&buf[i], "\n");
> > >
> > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > Applied as 5.20 material, thanks!
> 
> And dropped, because it has been superseded by this one:
> 
> https://patchwork.kernel.org/project/linux-pm/patch/b9fa08171c09343ace94a7343553a4bee4695c90.1653565641.git.viresh.kumar@linaro.org/

The $Subject patch is still required AFAICT, it is fixing a different problem.
Though it needs to be rebased on top of your branch now.
yiyang (D) June 18, 2022, 10:32 a.m. UTC | #5
On 2022/6/15 12:56, Viresh Kumar wrote:
> On 14-06-22, 15:48, Rafael J. Wysocki wrote:
>> On Tue, Jun 14, 2022 at 3:37 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>
>>> On Tue, May 24, 2022 at 9:10 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>>
>>>> On 21-05-22, 14:35, Yi Yang wrote:
>>>>> Function scnprintf() would reserve space for the trailing '\0' and return
>>>>> value is the number of characters written into buf not including the
>>>>> trailing '\0'. internally meaning the next scnprintf() would write begin
>>>>> the trailing '\0'. The code specifying "PAGE_SIZE - i - 2" here is trying
>>>>> to reserve space for "\n\0" which would cause scnprintf() to reserve an
>>>>> additional byte making the tail of the buf looks like this: "\n\0\0".
>>>>> Thus. we should reserve only the space for one '\0'. passing in
>>>>> "PAGE_SIZE - i - 1".
>>>>>
>>>>> Additionally, each iteration would replace the trailing '\0' from the last
>>>>> iteration with a space, and append 4 additional bytes to the string making
>>>>> it a total of 5 additional bytes. That means we should stop printing into
>>>>> the buffer if the remaining size is less than 7 bytes(1 for the ' ', 4 for
>>>>> the %u and 2 for the tailing "\n\0")
>>>>>
>>>>> Signed-off-by: Yi Yang <yiyang13@huawei.com>
>>>>> ---
>>>>>   drivers/cpufreq/cpufreq.c | 6 +++---
>>>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>>>> index 1f6667ce43bd..60c005c9961e 100644
>>>>> --- a/drivers/cpufreq/cpufreq.c
>>>>> +++ b/drivers/cpufreq/cpufreq.c
>>>>> @@ -844,9 +844,9 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
>>>>>
>>>>>        for_each_cpu(cpu, mask) {
>>>>>                if (i)
>>>>> -                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), " ");
>>>>> -             i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), "%u", cpu);
>>>>> -             if (i >= (PAGE_SIZE - 5))
>>>>> +                     i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), " ");
>>>>> +             i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), "%u", cpu);
>>>>> +             if (i >= (PAGE_SIZE - 6))
>>>>>                        break;
>>>>>        }
>>>>>        i += sprintf(&buf[i], "\n");
>>>>
>>>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>
>>> Applied as 5.20 material, thanks!
>>
>> And dropped, because it has been superseded by this one:
>>
>> https://patchwork.kernel.org/project/linux-pm/patch/b9fa08171c09343ace94a7343553a4bee4695c90.1653565641.git.viresh.kumar@linaro.org/
> 
> The $Subject patch is still required AFAICT, it is fixing a different problem.
> Though it needs to be rebased on top of your branch now.
> 

What can i do for this patch?

--
Yi
Viresh Kumar June 20, 2022, 4:24 a.m. UTC | #6
On 18-06-22, 18:32, yiyang (D) wrote:
> What can i do for this patch?

Resend it after rebasing on pm/linux-next.
diff mbox series

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1f6667ce43bd..60c005c9961e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -844,9 +844,9 @@  ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
 
 	for_each_cpu(cpu, mask) {
 		if (i)
-			i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), " ");
-		i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), "%u", cpu);
-		if (i >= (PAGE_SIZE - 5))
+			i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), " ");
+		i += scnprintf(&buf[i], (PAGE_SIZE - i - 1), "%u", cpu);
+		if (i >= (PAGE_SIZE - 6))
 			break;
 	}
 	i += sprintf(&buf[i], "\n");