[Cocci] [PATCH v3] coccinelle: semantic code search for missing of_node_put

[Cocci] [PATCH v3] coccinelle: semantic code search for missing of_node_put

[Wen Yang]

There are functions which increment a reference counter for a device node.
These functions belong to a programming interface for the management
of information from device trees.
The counter must be decremented after the last usage of a device node.
We find these functions by using the following script:

<SmPL>
@initialize:ocaml@
@@

let relevant_str = "use of_node_put() on it when done"

let contains s1 s2 =
    let re = Str.regexp_string s2
    in
        try ignore (Str.search_forward re s1 0); true
        with Not_found -> false

let relevant_functions = ref []

let add_function f c =
    if not (List.mem f !relevant_functions)
    then
      begin
        let s = String.concat " "
          (
            (List.map String.lowercase_ascii
              (List.filter
                (function x ->
                  Str.string_match
                  (Str.regexp "[a-zA-Z_\\(\\)][-a-zA-Z0-9_\\(\\)]*$")
                x 0) (Str.split (Str.regexp "[ .;\t\n]+") c)))) in
             if contains s relevant_str
             then
               Printf.printf "Found relevant function: %s\n" f;
               relevant_functions := f :: !relevant_functions;
      end

@r@
identifier fn;
comments c;
type T = struct device_node *;
@@

T@c fn(...) {
...
}

@script:ocaml@
f << r.fn;
c << r.c;
@@

let (cb,cm,ca) = List.hd c in
let c = String.concat " " cb in
add_function f c
</SmPL>

Then copy the function names found by the above script to the r_miss_put
rule. This rule checks for missing of_node_put.

And this patch also looks for places where an of_node_put() call is on some
paths but not on others (implemented by the r_miss_put_ext rule).

Finally, this patch finds use-after-free issues for a node.
(implemented by the r_use_after_put rule)

Suggested-by: Julia Lawall <Julia.Lawall@lip6.fr>
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Gilles Muller <Gilles.Muller@lip6.fr>
Cc: Nicolas Palix <nicolas.palix@imag.fr>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Wen Yang <yellowriver2010@hotmail.com>
Cc: Markus Elfring <Markus.Elfring@web.de>
Cc: cocci@systeme.lip6.fr
---
v3: delete the global set, add a rule that checks for use-after-free.
v2: improve the commit description and delete duplicate code.

 scripts/coccinelle/free/of_node_put.cocci | 192 ++++++++++++++++++++++++++++++
 1 file changed, 192 insertions(+)
 create mode 100644 scripts/coccinelle/free/of_node_put.cocci

diff --git a/scripts/coccinelle/free/of_node_put.cocci b/scripts/coccinelle/free/of_node_put.cocci
new file mode 100644
index 0000000..cda43fa
--- /dev/null
+++ b/scripts/coccinelle/free/of_node_put.cocci
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Find missing of_node_put
+///
+// Confidence: Moderate
+// Copyright: (C) 2018-2019 Wen Yang, ZTE.
+// Comments:
+// Options: --no-includes --include-headers
+
+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"
+
+@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\|
+         of_get_cpu_node\|
+         of_get_parent\|
+         of_get_next_parent\|
+         of_get_next_child\|
+         of_get_next_cpu_node\|
+         of_get_compatible_child\|
+         of_get_child_by_name\|
+         of_find_node_opts_by_path\|
+         of_find_node_by_name\|
+         of_find_node_by_type\|
+         of_find_compatible_node\|
+         of_find_node_with_property\|
+         of_find_matching_node_and_match\|
+         of_find_node_by_phandle\|
+         of_parse_phandle\|
+         of_find_next_cache_node\|
+         of_get_next_available_child\)(...);
+...
+if (x == NULL || ...) S
+... when != e = (T)x
+    when != of_node_put(x)
+    when != of_get_next_parent(x)
+    when != of_find_matching_node(x, ...)
+    when != if (x) { ... return x; }
+    when != v4l2_async_notifier_add_fwnode_subdev(..., <+...x...+>, ...)
+    when != e1 = of_fwnode_handle(x)
+(
+ if (x) { ... when forall
+         of_node_put(x) ... }
+|
+ return (T1)x;
+|
+ return of_fwnode_handle(x);
+|
+* return@p2 ...;
+)
+
+@script:python depends on report && r_miss_put@
+p1 << r_miss_put.p1;
+p2 << r_miss_put.p2;
+@@
+
+coccilib.report.print_report(p2[0], report_miss_prefix + p1[0].line + report_miss_suffix)
+
+@script:python depends on org && r_miss_put@
+p1 << r_miss_put.p1;
+p2 << r_miss_put.p2;
+@@
+
+cocci.print_main(org_miss_main, p1)
+cocci.print_secs(org_miss_sec, p2)
+
+@r_miss_put_ext exists@
+local idexpression struct device_node *x;
+expression e, e1;
+position p1 != r_miss_put.p1, p2 != r_miss_put.p2;
+identifier f;
+statement S;
+type T, T1;
+@@
+
+(
+* x = f@p1(...);
+... when != e = (T)x
+    when != true x == NULL
+    when != of_node_put(x)
+    when != of_get_next_parent(x)
+    when != of_find_matching_node(x, ...)
+    when != if (x) { ... return x; }
+    when != v4l2_async_notifier_add_fwnode_subdev(..., <+...x...+>, ...)
+    when != e1 = of_fwnode_handle(x)
+(
+ if (x) { ... when forall
+         of_node_put(x) ... }
+|
+ return (T1)x;
+|
+ return of_fwnode_handle(x);
+|
+* return@p2 ...;
+)
+&
+x = f(...)
+...
+if (<+...x...+>) S
+...
+of_node_put(x);
+)
+
+@script:python depends on report && r_miss_put_ext@
+p1 << r_miss_put_ext.p1;
+p2 << r_miss_put_ext.p2;
+@@
+
+coccilib.report.print_report(p2[0], report_miss_prefix + p1[0].line + report_miss_suffix)
+
+@script:python depends on org && r_miss_put_ext@
+p1 << r_miss_put_ext.p1;
+p2 << r_miss_put_ext.p2;
+@@
+cocci.print_main(org_miss_main, p1)
+cocci.print_secs(org_miss_sec, p2)
+
+@r_put@
+expression E;
+position p1;
+@@
+
+* of_node_put@p1(E)
+
+@r_use_after_put exists@
+expression r_put.E, subE<=r_put.E;
+constant char [] c;
+expression E1;
+iterator iter;
+identifier f;
+statement S;
+type T;
+position r_put.p1, p2;
+@@
+
+* of_node_put@p1(E,...)
+...
+(
+  iter(...,subE,...) S
+|
+ subE = (T)E1
+|
+ &(T)subE
+|
+ f(...,c,...,(T)E,...)
+|
+ E == (T)E1
+|
+ E != (T)E1
+|
+ E1 == (T)E
+|
+ E1 != (T)E
+|
+ !E
+|
+ (T)E || ...
+|
+* (T)E@p2
+)
+
+@script:python depends on r_use_after_put && report@
+p1 << r_put.p1;
+p2 << r_use_after_put.p2;
+@@
+
+coccilib.report.print_report(p2[0], report_use_after_put + p1[0].line)
+
+@script:python depends on r_use_after_put && org@
+p1 << r_put.p1;
+p2 << r_use_after_put.p2;
+@@
+
+cocci.print_main(org_use_after_put_main, p1)
+cocci.print_secs(org_use_after_put_sec, p2)
-- 
2.9.5

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH v3] coccinelle: semantic code search for missing of_node_put

[Markus Elfring]

> 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/44be5924-26ca-5106-aa25-3cbc3343aa2c@web.de/
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/904b9362-cd01-ffc9-600b-0c48848617a0@web.de/
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.


> And this patch also looks for places …

Does a SmPL script perform an action?


> 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.


> v3: delete the global set, …

To which previous implementation detail do you refer here?


> +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


> +@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?


> +... when != e = (T)x
> +    when != true x == NULL

Will assignment exclusions get any more software development attention?
https://lore.kernel.org/lkml/03cc4df5-ce7f-ba91-36b5-687fec8c7297@web.de/
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
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH v3] coccinelle: semantic code search for missing of_node_put

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 3867 bytes --]



On Tue, 16 Jul 2019, Markus Elfring wrote:

> > We find these functions by using the following script:
>
> Why would you like to keep this SmPL code in the commit description?

I don't know indetail what you are proposing, but I would prefer not to
put semantic patches that involve iteration into the kernel, for
simplicity.

julia


>
> I would prefer software evolution in an other direction.
> https://lore.kernel.org/lkml/44be5924-26ca-5106-aa25-3cbc3343aa2c@web.de/
> 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/904b9362-cd01-ffc9-600b-0c48848617a0@web.de/
> 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.
>
>
> > And this patch also looks for places …
>
> Does a SmPL script perform an action?
>
>
> > 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.
>
>
> > v3: delete the global set, …
>
> To which previous implementation detail do you refer here?
>
>
> > +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
>
>
> > +@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?
>
>
> > +... when != e = (T)x
> > +    when != true x == NULL
>
> Will assignment exclusions get any more software development attention?
> https://lore.kernel.org/lkml/03cc4df5-ce7f-ba91-36b5-687fec8c7297@web.de/
> 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
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3] coccinelle: semantic code search for missing of_node_put

[Markus Elfring]

>> Why would you like to keep this SmPL code in the commit description?
>
> I don't know indetail what you are proposing,

I imagine that you can get more interesting software development ideas
from links to previous messages.
I hope that the desired clarification can become more constructive.

How are the chances to move such code into SmPL script files?


> but I would prefer not to put semantic patches that involve iteration
> into the kernel, for simplicity.

This view is also interesting.

But I hope that this functionality will become more helpful
if we can agree on value combinations which should be iterated
for powerful source code analysis.


>> I would prefer software evolution in an other direction.
>> https://lore.kernel.org/lkml/44be5924-26ca-5106-aa25-3cbc3343aa2c@web.de/
>> https://lkml.org/lkml/2019/7/4/21

Would you like to add any more advices for affected software components?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3] coccinelle: semantic code search for missing of_node_put

[Markus Elfring]

>> Why would you like to keep this SmPL code in the commit description?
>
> I don't know indetail what you are proposing,

I imagine that you can get more interesting software development ideas
from links to previous messages.
I hope that the desired clarification can become more constructive.

How are the chances to move such code into SmPL script files?


> but I would prefer not to put semantic patches that involve iteration
> into the kernel, for simplicity.

This view is also interesting.

But I hope that this functionality will become more helpful
if we can agree on value combinations which should be iterated
for powerful source code analysis.


>> I would prefer software evolution in an other direction.
>> https://lore.kernel.org/lkml/44be5924-26ca-5106-aa25-3cbc3343aa2c@web.de/
>> https://lkml.org/lkml/2019/7/4/21

Would you like to add any more advices for affected software components?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH v3] coccinelle: semantic code search for missingof_node_put

[wen.yang99]

[-- Attachment #1.1: Type: text/plain, Size: 6283 bytes --]

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/44be5924-26ca-5106-aa25-3cbc3343aa2c@web.de/
> 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/904b9362-cd01-ffc9-600b-0c48848617a0@web.de/
> 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/03cc4df5-ce7f-ba91-36b5-687fec8c7297@web.de/
> 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

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3] coccinelle: semantic code search for missing of_node_put

[Markus Elfring]

> 2), Then SmPL A generates another SmPL B based on the function name list;

This would be a general data processing possibility.
Another option would be to let SmPL scripts to import relevant data
from external files or to query facts from databases.


> You expect the entire process above to be automated.

I hope that this can be achieved finally.


> This idea may be interesting,

Thanks for your feedback.


> but it can't be done now,

I got an other view. - Why is your view so limited at the moment?


> and it will introduce uncontrollable factors.

I suggest to take additional design options into account so that you might get
more control on some factors.
Which software development challenges are still waiting for better solutions?


> We agree with julia's comments:
> I would prefer not to put semantic patches that involve iteration into the kernel, for simplicity.

I guess that this kind of change reluctance can be also adjusted.
Some source code analysis approaches can look simple enough
while advanced ones will show more of the inherent complexity.


> Our file is called of_node_put.cocci, which contains three rules: r_miss_put,
>  r_miss_put_ext and r_use_after_put.

This combination is interesting, isn't it?


> If you separate them, it seems inappropriate.

* Would you like to be able to let each source code analysis task to be executed
  on its own?

* I guess that it can become possible with additional development efforts
  to support also a mixture of analysis patterns.

* The patch subject “… missing …” does probably not fit to the detection “use after …”.


>>> 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

I would find an other description clearer then.
* Drop of functions around “add_if_not_present”
* Omission of iteration functionality


Are any more adjustments worth to be explicitly mentioned in this patch change log?


> Here are some improvements.

Are you going to contribute further patch versions?


> Adding an asterisk here is more convenient to use,

This might be. - I wonder how good additional data fit to supported output formats.


> it can mark the location of the code of interest, such as:

I know its functionality also. - I got the impression that the use of SmPL asterisks
will be safe for the operation mode “context”.


>>> +... when != e = (T)x
>>> +    when != true x == NULL
>>
>> Will assignment exclusions get any more software development attention?
>> https://lore.kernel.org/lkml/03cc4df5-ce7f-ba91-36b5-687fec8c7297@web.de/
>> https://lore.kernel.org/patchwork/patch/1095169/#1291892
>> https://lkml.org/lkml/2019/6/29/193

Will this aspect evolve further anyhow?


>> 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.

Would you like to clarify this software detail any more?


>>> +@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?

More helpful information?


> +|
> + f(...,c,...,(T)E,...)

I would interpret such passing of a pointer for a device node
as an undesirable “use after free (or put)”.
Will this SmPL disjunction need further adjustments?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3] Coccinelle: semantic code search for “use after …”

[Markus Elfring]

> Finally, this patch finds use-after-free issues for a node.
> (implemented by the r_use_after_put rule)

I suggest to take another look also at information from a clarification attempt
on a topic like “Checking statement order for patch generation with SmPL support”.
https://systeme.lip6.fr/pipermail/cocci/2017-September/004483.html
https://lore.kernel.org/cocci/alpine.DEB.2.20.1709071519240.3168@hadrien/

Under which circumstances will it become safer to develop SmPL script variants
for such source code search patterns?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3] Coccinelle: semantic code search for “use after …”

[Markus Elfring]

> Finally, this patch finds use-after-free issues for a node.
> (implemented by the r_use_after_put rule)

I suggest to take another look also at information from a clarification attempt
on a topic like “Checking statement order for patch generation with SmPL support”.
https://systeme.lip6.fr/pipermail/cocci/2017-September/004483.html
https://lore.kernel.org/cocci/alpine.DEB.2.20.1709071519240.3168@hadrien/

Under which circumstances will it become safer to develop SmPL script variants
for such source code search patterns?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci