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; }

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!