Re: [PATCH] cpufreq: update to sysfs_emit for safer buffer handling

[Mario Limonciello <mario.limonciello@amd.com>]

On 6/19/2024 03:15, Perry Yuan wrote:
> Replaced sprintf and scnprintf with sysfs_emit and sysfs_emit_at in the
> cpufreq driver. This ensures safer buffer handling and consistency with
> sysfs interfaces. Updated show_scaling_available_governors and related
> functions for compliance with the new API.
> 
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

> ---
>   drivers/cpufreq/cpufreq.c | 37 ++++++++++++++++++-------------------
>   1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index e1a4730f4f8c..e76d8e2b4c87 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -608,7 +608,7 @@ EXPORT_SYMBOL_GPL(cpufreq_policy_transition_delay_us);
>   static ssize_t show_boost(struct kobject *kobj,
>   			  struct kobj_attribute *attr, char *buf)
>   {
> -	return sprintf(buf, "%d\n", cpufreq_driver->boost_enabled);
> +	return sysfs_emit(buf, "%d\n", cpufreq_driver->boost_enabled);
>   }
>   
>   static ssize_t store_boost(struct kobject *kobj, struct kobj_attribute *attr,
> @@ -738,7 +738,7 @@ static struct cpufreq_governor *cpufreq_parse_governor(char *str_governor)
>   static ssize_t show_##file_name				\
>   (struct cpufreq_policy *policy, char *buf)		\
>   {							\
> -	return sprintf(buf, "%u\n", policy->object);	\
> +	return sysfs_emit(buf, "%u\n", policy->object);	\
>   }
>   
>   show_one(cpuinfo_min_freq, cpuinfo.min_freq);
> @@ -759,11 +759,11 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
>   
>   	freq = arch_freq_get_on_cpu(policy->cpu);
>   	if (freq)
> -		ret = sprintf(buf, "%u\n", freq);
> +		ret = sysfs_emit(buf, "%u\n", freq);
>   	else if (cpufreq_driver->setpolicy && cpufreq_driver->get)
> -		ret = sprintf(buf, "%u\n", cpufreq_driver->get(policy->cpu));
> +		ret = sysfs_emit(buf, "%u\n", cpufreq_driver->get(policy->cpu));
>   	else
> -		ret = sprintf(buf, "%u\n", policy->cur);
> +		ret = sysfs_emit(buf, "%u\n", policy->cur);
>   	return ret;
>   }
>   
> @@ -797,9 +797,9 @@ static ssize_t show_cpuinfo_cur_freq(struct cpufreq_policy *policy,
>   	unsigned int cur_freq = __cpufreq_get(policy);
>   
>   	if (cur_freq)
> -		return sprintf(buf, "%u\n", cur_freq);
> +		return sysfs_emit(buf, "%u\n", cur_freq);
>   
> -	return sprintf(buf, "<unknown>\n");
> +	return sysfs_emit(buf, "<unknown>\n");
>   }
>   
>   /*
> @@ -808,12 +808,11 @@ static ssize_t show_cpuinfo_cur_freq(struct cpufreq_policy *policy,
>   static ssize_t show_scaling_governor(struct cpufreq_policy *policy, char *buf)
>   {
>   	if (policy->policy == CPUFREQ_POLICY_POWERSAVE)
> -		return sprintf(buf, "powersave\n");
> +		return sysfs_emit(buf, "powersave\n");
>   	else if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
> -		return sprintf(buf, "performance\n");
> +		return sysfs_emit(buf, "performance\n");
>   	else if (policy->governor)
> -		return scnprintf(buf, CPUFREQ_NAME_PLEN, "%s\n",
> -				policy->governor->name);
> +		return sysfs_emit(buf, "%s\n", policy->governor->name);
>   	return -EINVAL;
>   }
>   
> @@ -872,7 +871,7 @@ static ssize_t show_scaling_available_governors(struct cpufreq_policy *policy,
>   	struct cpufreq_governor *t;
>   
>   	if (!has_target()) {
> -		i += sprintf(buf, "performance powersave");
> +		i += sysfs_emit(buf, "performance powersave");
>   		goto out;
>   	}
>   
> @@ -881,11 +880,11 @@ static ssize_t show_scaling_available_governors(struct cpufreq_policy *policy,
>   		if (i >= (ssize_t) ((PAGE_SIZE / sizeof(char))
>   		    - (CPUFREQ_NAME_LEN + 2)))
>   			break;
> -		i += scnprintf(&buf[i], CPUFREQ_NAME_PLEN, "%s ", t->name);
> +		i += sysfs_emit_at(buf, i, "%s ", t->name);
>   	}
>   	mutex_unlock(&cpufreq_governor_mutex);
>   out:
> -	i += sprintf(&buf[i], "\n");
> +	i += sysfs_emit_at(buf, i, "\n");
>   	return i;
>   }
>   
> @@ -895,7 +894,7 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
>   	unsigned int cpu;
>   
>   	for_each_cpu(cpu, mask) {
> -		i += scnprintf(&buf[i], (PAGE_SIZE - i - 2), "%u ", cpu);
> +		i += sysfs_emit_at(buf, i, "%u ", cpu);
>   		if (i >= (PAGE_SIZE - 5))
>   			break;
>   	}
> @@ -903,7 +902,7 @@ ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
>   	/* Remove the extra space at the end */
>   	i--;
>   
> -	i += sprintf(&buf[i], "\n");
> +	i += sysfs_emit_at(buf, i, "\n");
>   	return i;
>   }
>   EXPORT_SYMBOL_GPL(cpufreq_show_cpus);
> @@ -946,7 +945,7 @@ static ssize_t store_scaling_setspeed(struct cpufreq_policy *policy,
>   static ssize_t show_scaling_setspeed(struct cpufreq_policy *policy, char *buf)
>   {
>   	if (!policy->governor || !policy->governor->show_setspeed)
> -		return sprintf(buf, "<unsupported>\n");
> +		return sysfs_emit(buf, "<unsupported>\n");
>   
>   	return policy->governor->show_setspeed(policy, buf);
>   }
> @@ -960,8 +959,8 @@ static ssize_t show_bios_limit(struct cpufreq_policy *policy, char *buf)
>   	int ret;
>   	ret = cpufreq_driver->bios_limit(policy->cpu, &limit);
>   	if (!ret)
> -		return sprintf(buf, "%u\n", limit);
> -	return sprintf(buf, "%u\n", policy->cpuinfo.max_freq);
> +		return sysfs_emit(buf, "%u\n", limit);
> +	return sysfs_emit(buf, "%u\n", policy->cpuinfo.max_freq);
>   }
>   
>   cpufreq_freq_attr_ro_perm(cpuinfo_cur_freq, 0400);