* [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats
@ 2023-10-24 18:30 Christian Marangi
2023-10-24 18:30 ` [PATCH 2/3] PM / devfreq: Fix buffer overflow in trans_stat_show Christian Marangi
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Christian Marangi @ 2023-10-24 18:30 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian Marangi, Takashi Iwai, Jonghwa Lee,
linux-kernel, linux-pm
Cc: stable
Commit 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential
buffer overflow") switched from snprintf to the more secure scnprintf
but never updated the exit condition for PAGE_SIZE.
As the commit say and as scnprintf document, what scnprintf returns what
is actually written not counting the '\0' end char. This results in the
case of len exceeding the size, len set to PAGE_SIZE - 1, as it can be
written at max PAGESIZE - 1 (as '\0' is not counted)
Because of len is never set to PAGE_SIZE, the function never break early,
never print the warning and never return -EFBIG.
Fix this by fixing the condition to PAGE_SIZE -1 to correctly trigger
the error condition.
Cc: stable@vger.kernel.org
Fixes: 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential buffer overflow")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/cpufreq/cpufreq_stats.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index a33df3c66c88..40a9ff18da06 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -131,23 +131,23 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
len += sysfs_emit_at(buf, len, " From : To\n");
len += sysfs_emit_at(buf, len, " : ");
for (i = 0; i < stats->state_num; i++) {
- if (len >= PAGE_SIZE)
+ if (len >= PAGE_SIZE - 1)
break;
len += sysfs_emit_at(buf, len, "%9u ", stats->freq_table[i]);
}
- if (len >= PAGE_SIZE)
- return PAGE_SIZE;
+ if (len >= PAGE_SIZE - 1)
+ return PAGE_SIZE - 1;
len += sysfs_emit_at(buf, len, "\n");
for (i = 0; i < stats->state_num; i++) {
- if (len >= PAGE_SIZE)
+ if (len >= PAGE_SIZE - 1)
break;
len += sysfs_emit_at(buf, len, "%9u: ", stats->freq_table[i]);
for (j = 0; j < stats->state_num; j++) {
- if (len >= PAGE_SIZE)
+ if (len >= PAGE_SIZE - 1)
break;
if (pending)
@@ -157,12 +157,12 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
len += sysfs_emit_at(buf, len, "%9u ", count);
}
- if (len >= PAGE_SIZE)
+ if (len >= PAGE_SIZE - 1)
break;
len += sysfs_emit_at(buf, len, "\n");
}
- if (len >= PAGE_SIZE) {
+ if (len >= PAGE_SIZE - 1) {
pr_warn_once("cpufreq transition table exceeds PAGE_SIZE. Disabling\n");
return -EFBIG;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] PM / devfreq: Fix buffer overflow in trans_stat_show
2023-10-24 18:30 [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Christian Marangi
@ 2023-10-24 18:30 ` Christian Marangi
2023-11-02 17:26 ` Christian Marangi
2023-10-24 18:30 ` [PATCH 3/3] PM / devfreq: Convert to use sysfs_emit_at() API Christian Marangi
2023-10-24 20:03 ` [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Rafael J. Wysocki
2 siblings, 1 reply; 7+ messages in thread
From: Christian Marangi @ 2023-10-24 18:30 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian Marangi, Takashi Iwai, Jonghwa Lee,
linux-kernel, linux-pm
Cc: stable
Fix buffer overflow in trans_stat_show().
Convert simple snprintf to the more secure scnprintf with size of
PAGE_SIZE.
Add condition checking if we are exceeding PAGE_SIZE and exit early from
loop. Also add at the end a warning that we exceeded PAGE_SIZE and that
stats is disabled.
Return -EFBIG in the case where we don't have enough space to write the
full transition table.
Also document in the ABI that this function can return -EFBIG error.
Cc: stable@vger.kernel.org
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218041
Fixes: e552bbaf5b98 ("PM / devfreq: Add sysfs node for representing frequency transition information.")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Documentation/ABI/testing/sysfs-class-devfreq | 3 +
drivers/devfreq/devfreq.c | 57 +++++++++++++------
2 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index 5e6b74f30406..1e7e0bb4c14e 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -52,6 +52,9 @@ Description:
echo 0 > /sys/class/devfreq/.../trans_stat
+ If the transition table is bigger than PAGE_SIZE, reading
+ this will return an -EFBIG error.
+
What: /sys/class/devfreq/.../available_frequencies
Date: October 2012
Contact: Nishanth Menon <nm@ti.com>
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 474d81831ad3..81d9df89dde6 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1688,7 +1688,7 @@ static ssize_t trans_stat_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct devfreq *df = to_devfreq(dev);
- ssize_t len;
+ ssize_t len = 0;
int i, j;
unsigned int max_state;
@@ -1697,7 +1697,7 @@ static ssize_t trans_stat_show(struct device *dev,
max_state = df->max_state;
if (max_state == 0)
- return sprintf(buf, "Not Supported.\n");
+ return scnprintf(buf, PAGE_SIZE, "Not Supported.\n");
mutex_lock(&df->lock);
if (!df->stop_polling &&
@@ -1707,31 +1707,52 @@ static ssize_t trans_stat_show(struct device *dev,
}
mutex_unlock(&df->lock);
- len = sprintf(buf, " From : To\n");
- len += sprintf(buf + len, " :");
- for (i = 0; i < max_state; i++)
- len += sprintf(buf + len, "%10lu",
- df->freq_table[i]);
+ len += scnprintf(buf + len, PAGE_SIZE - len, " From : To\n");
+ len += scnprintf(buf + len, PAGE_SIZE - len, " :");
+ for (i = 0; i < max_state; i++) {
+ if (len >= PAGE_SIZE - 1)
+ break;
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%10lu",
+ df->freq_table[i]);
+ }
+ if (len >= PAGE_SIZE - 1)
+ return PAGE_SIZE - 1;
- len += sprintf(buf + len, " time(ms)\n");
+ len += scnprintf(buf + len, PAGE_SIZE - len, " time(ms)\n");
for (i = 0; i < max_state; i++) {
+ if (len >= PAGE_SIZE - 1)
+ break;
if (df->freq_table[i] == df->previous_freq)
- len += sprintf(buf + len, "*");
+ len += scnprintf(buf + len, PAGE_SIZE - len, "*");
else
- len += sprintf(buf + len, " ");
+ len += scnprintf(buf + len, PAGE_SIZE - len, " ");
+ if (len >= PAGE_SIZE - 1)
+ break;
+
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%10lu:",
+ df->freq_table[i]);
+ for (j = 0; j < max_state; j++) {
+ if (len >= PAGE_SIZE - 1)
+ break;
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%10u",
+ df->stats.trans_table[(i * max_state) + j]);
+ }
+ if (len >= PAGE_SIZE - 1)
+ break;
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%10llu\n", (u64)
+ jiffies64_to_msecs(df->stats.time_in_state[i]));
+ }
- len += sprintf(buf + len, "%10lu:", df->freq_table[i]);
- for (j = 0; j < max_state; j++)
- len += sprintf(buf + len, "%10u",
- df->stats.trans_table[(i * max_state) + j]);
+ if (len < PAGE_SIZE - 1)
+ len += scnprintf(buf + len, PAGE_SIZE - len, "Total transition : %u\n",
+ df->stats.total_trans);
- len += sprintf(buf + len, "%10llu\n", (u64)
- jiffies64_to_msecs(df->stats.time_in_state[i]));
+ if (len >= PAGE_SIZE - 1) {
+ pr_warn_once("devfreq transition table exceeds PAGE_SIZE. Disabling\n");
+ return -EFBIG;
}
- len += sprintf(buf + len, "Total transition : %u\n",
- df->stats.total_trans);
return len;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] PM / devfreq: Convert to use sysfs_emit_at() API
2023-10-24 18:30 [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Christian Marangi
2023-10-24 18:30 ` [PATCH 2/3] PM / devfreq: Fix buffer overflow in trans_stat_show Christian Marangi
@ 2023-10-24 18:30 ` Christian Marangi
2023-10-24 20:03 ` [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Rafael J. Wysocki
2 siblings, 0 replies; 7+ messages in thread
From: Christian Marangi @ 2023-10-24 18:30 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Christian Marangi, Takashi Iwai, Jonghwa Lee,
linux-kernel, linux-pm
Follow the advice of the Documentation/filesystems/sysfs.rst and show()
should only use sysfs_emit() or sysfs_emit_at() when formatting the
value to be returned to user space.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/devfreq/devfreq.c | 37 +++++++++++++++++--------------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 81d9df89dde6..2345c86cf19b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1697,7 +1697,7 @@ static ssize_t trans_stat_show(struct device *dev,
max_state = df->max_state;
if (max_state == 0)
- return scnprintf(buf, PAGE_SIZE, "Not Supported.\n");
+ return sysfs_emit(buf, "Not Supported.\n");
mutex_lock(&df->lock);
if (!df->stop_polling &&
@@ -1707,47 +1707,44 @@ static ssize_t trans_stat_show(struct device *dev,
}
mutex_unlock(&df->lock);
- len += scnprintf(buf + len, PAGE_SIZE - len, " From : To\n");
- len += scnprintf(buf + len, PAGE_SIZE - len, " :");
+ len += sysfs_emit_at(buf, len, " From : To\n");
+ len += sysfs_emit_at(buf, len, " :");
for (i = 0; i < max_state; i++) {
if (len >= PAGE_SIZE - 1)
break;
- len += scnprintf(buf + len, PAGE_SIZE - len, "%10lu",
- df->freq_table[i]);
+ len += sysfs_emit_at(buf, len, "%10lu",
+ df->freq_table[i]);
}
+
if (len >= PAGE_SIZE - 1)
return PAGE_SIZE - 1;
-
- len += scnprintf(buf + len, PAGE_SIZE - len, " time(ms)\n");
+ len += sysfs_emit_at(buf, len, " time(ms)\n");
for (i = 0; i < max_state; i++) {
if (len >= PAGE_SIZE - 1)
break;
- if (df->freq_table[i] == df->previous_freq)
- len += scnprintf(buf + len, PAGE_SIZE - len, "*");
+ if (df->freq_table[2] == df->previous_freq)
+ len += sysfs_emit_at(buf, len, "*");
else
- len += scnprintf(buf + len, PAGE_SIZE - len, " ");
+ len += sysfs_emit_at(buf, len, " ");
if (len >= PAGE_SIZE - 1)
break;
-
- len += scnprintf(buf + len, PAGE_SIZE - len, "%10lu:",
- df->freq_table[i]);
+ len += sysfs_emit_at(buf, len, "%10lu:", df->freq_table[i]);
for (j = 0; j < max_state; j++) {
if (len >= PAGE_SIZE - 1)
break;
- len += scnprintf(buf + len, PAGE_SIZE - len, "%10u",
- df->stats.trans_table[(i * max_state) + j]);
+ len += sysfs_emit_at(buf, len, "%10u",
+ df->stats.trans_table[(i * max_state) + j]);
}
if (len >= PAGE_SIZE - 1)
break;
- len += scnprintf(buf + len, PAGE_SIZE - len, "%10llu\n", (u64)
- jiffies64_to_msecs(df->stats.time_in_state[i]));
+ len += sysfs_emit_at(buf, len, "%10llu\n", (u64)
+ jiffies64_to_msecs(df->stats.time_in_state[i]));
}
if (len < PAGE_SIZE - 1)
- len += scnprintf(buf + len, PAGE_SIZE - len, "Total transition : %u\n",
- df->stats.total_trans);
-
+ len += sysfs_emit_at(buf, len, "Total transition : %u\n",
+ df->stats.total_trans);
if (len >= PAGE_SIZE - 1) {
pr_warn_once("devfreq transition table exceeds PAGE_SIZE. Disabling\n");
return -EFBIG;
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats
2023-10-24 18:30 [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Christian Marangi
2023-10-24 18:30 ` [PATCH 2/3] PM / devfreq: Fix buffer overflow in trans_stat_show Christian Marangi
2023-10-24 18:30 ` [PATCH 3/3] PM / devfreq: Convert to use sysfs_emit_at() API Christian Marangi
@ 2023-10-24 20:03 ` Rafael J. Wysocki
2023-10-26 10:53 ` Christian Marangi
2 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2023-10-24 20:03 UTC (permalink / raw)
To: Christian Marangi
Cc: Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Takashi Iwai, Jonghwa Lee, linux-kernel, linux-pm,
stable
On Tue, Oct 24, 2023 at 8:30 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> Commit 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential
> buffer overflow") switched from snprintf to the more secure scnprintf
> but never updated the exit condition for PAGE_SIZE.
>
> As the commit say and as scnprintf document, what scnprintf returns what
> is actually written not counting the '\0' end char. This results in the
> case of len exceeding the size, len set to PAGE_SIZE - 1, as it can be
> written at max PAGESIZE - 1 (as '\0' is not counted)
>
> Because of len is never set to PAGE_SIZE, the function never break early,
> never print the warning and never return -EFBIG.
>
> Fix this by fixing the condition to PAGE_SIZE -1 to correctly trigger
> the error condition.
>
> Cc: stable@vger.kernel.org
> Fixes: 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential buffer overflow")
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> drivers/cpufreq/cpufreq_stats.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> index a33df3c66c88..40a9ff18da06 100644
> --- a/drivers/cpufreq/cpufreq_stats.c
> +++ b/drivers/cpufreq/cpufreq_stats.c
> @@ -131,23 +131,23 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
> len += sysfs_emit_at(buf, len, " From : To\n");
> len += sysfs_emit_at(buf, len, " : ");
> for (i = 0; i < stats->state_num; i++) {
> - if (len >= PAGE_SIZE)
> + if (len >= PAGE_SIZE - 1)
> break;
> len += sysfs_emit_at(buf, len, "%9u ", stats->freq_table[i]);
> }
> - if (len >= PAGE_SIZE)
> - return PAGE_SIZE;
> + if (len >= PAGE_SIZE - 1)
> + return PAGE_SIZE - 1;
>
> len += sysfs_emit_at(buf, len, "\n");
>
> for (i = 0; i < stats->state_num; i++) {
> - if (len >= PAGE_SIZE)
> + if (len >= PAGE_SIZE - 1)
> break;
>
> len += sysfs_emit_at(buf, len, "%9u: ", stats->freq_table[i]);
>
> for (j = 0; j < stats->state_num; j++) {
> - if (len >= PAGE_SIZE)
> + if (len >= PAGE_SIZE - 1)
> break;
>
> if (pending)
> @@ -157,12 +157,12 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
>
> len += sysfs_emit_at(buf, len, "%9u ", count);
> }
> - if (len >= PAGE_SIZE)
> + if (len >= PAGE_SIZE - 1)
> break;
> len += sysfs_emit_at(buf, len, "\n");
> }
>
> - if (len >= PAGE_SIZE) {
> + if (len >= PAGE_SIZE - 1) {
> pr_warn_once("cpufreq transition table exceeds PAGE_SIZE. Disabling\n");
> return -EFBIG;
> }
> --
Applied (with some edits in the subject and changelog) as 6.7 material, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats
2023-10-24 20:03 ` [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Rafael J. Wysocki
@ 2023-10-26 10:53 ` Christian Marangi
2023-10-26 11:22 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Christian Marangi @ 2023-10-26 10:53 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Viresh Kumar, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Takashi Iwai, Jonghwa Lee, linux-kernel, linux-pm, stable
On Tue, Oct 24, 2023 at 10:03:35PM +0200, Rafael J. Wysocki wrote:
> On Tue, Oct 24, 2023 at 8:30 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > Commit 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential
> > buffer overflow") switched from snprintf to the more secure scnprintf
> > but never updated the exit condition for PAGE_SIZE.
> >
> > As the commit say and as scnprintf document, what scnprintf returns what
> > is actually written not counting the '\0' end char. This results in the
> > case of len exceeding the size, len set to PAGE_SIZE - 1, as it can be
> > written at max PAGESIZE - 1 (as '\0' is not counted)
> >
> > Because of len is never set to PAGE_SIZE, the function never break early,
> > never print the warning and never return -EFBIG.
> >
> > Fix this by fixing the condition to PAGE_SIZE -1 to correctly trigger
> > the error condition.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential buffer overflow")
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> > drivers/cpufreq/cpufreq_stats.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> > index a33df3c66c88..40a9ff18da06 100644
> > --- a/drivers/cpufreq/cpufreq_stats.c
> > +++ b/drivers/cpufreq/cpufreq_stats.c
> > @@ -131,23 +131,23 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
> > len += sysfs_emit_at(buf, len, " From : To\n");
> > len += sysfs_emit_at(buf, len, " : ");
> > for (i = 0; i < stats->state_num; i++) {
> > - if (len >= PAGE_SIZE)
> > + if (len >= PAGE_SIZE - 1)
> > break;
> > len += sysfs_emit_at(buf, len, "%9u ", stats->freq_table[i]);
> > }
> > - if (len >= PAGE_SIZE)
> > - return PAGE_SIZE;
> > + if (len >= PAGE_SIZE - 1)
> > + return PAGE_SIZE - 1;
> >
> > len += sysfs_emit_at(buf, len, "\n");
> >
> > for (i = 0; i < stats->state_num; i++) {
> > - if (len >= PAGE_SIZE)
> > + if (len >= PAGE_SIZE - 1)
> > break;
> >
> > len += sysfs_emit_at(buf, len, "%9u: ", stats->freq_table[i]);
> >
> > for (j = 0; j < stats->state_num; j++) {
> > - if (len >= PAGE_SIZE)
> > + if (len >= PAGE_SIZE - 1)
> > break;
> >
> > if (pending)
> > @@ -157,12 +157,12 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
> >
> > len += sysfs_emit_at(buf, len, "%9u ", count);
> > }
> > - if (len >= PAGE_SIZE)
> > + if (len >= PAGE_SIZE - 1)
> > break;
> > len += sysfs_emit_at(buf, len, "\n");
> > }
> >
> > - if (len >= PAGE_SIZE) {
> > + if (len >= PAGE_SIZE - 1) {
> > pr_warn_once("cpufreq transition table exceeds PAGE_SIZE. Disabling\n");
> > return -EFBIG;
> > }
> > --
>
> Applied (with some edits in the subject and changelog) as 6.7 material, thanks!
Hi, I just notice this landed in linux-next but I can't find the devfreq
change. Only the cpufreq patch has been taken and the devfreq ones are
still pending?
--
Ansuel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats
2023-10-26 10:53 ` Christian Marangi
@ 2023-10-26 11:22 ` Rafael J. Wysocki
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2023-10-26 11:22 UTC (permalink / raw)
To: Christian Marangi
Cc: Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Takashi Iwai, Jonghwa Lee, linux-kernel, linux-pm,
stable
On Thu, Oct 26, 2023 at 12:54 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Tue, Oct 24, 2023 at 10:03:35PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Oct 24, 2023 at 8:30 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
> > >
> > > Commit 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential
> > > buffer overflow") switched from snprintf to the more secure scnprintf
> > > but never updated the exit condition for PAGE_SIZE.
> > >
> > > As the commit say and as scnprintf document, what scnprintf returns what
> > > is actually written not counting the '\0' end char. This results in the
> > > case of len exceeding the size, len set to PAGE_SIZE - 1, as it can be
> > > written at max PAGESIZE - 1 (as '\0' is not counted)
> > >
> > > Because of len is never set to PAGE_SIZE, the function never break early,
> > > never print the warning and never return -EFBIG.
> > >
> > > Fix this by fixing the condition to PAGE_SIZE -1 to correctly trigger
> > > the error condition.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential buffer overflow")
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > > drivers/cpufreq/cpufreq_stats.c | 14 +++++++-------
> > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> > > index a33df3c66c88..40a9ff18da06 100644
> > > --- a/drivers/cpufreq/cpufreq_stats.c
> > > +++ b/drivers/cpufreq/cpufreq_stats.c
> > > @@ -131,23 +131,23 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
> > > len += sysfs_emit_at(buf, len, " From : To\n");
> > > len += sysfs_emit_at(buf, len, " : ");
> > > for (i = 0; i < stats->state_num; i++) {
> > > - if (len >= PAGE_SIZE)
> > > + if (len >= PAGE_SIZE - 1)
> > > break;
> > > len += sysfs_emit_at(buf, len, "%9u ", stats->freq_table[i]);
> > > }
> > > - if (len >= PAGE_SIZE)
> > > - return PAGE_SIZE;
> > > + if (len >= PAGE_SIZE - 1)
> > > + return PAGE_SIZE - 1;
> > >
> > > len += sysfs_emit_at(buf, len, "\n");
> > >
> > > for (i = 0; i < stats->state_num; i++) {
> > > - if (len >= PAGE_SIZE)
> > > + if (len >= PAGE_SIZE - 1)
> > > break;
> > >
> > > len += sysfs_emit_at(buf, len, "%9u: ", stats->freq_table[i]);
> > >
> > > for (j = 0; j < stats->state_num; j++) {
> > > - if (len >= PAGE_SIZE)
> > > + if (len >= PAGE_SIZE - 1)
> > > break;
> > >
> > > if (pending)
> > > @@ -157,12 +157,12 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
> > >
> > > len += sysfs_emit_at(buf, len, "%9u ", count);
> > > }
> > > - if (len >= PAGE_SIZE)
> > > + if (len >= PAGE_SIZE - 1)
> > > break;
> > > len += sysfs_emit_at(buf, len, "\n");
> > > }
> > >
> > > - if (len >= PAGE_SIZE) {
> > > + if (len >= PAGE_SIZE - 1) {
> > > pr_warn_once("cpufreq transition table exceeds PAGE_SIZE. Disabling\n");
> > > return -EFBIG;
> > > }
> > > --
> >
> > Applied (with some edits in the subject and changelog) as 6.7 material, thanks!
>
> Hi, I just notice this landed in linux-next but I can't find the devfreq
> change. Only the cpufreq patch has been taken and the devfreq ones are
> still pending?
That's correct AFAICS. I've only picked up the cpufreq change.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] PM / devfreq: Fix buffer overflow in trans_stat_show
2023-10-24 18:30 ` [PATCH 2/3] PM / devfreq: Fix buffer overflow in trans_stat_show Christian Marangi
@ 2023-11-02 17:26 ` Christian Marangi
0 siblings, 0 replies; 7+ messages in thread
From: Christian Marangi @ 2023-11-02 17:26 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Takashi Iwai, Jonghwa Lee, linux-kernel, linux-pm
Cc: stable
On Tue, Oct 24, 2023 at 08:30:15PM +0200, Christian Marangi wrote:
> Fix buffer overflow in trans_stat_show().
>
> Convert simple snprintf to the more secure scnprintf with size of
> PAGE_SIZE.
>
> Add condition checking if we are exceeding PAGE_SIZE and exit early from
> loop. Also add at the end a warning that we exceeded PAGE_SIZE and that
> stats is disabled.
>
> Return -EFBIG in the case where we don't have enough space to write the
> full transition table.
>
> Also document in the ABI that this function can return -EFBIG error.
>
> Cc: stable@vger.kernel.org
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218041
> Fixes: e552bbaf5b98 ("PM / devfreq: Add sysfs node for representing frequency transition information.")
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Any news for this?
--
Ansuel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-11-02 17:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-24 18:30 [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Christian Marangi
2023-10-24 18:30 ` [PATCH 2/3] PM / devfreq: Fix buffer overflow in trans_stat_show Christian Marangi
2023-11-02 17:26 ` Christian Marangi
2023-10-24 18:30 ` [PATCH 3/3] PM / devfreq: Convert to use sysfs_emit_at() API Christian Marangi
2023-10-24 20:03 ` [PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats Rafael J. Wysocki
2023-10-26 10:53 ` Christian Marangi
2023-10-26 11:22 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).