Hi Markus,
> > We find these functions by using the following script:
>
> Why would you like to keep this SmPL code in the commit description?
>
> I would prefer software evolution in an other direction.
> https://lore.kernel.org/lkml/[email protected]/
> https://lkml.org/lkml/2019/7/4/21
> > @initialize:ocaml@
> > @@
> >
> > let relevant_str = "use of_node_put() on it when done"
>
> I see further possibilities to improve this data processing approach.
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/patchwork/patch/1095169/#1291378
> https://lkml.org/lkml/2019/6/28/326
>
>
> I am missing more constructive answers for mentioned development concerns.
>
Let me try to guess what you mean:
1), Provides a SmPL A that parses the annotations of a particular kernel file
and extracts a list of function names to be followed;
2), Then SmPL A generates another SmPL B based on the function name list;
3), Finally SmPL A executes SmPL B on the entire kernel code, looking for the
missing of_node_put.
You expect the entire process above to be automated.
This idea may be interesting, but it can't be done now, and it will introduce
uncontrollable factors.
We agree with julia's comments:
I would prefer not to put semantic patches that involve iteration into the
kernel, for simplicity.
>
> > And this patch also looks for places …
>
> Does a SmPL script perform an action?
>
Thanks.
We'll continue to improve the description here.
> > Finally, this patch finds use-after-free issues for a node.
> > (implemented by the r_use_after_put rule)
>
> This software extension is another interesting contribution.
> But I imagine that a separate SmPL script can be more helpful for
> this source code search pattern.
We found that adding the missing of_node_put may cause use-after-free
if not handled properly, so we have added a new r_use_after_put to detect it.
Our file is called of_node_put.cocci, which contains three rules: r_miss_put,
r_miss_put_ext and r_use_after_put.
If you separate them, it seems inappropriate.
> > v3: delete the global set, …
>
> To which previous implementation detail do you refer here?
Here is an improvement based on julia's comments:
https://lkml.org/lkml/2019/7/5/55
We are very grateful to her.
This is a real, valuable comment that can be applied in code practice.
> > +virtual report
> > +virtual org
> > +
> > +@initialize:python@
> > +@@
> > +
> > +report_miss_prefix = "ERROR: missing of_node_put; acquired a node pointer
> > with refcount incremented on line "
> > +report_miss_suffix = ", but without a corresponding object release within
> > this function."
> > +org_miss_main = "acquired a node pointer with refcount incremented"
> > +org_miss_sec = "needed of_node_put"
> > +report_use_after_put = "ERROR: use-after-free; reference preceded by
> > of_node_put on line "
> > +org_use_after_put_main = "of_node_put"
> > +org_use_after_put_sec = "reference"
>
> If you would insist on the usage of these variables, they should be applied
> only for the selected analysis operation mode.
> I would expect corresponding SmPL dependency specifications.
> https://github.com/coccinelle/coccinelle/blob/b4509f6e7fb06d5616bb19dd5a110b203fd0e566/docs/manual/cocci_syntax.tex#L559
>
Thanks.
Here are some improvements.
> > +@r_miss_put exists@
> > +local idexpression struct device_node *x;
> > +expression e, e1;
> > +position p1, p2;
> > +statement S;
> > +type T, T1;
> > +@@
> > +
> > +* x = @p1\(of_find_all_nodes\|
>
> The usage of the SmPL asterisk functionality can fit to the operation mode
> “context”.
> https://bottest.wiki.kernel.org/coccicheck#modes
> Would you like to add any corresponding SmPL details?
>
> Under which circumstances will remaining programming concerns be clarified
> for such SmPL disjunctions?
Adding an asterisk here is more convenient to use,
it can mark the location of the code of interest, such as:
$ /usr/local/bin/spatch -D report --cocci-file
scripts/coccinelle/free/of_node_put.cocci arch/arm/mach-axxia/platsmp.c
init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
HANDLING: arch/arm/mach-axxia/platsmp.c
arch/arm/mach-axxia/platsmp.c:43:2-8: ERROR: missing of_node_put; acquired a
node pointer with refcount incremented on line 37, but without a corresponding
object release within this function.
arch/arm/mach-axxia/platsmp.c:50:1-7: ERROR: missing of_node_put; acquired a
node pointer with refcount incremented on line 37, but without a corresponding
object release within this function.
diff =
--- arch/arm/mach-axxia/platsmp.c
+++ /tmp/cocci-output-13026-88f3a1-platsmp.c
@@ -34,20 +34,17 @@ static int axxia_boot_secondary(unsigned
void __iomem *syscon;
u32 tmp;
- syscon_np = of_find_compatible_node(NULL, NULL, "lsi,axxia-syscon");
if (!syscon_np)
return -ENOENT;
syscon = of_iomap(syscon_np, 0);
if (!syscon)
- return -ENOMEM;
tmp = readl(syscon + SC_RST_CPU_HOLD);
writel(0xab, syscon + SC_CRIT_WRITE_KEY);
tmp &= ~(1 << cpu);
writel(tmp, syscon + SC_RST_CPU_HOLD);
- return 0;
}
...
In addition to reading the documentation,
you may also need to do some experiments yourself.
Thank you.
--
Regards,
Wen
> > +... when != e = (T)x
> > + when != true x == NULL
>
> Will assignment exclusions get any more software development attention?
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/patchwork/patch/1095169/#1291892
> https://lkml.org/lkml/2019/6/29/193
>
>
> > + when != of_node_put(x)
> …
> > +)
> > +&
> > +x = f(...)
> > +...
> > +if (<+...x...+>) S
> > +...
> > +of_node_put(x);
> > +)
>
> You propose once more to use a SmPL conjunction in the rule “r_miss_put_ext”.
> I am also still waiting for a definitive explanation on the applicability
> of this combination.
>
>
> > +@r_put@
> > +expression E;
> > +position p1;
> > +@@
> > +
> > +* of_node_put@p1(E)
>
> I guess that this SmPL code will need further adjustments.
>
>
> > +@r_use_after_put exists@
> > +expression r_put.E, subE<=r_put.E;
>
> I have got an understanding difficulty around the interpretation
> of the shown SmPL constraint.
> How will the clarification be continued?
>
> Regards,
> Markus_______________________________________________
Cocci mailing list
[email protected]
https://systeme.lip6.fr/mailman/listinfo/cocci
