Le 18/05/2023 à 09:27, Dan Carpenter a écrit :
I sometimes have more ideas than I can handle in a day and my mind
resets every night so every morning I am a blank slate like that guy
from that movie.
So I'm going to start writing them down and adding a hashtag.
#StaticCheckerIdeas I'm going to CC kernel janitors until the Smatch
list gets archived on lore.kernel.org
There is a bug in ti_sci_probe() where it does:
info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
...
list_add_tail(&info->node, &ti_sci_list);
...
return of_platform_populate(dev->of_node, NULL, NULL, dev);
Idea 1: If of_platform_populate() fails, then info freed but it's it's
still on the the &ti_sci_list list. Look for list_add of devm_ data
and ensure that it's removed from the list before an error code.
There are other lifetime issues surrounding devm_ data. We need a
list_del in the release function as well.
Idea 2: The other error paths all have clean up so last error path
should have clean up too.
regards,
dan carpenter
Nice!
A really naive:
@r@
expression m;
identifier alloc_fn =~ "devm_";
identifier list_fn =~ "list_add";
@@
* m = alloc_fn(...);
...
* list_fn(..., <+... m ...+>, ...);
also seems to give some decent results.
Written as-is, there is a lot of false positive, but is a start.
I'll give it a look.
One of my first match is:diff -u -p drivers/perf/qcom_l2_pmu.c
/tmp/nothing/perf/qcom_l2_pmu.c
--- drivers/perf/qcom_l2_pmu.c
+++ /tmp/nothing/perf/qcom_l2_pmu.c
@@ -852,12 +852,10 @@ static int l2_cache_pmu_probe_cluster(st
return err;
}
- cluster = devm_kzalloc(&pdev->dev, sizeof(*cluster), GFP_KERNEL);
if (!cluster)
return -ENOMEM;
INIT_LIST_HEAD(&cluster->next);
- list_add(&cluster->next, &l2cache_pmu->clusters);
cluster->cluster_id = fw_cluster_id;
irq = platform_get_irq(sdev, 0);
which looks like a real one.
list_add() should be at the very end of the probe in order not to
"corrupt" the l2cache_pmu list.
Adding Julia if she wishes to dig it a little more with coccinelle.
CJ
