| Message ID | c4765377-7830-44c2-84fa-706b6e304e10@stanley.mountain |
|---|---|
| State | New |
| Headers | show |
| Series | [bug,report] cpufreq: CPPC: Fix possible null-ptr-deref for cppc_get_cpu_cost() | expand |
On 2024/11/1 19:23, Dan Carpenter wrote: > Hello Jinjie Ruan, > > Commit 1a1374bb8c59 ("cpufreq: CPPC: Fix possible null-ptr-deref for > cppc_get_cpu_cost()") from Oct 30, 2024 (linux-next), leads to the > following Smatch static checker warning: > > kernel/power/energy_model.c:254 em_compute_costs() > error: uninitialized symbol 'cost'. Thank you, will fix it. > > kernel/power/energy_model.c > 241 static int em_compute_costs(struct device *dev, struct em_perf_state *table, > 242 struct em_data_callback *cb, int nr_states, > 243 unsigned long flags) > 244 { > 245 unsigned long prev_cost = ULONG_MAX; > 246 int i, ret; > 247 > 248 /* Compute the cost of each performance state. */ > 249 for (i = nr_states - 1; i >= 0; i--) { > 250 unsigned long power_res, cost; > 251 > 252 if ((flags & EM_PERF_DOMAIN_ARTIFICIAL) && cb->get_cost) { > 253 ret = cb->get_cost(dev, table[i].frequency, &cost); > --> 254 if (ret || !cost || cost > EM_MAX_POWER) { > ^^^^ > cost can be uninitialized. > > 255 dev_err(dev, "EM: invalid cost %lu %d\n", > 256 cost, ret); > 257 return -EINVAL; > 258 } > 259 } else { > 260 /* increase resolution of 'cost' precision */ > 261 power_res = table[i].power * 10; > 262 cost = power_res / table[i].performance; > 263 } > 264 > 265 table[i].cost = cost; > 266 > 267 if (table[i].cost >= prev_cost) { > 268 table[i].flags = EM_PERF_STATE_INEFFICIENT; > 269 dev_dbg(dev, "EM: OPP:%lu is inefficient\n", > 270 table[i].frequency); > 271 } else { > 272 prev_cost = table[i].cost; > 273 } > 274 } > 275 > 276 return 0; > 277 } > > The commit added a new success path: > > commit 1a1374bb8c5926674973d849feed500bc61ad535 > Author: Jinjie Ruan <ruanjinjie@huawei.com> > Date: Wed Oct 30 16:24:49 2024 +0800 > > cpufreq: CPPC: Fix possible null-ptr-deref for cppc_get_cpu_cost() > > cpufreq_cpu_get_raw() may return NULL if the cpu is not in > policy->cpus cpu mask and it will cause null pointer dereference, > so check NULL for cppc_get_cpu_cost(). > > Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c > index 309cca11b239..956d672c6d57 100644 > --- a/drivers/cpufreq/cppc_cpufreq.c > +++ b/drivers/cpufreq/cppc_cpufreq.c > @@ -474,6 +474,9 @@ static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz, > int step; > > policy = cpufreq_cpu_get_raw(cpu_dev->id); > + if (!policy) > + return 0; > + > cpu_data = policy->driver_data; > perf_caps = &cpu_data->perf_caps; > max_cap = arch_scale_cpu_capacity(cpu_dev->id); > > regards, > dan carpenter
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 309cca11b239..956d672c6d57 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -474,6 +474,9 @@ static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz, int step; policy = cpufreq_cpu_get_raw(cpu_dev->id); + if (!policy) + return 0; + cpu_data = policy->driver_data; perf_caps = &cpu_data->perf_caps; max_cap = arch_scale_cpu_capacity(cpu_dev->id);
Hello Jinjie Ruan, Commit 1a1374bb8c59 ("cpufreq: CPPC: Fix possible null-ptr-deref for cppc_get_cpu_cost()") from Oct 30, 2024 (linux-next), leads to the following Smatch static checker warning: kernel/power/energy_model.c:254 em_compute_costs() error: uninitialized symbol 'cost'. kernel/power/energy_model.c 241 static int em_compute_costs(struct device *dev, struct em_perf_state *table, 242 struct em_data_callback *cb, int nr_states, 243 unsigned long flags) 244 { 245 unsigned long prev_cost = ULONG_MAX; 246 int i, ret; 247 248 /* Compute the cost of each performance state. */ 249 for (i = nr_states - 1; i >= 0; i--) { 250 unsigned long power_res, cost; 251 252 if ((flags & EM_PERF_DOMAIN_ARTIFICIAL) && cb->get_cost) { 253 ret = cb->get_cost(dev, table[i].frequency, &cost); --> 254 if (ret || !cost || cost > EM_MAX_POWER) { ^^^^ cost can be uninitialized. 255 dev_err(dev, "EM: invalid cost %lu %d\n", 256 cost, ret); 257 return -EINVAL; 258 } 259 } else { 260 /* increase resolution of 'cost' precision */ 261 power_res = table[i].power * 10; 262 cost = power_res / table[i].performance; 263 } 264 265 table[i].cost = cost; 266 267 if (table[i].cost >= prev_cost) { 268 table[i].flags = EM_PERF_STATE_INEFFICIENT; 269 dev_dbg(dev, "EM: OPP:%lu is inefficient\n", 270 table[i].frequency); 271 } else { 272 prev_cost = table[i].cost; 273 } 274 } 275 276 return 0; 277 } The commit added a new success path: commit 1a1374bb8c5926674973d849feed500bc61ad535 Author: Jinjie Ruan <ruanjinjie@huawei.com> Date: Wed Oct 30 16:24:49 2024 +0800 cpufreq: CPPC: Fix possible null-ptr-deref for cppc_get_cpu_cost() cpufreq_cpu_get_raw() may return NULL if the cpu is not in policy->cpus cpu mask and it will cause null pointer dereference, so check NULL for cppc_get_cpu_cost(). Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> regards, dan carpenter