Hi Dan,
On Tue, 2020-02-04 at 10:48 +0300, Dan Carpenter wrote:
> On Tue, Feb 04, 2020 at 10:00:09AM +0300, Dan Carpenter wrote:
> > Hello Srinivas Pandruvada,
> >
> > The patch 49a474c7ba51: "platform/x86: Add support for Uncore
> > frequency control" from Jan 13, 2020, leads to the following static
> > checker warning:
> >
> > drivers/platform/x86/intel-uncore-frequency.c:285
> > uncore_remove_die_entry()
> > error: dereferencing freed memory 'data'
> >
> > drivers/platform/x86/intel-uncore-frequency.c
> > 276 /* Last CPU in this die is offline, so remove sysfs entries
> > */
> > 277 static void uncore_remove_die_entry(int cpu)
> > 278 {
> > 279 struct uncore_data *data;
> > 280
> > 281 mutex_lock(&uncore_lock);
> > 282 data = uncore_get_instance(cpu);
> > 283 if (data) {
> > 284 kobject_put(&data->kobj);
> > ^^^^^^^^^^^^^^^^^^^^^^^^
> > This leads to a slightly delayed free.
>
> These static checker warnings are new and I'm still working out the
> kinks. This doesn't actually free anything, but it's not right
> either
> it seems? The uncore_ktype struct doesn't have a release function so
> won't kobject_cleanup() complain?
>
> lib/kobject.c
> 664 static void kobject_cleanup(struct kobject *kobj)
> 665 {
> 666 struct kobj_type *t = get_ktype(kobj);
> 667 const char *name = kobj->name;
> 668
> 669 pr_debug("kobject: '%s' (%p): %s, parent %p\n",
> 670 kobject_name(kobj), kobj, __func__, kobj-
> >parent);
> 671
> 672 if (t && !t->release)
> 673 pr_debug("kobject: '%s' (%p): does not have a
> release() function, it is broken and must be fixed. See
> Documentation/kobject.txt.\n",
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 674 kobject_name(kobj), kobj);
>
There are several usages in kernel without release callback.
Does the attach patch fixes the warning?
Thanks,
Srinivas
> regards,
> dan carpenter
>
From 959ee9fe784d623960df2560fd13cae215b464d4 Mon Sep 17 00:00:00 2001
From: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
Date: Tue, 4 Feb 2020 06:50:11 -0800
Subject: [PATCH] platform/x86/intel-uncore-freq: Add release function
When user mode is reference to the kobject, wait to free the memory
during module unload by adding a release callback.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
---
drivers/platform/x86/intel-uncore-frequency.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel-uncore-frequency.c b/drivers/platform/x86/intel-uncore-frequency.c
index 2b1a0734c3f8..3eda66d0713b 100644
--- a/drivers/platform/x86/intel-uncore-frequency.c
+++ b/drivers/platform/x86/intel-uncore-frequency.c
@@ -217,12 +217,21 @@ static struct attribute *uncore_attrs[] = {
NULL
};
+
+static void uncore_sysfs_entry_release(struct kobject *kobj)
+{
+ if (!uncore_max_entries)
+ kfree(uncore_instances);
+}
+
static struct kobj_type uncore_ktype = {
+ .release = uncore_sysfs_entry_release,
.sysfs_ops = &kobj_sysfs_ops,
.default_attrs = uncore_attrs,
};
static struct kobj_type uncore_root_ktype = {
+ .release = uncore_sysfs_entry_release,
.sysfs_ops = &kobj_sysfs_ops,
};
@@ -281,9 +290,9 @@ static void uncore_remove_die_entry(int cpu)
mutex_lock(&uncore_lock);
data = uncore_get_instance(cpu);
if (data) {
- kobject_put(&data->kobj);
data->control_cpu = -1;
data->valid = false;
+ kobject_put(&data->kobj);
}
mutex_unlock(&uncore_lock);
}
@@ -424,12 +433,14 @@ static void __exit intel_uncore_exit(void)
unregister_pm_notifier(&uncore_pm_nb);
cpuhp_remove_state(uncore_hp_state);
+ mutex_lock(&uncore_lock);
for (i = 0; i < uncore_max_entries; ++i) {
if (uncore_instances[i].valid)
kobject_put(&uncore_instances[i].kobj);
}
+ uncore_max_entries = 0;
kobject_put(&uncore_root_kobj);
- kfree(uncore_instances);
+ mutex_unlock(&uncore_lock);
}
module_exit(intel_uncore_exit)
--
2.20.1
