On Friday, January 25, 2019 8:23:07 AM CET Viresh Kumar wrote:
> The cpufreq_global_kobject is created using kobject_create_and_add()
> helper, which assigns the kobj_type as dynamic_kobj_ktype and show/store
> routines are set to kobj_attr_show() and kobj_attr_store().
>
> These routines pass struct kobj_attribute as an argument to the
> show/store callbacks. But all the cpufreq files created using the
> cpufreq_global_kobject expect the argument to be of type struct
> attribute. Things work fine currently as no one accesses the "attr"
> argument. We may not see issues even if the argument is used, as struct
> kobj_attribute has struct attribute as its first element and so they
> will both get same address.
>
> But this is logically incorrect and we should rather use struct
> kobj_attribute instead of struct global_attr in the cpufreq core and
> drivers and the show/store callbacks should take struct kobj_attribute
> as argument instead.
>
> This bug is caught using CFI CLANG builds in android kernel which
> catches mismatch in function prototypes for such callbacks.
>
> Reported-by: Donghee Han <dh....@samsung.com>
> Reported-by: Sangkyu Kim <skwith....@samsung.com>
> Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 6 +++---
> drivers/cpufreq/intel_pstate.c | 23 ++++++++++++-----------
> include/linux/cpufreq.h | 12 ++----------
> 3 files changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index e35a886e00bc..ef0e33e21b98 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -545,13 +545,13 @@ EXPORT_SYMBOL_GPL(cpufreq_policy_transition_delay_us);
> * SYSFS INTERFACE *
> *********************************************************************/
> static ssize_t show_boost(struct kobject *kobj,
> - struct attribute *attr, char *buf)
> + struct kobj_attribute *attr, char *buf)
> {
> return sprintf(buf, "%d\n", cpufreq_driver->boost_enabled);
> }
>
> -static ssize_t store_boost(struct kobject *kobj, struct attribute *attr,
> - const char *buf, size_t count)
> +static ssize_t store_boost(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> {
> int ret, enable;
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index dd66decf2087..5ab6a4fe93aa 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -895,7 +895,7 @@ static void intel_pstate_update_policies(void)
> /************************** sysfs begin ************************/
> #define show_one(file_name, object) \
> static ssize_t show_##file_name \
> - (struct kobject *kobj, struct attribute *attr, char *buf) \
> + (struct kobject *kobj, struct kobj_attribute *attr, char *buf) \
> { \
> return sprintf(buf, "%u\n", global.object); \
> }
> @@ -904,7 +904,7 @@ static ssize_t intel_pstate_show_status(char *buf);
> static int intel_pstate_update_status(const char *buf, size_t size);
>
> static ssize_t show_status(struct kobject *kobj,
> - struct attribute *attr, char *buf)
> + struct kobj_attribute *attr, char *buf)
> {
> ssize_t ret;
>
> @@ -915,7 +915,7 @@ static ssize_t show_status(struct kobject *kobj,
> return ret;
> }
>
> -static ssize_t store_status(struct kobject *a, struct attribute *b,
> +static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
> const char *buf, size_t count)
> {
> char *p = memchr(buf, '\n', count);
> @@ -929,7 +929,7 @@ static ssize_t store_status(struct kobject *a, struct
> attribute *b,
> }
>
> static ssize_t show_turbo_pct(struct kobject *kobj,
> - struct attribute *attr, char *buf)
> + struct kobj_attribute *attr, char *buf)
> {
> struct cpudata *cpu;
> int total, no_turbo, turbo_pct;
> @@ -955,7 +955,7 @@ static ssize_t show_turbo_pct(struct kobject *kobj,
> }
>
> static ssize_t show_num_pstates(struct kobject *kobj,
> - struct attribute *attr, char *buf)
> + struct kobj_attribute *attr, char *buf)
> {
> struct cpudata *cpu;
> int total;
> @@ -976,7 +976,7 @@ static ssize_t show_num_pstates(struct kobject *kobj,
> }
>
> static ssize_t show_no_turbo(struct kobject *kobj,
> - struct attribute *attr, char *buf)
> + struct kobj_attribute *attr, char *buf)
> {
> ssize_t ret;
>
> @@ -998,7 +998,7 @@ static ssize_t show_no_turbo(struct kobject *kobj,
> return ret;
> }
>
> -static ssize_t store_no_turbo(struct kobject *a, struct attribute *b,
> +static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b,
> const char *buf, size_t count)
> {
> unsigned int input;
> @@ -1045,7 +1045,7 @@ static ssize_t store_no_turbo(struct kobject *a, struct
> attribute *b,
> return count;
> }
>
> -static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b,
> +static ssize_t store_max_perf_pct(struct kobject *a, struct kobj_attribute
> *b,
> const char *buf, size_t count)
> {
> unsigned int input;
> @@ -1075,7 +1075,7 @@ static ssize_t store_max_perf_pct(struct kobject *a,
> struct attribute *b,
> return count;
> }
>
> -static ssize_t store_min_perf_pct(struct kobject *a, struct attribute *b,
> +static ssize_t store_min_perf_pct(struct kobject *a, struct kobj_attribute
> *b,
> const char *buf, size_t count)
> {
> unsigned int input;
> @@ -1107,12 +1107,13 @@ static ssize_t store_min_perf_pct(struct kobject *a,
> struct attribute *b,
> }
>
> static ssize_t show_hwp_dynamic_boost(struct kobject *kobj,
> - struct attribute *attr, char *buf)
> + struct kobj_attribute *attr, char *buf)
> {
> return sprintf(buf, "%u\n", hwp_boost);
> }
>
> -static ssize_t store_hwp_dynamic_boost(struct kobject *a, struct attribute
> *b,
> +static ssize_t store_hwp_dynamic_boost(struct kobject *a,
> + struct kobj_attribute *b,
> const char *buf, size_t count)
> {
> unsigned int input;
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index c86d6d8bdfed..0b427d5df0fe 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -254,20 +254,12 @@ __ATTR(_name, 0644, show_##_name, store_##_name)
> static struct freq_attr _name = \
> __ATTR(_name, 0200, NULL, store_##_name)
>
> -struct global_attr {
> - struct attribute attr;
> - ssize_t (*show)(struct kobject *kobj,
> - struct attribute *attr, char *buf);
> - ssize_t (*store)(struct kobject *a, struct attribute *b,
> - const char *c, size_t count);
> -};
> -
> #define define_one_global_ro(_name) \
> -static struct global_attr _name = \
> +static struct kobj_attribute _name = \
> __ATTR(_name, 0444, show_##_name, NULL)
>
> #define define_one_global_rw(_name) \
> -static struct global_attr _name = \
> +static struct kobj_attribute _name = \
> __ATTR(_name, 0644, show_##_name, store_##_name)
>
>
>
Applied, thanks!
