Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- List of patches to stable: - https://lkml.org/lkml/2020/6/1/713 - https://lkml.org/lkml/2020/6/5/200 - https://lkml.org/lkml/2020/6/5/838 - https://lkml.org/lkml/2020/6/5/887 Other patches: - https://lkml.org/lkml/2020/6/1/701 - https://lkml.org/lkml/2020/6/5/839 - https://lkml.org/lkml/2020/6/5/864 - https://lkml.org/lkml/2020/6/5/865 - https://lkml.org/lkml/2020/6/5/895 - https://lkml.org/lkml/2020/6/5/901 There is a false positive that I can't beat: fs/btrfs/send.c:1119:11-12: WARNING: kmalloc is used to allocate this memory at line 1036 scripts/coccinelle/api/kvfree.cocci | 196 ++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 scripts/coccinelle/api/kvfree.cocci diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci new file mode 100644 index 000000000000..e3fa3d0fd2fd --- /dev/null +++ b/scripts/coccinelle/api/kvfree.cocci @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + + +@choice@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...) + ... + } else { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|__vmalloc_node@vok\)(...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...) + ... when != E = E1 + when any + if (\(!E\|E == NULL\)) { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|__vmalloc_node@vok\)(...) + ... + } +) + +// exclude mm/vmalloc.c because of kvmalloc* definitions +@opportunity depends on !patch && !(file in "mm/vmalloc.c")@ +expression E, E1, size; +position p; +@@ + +( +* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... when != E = E1 + when != size = E1 + when any +* if (\(!E\|E == NULL\))@p { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +) + +@vfree depends on !patch@ +expression E; +position k != choice.kok; +position p; +@@ + +* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@p + +@pvfree depends on patch exists@ +expression E; +position k != choice.kok; +@@ + + E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E) ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position v != choice.vok; +position p; +@@ + +* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|__vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|kvfree\)(E) + +@pkfree depends on patch exists@ +expression E; +position v != choice.vok; +@@ + + E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|__vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E) ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position p, k; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...)@k + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p + +@pkvfree depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@script: python depends on report@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +p << opportunity.p; +@@ + +coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc") + +@script: python depends on org@ +p << opportunity.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc") -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Fri, 5 Jun 2020, Denis Efremov wrote: > Check that alloc and free types of functions match each other. Is there a strong reason for putting the choice rule first? It may make things somewhat slower than necessary, if it matches in many places, because the opportunity rule will have to detect that it doesn't care about all of those places. Also, there is no need to exceed 80 characters here. You can put a newline in the middle of a \( ... \) julia > > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > List of patches to stable: > - https://lkml.org/lkml/2020/6/1/713 > - https://lkml.org/lkml/2020/6/5/200 > - https://lkml.org/lkml/2020/6/5/838 > - https://lkml.org/lkml/2020/6/5/887 > > Other patches: > - https://lkml.org/lkml/2020/6/1/701 > - https://lkml.org/lkml/2020/6/5/839 > - https://lkml.org/lkml/2020/6/5/864 > - https://lkml.org/lkml/2020/6/5/865 > - https://lkml.org/lkml/2020/6/5/895 > - https://lkml.org/lkml/2020/6/5/901 > > There is a false positive that I can't beat: > fs/btrfs/send.c:1119:11-12: WARNING: kmalloc is used to allocate > this memory at line 1036 > > scripts/coccinelle/api/kvfree.cocci | 196 ++++++++++++++++++++++++++++ > 1 file changed, 196 insertions(+) > create mode 100644 scripts/coccinelle/api/kvfree.cocci > > diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci > new file mode 100644 > index 000000000000..e3fa3d0fd2fd > --- /dev/null > +++ b/scripts/coccinelle/api/kvfree.cocci > @@ -0,0 +1,196 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/// > +/// Check that kvmalloc'ed memory is freed by kfree functions, > +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree > +/// functions. > +/// > +// Confidence: High > +// Copyright: (C) 2020 Denis Efremov ISPRAS > +// Options: --no-includes --include-headers > +// > + > +virtual patch > +virtual report > +virtual org > +virtual context > + > + > +@choice@ > +expression E, E1; > +position kok, vok; > +@@ > + > +( > + if (...) { > + ... > + E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...) > + ... > + } else { > + ... > + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|__vmalloc_node@vok\)(...) > + ... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...) > + ... when != E = E1 > + when any > + if (\(!E\|E == NULL\)) { > + ... > + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|__vmalloc_node@vok\)(...) > + ... > + } > +) > + > +// exclude mm/vmalloc.c because of kvmalloc* definitions > +@opportunity depends on !patch && !(file in "mm/vmalloc.c")@ > +expression E, E1, size; > +position p; > +@@ > + > +( > +* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p { > + ... > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) > + ... > + } else { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(..., size, ...) > + ... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) > + ... when != E = E1 > + when != size = E1 > + when any > +* if (\(!E\|E == NULL\))@p { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(..., size, ...) > + ... > + } > +) > + > +@vfree depends on !patch@ > +expression E; > +position k != choice.kok; > +position p; > +@@ > + > +* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|kcalloc_node@k\)(...) > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +* \(vfree\|vfree_atomic\|kvfree\)(E)@p > + > +@pvfree depends on patch exists@ > +expression E; > +position k != choice.kok; > +@@ > + > + E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|kcalloc_node@k\)(...) > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +- \(vfree\|vfree_atomic\|kvfree\)(E) > ++ kfree(E) > + > +@kfree depends on !patch@ > +expression E; > +position v != choice.vok; > +position p; > +@@ > + > +* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|__vmalloc_node_range@v\|__vmalloc_node@v\)(...) > + ... when != !is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|kvfree\)(E) > + > +@pkfree depends on patch exists@ > +expression E; > +position v != choice.vok; > +@@ > + > + E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|__vmalloc_node_range@v\|__vmalloc_node@v\)(...) > + ... when != !is_vmalloc_addr(E) > + when any > +- \(kfree\|kvfree\)(E) > ++ vfree(E) > + > +@kvfree depends on !patch@ > +expression E; > +position p, k; > +@@ > + > +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...)@k > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p > + > +@pkvfree depends on patch exists@ > +expression E; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...) > + ... when != is_vmalloc_addr(E) > + when any > +- \(kfree\|vfree\)(E) > ++ kvfree(E) > + > +@script: python depends on report@ > +k << vfree.k; > +p << vfree.p; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +k << vfree.k; > +p << vfree.p; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +k << kvfree.k; > +p << kvfree.p; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +k << kvfree.k; > +p << kvfree.p; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +p << opportunity.p; > +@@ > + > +coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc") > + > +@script: python depends on org@ > +p << opportunity.p; > +@@ > + > +coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc") > -- > 2.26.2 > > _______________________________________________ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On 6/5/20 11:51 PM, Julia Lawall wrote: > Is there a strong reason for putting the choice rule first? It may make > things somewhat slower than necessary, if it matches in many places, > because the opportunity rule will have to detect that it doesn't care > about all of those places. No, I didn't know that order of rules matters. I just checked it, my PC shows no difference in exec time if I swap these rules. "choice" doesn't check the size. "opportunity" is more strict. The motivation for adding 2 rules is that we could recommend to use kvmalloc* only when there is a size condition. At the same time, we should skip all if (...) {kmalloc()} else {vmalloc()}, res = kmalloc() if (!res) {vmalloc()} cases as false positives. It seems that it's not possible to use subexpression rule "expression size <= choice.E" in this case. > Also, there is no need to exceed 80 characters here. You can put a > newline in the middle of a \( ... \) Ok, I will fix it in v2 after all comments/suggestions. Thanks, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Sat, 6 Jun 2020, Denis Efremov wrote: > On 6/5/20 11:51 PM, Julia Lawall wrote: > > Is there a strong reason for putting the choice rule first? It may make > > things somewhat slower than necessary, if it matches in many places, > > because the opportunity rule will have to detect that it doesn't care > > about all of those places. > > No, I didn't know that order of rules matters. I just checked it, my PC > shows no difference in exec time if I swap these rules. OK, probably choice doesn't match in very many places, so there is not much impact. julia > > "choice" doesn't check the size. "opportunity" is more strict. > The motivation for adding 2 rules is that we could recommend to use > kvmalloc* only when there is a size condition. At the same time, we > should skip all if (...) {kmalloc()} else {vmalloc()}, > res = kmalloc() if (!res) {vmalloc()} cases as false positives. > > It seems that it's not possible to use subexpression rule > "expression size <= choice.E" in this case. > > > Also, there is no need to exceed 80 characters here. You can put a > > newline in the middle of a \( ... \) > > Ok, I will fix it in v2 after all comments/suggestions. > > Thanks, > Denis > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On 6/5/20 11:51 PM, Julia Lawall wrote: > Also, there is no need to exceed 80 characters here. You can put a > newline in the middle of a \( ... \) It's required. Looks like it's impossible to break "when" lines. ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... } Thanks, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Sun, 14 Jun 2020, Denis Efremov wrote: > > > On 6/5/20 11:51 PM, Julia Lawall wrote: > > Also, there is no need to exceed 80 characters here. You can put a > > newline in the middle of a \( ... \) > > It's required. Looks like it's impossible to break "when" lines. That's true. Sorry for the noise. julia > > ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... } > > Thanks, > Denis > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On 6/14/20 12:17 PM, Julia Lawall wrote: > > > On Sun, 14 Jun 2020, Denis Efremov wrote: > >> >> >> On 6/5/20 11:51 PM, Julia Lawall wrote: >>> Also, there is no need to exceed 80 characters here. You can put a >>> newline in the middle of a \( ... \) >> >> It's required. Looks like it's impossible to break "when" lines. > > That's true. Sorry for the noise. > Anyway, I will send v2 with other lines fixed. Thanks, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- Changes in v2: - Lines are limited to 80 characters where possible - Confidence changed from High to Medium because of fs/btrfs/send.c:1119 false-positive - __vmalloc_area_node() explicitly excluded from analysis instead of !(file in "mm/vmalloc.c") condition scripts/coccinelle/api/kvfree.cocci | 227 ++++++++++++++++++++++++++++ 1 file changed, 227 insertions(+) create mode 100644 scripts/coccinelle/api/kvfree.cocci diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci new file mode 100644 index 000000000000..9455f9866ad8 --- /dev/null +++ b/scripts/coccinelle/api/kvfree.cocci @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: Medium +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + +@initialize:python@ +@@ +# low-level memory api +filter = frozenset(['__vmalloc_area_node']) + +def relevant(p): + return not (filter & {el.current_element for el in p}) + +@choice@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\| + kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\| + kmalloc_array_node@kok\|kcalloc_node@kok\)(...) + ... + } else { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| + __vmalloc_node@vok\)(...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...) + ... when != E = E1 + when any + if (\(!E\|E == NULL\)) { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| + __vmalloc_node@vok\)(...) + ... + } +) + +@opportunity depends on !patch@ +expression E, E1, size; +position p : script:python() { relevant(p) }; +@@ + +( +* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... when != E = E1 + when != size = E1 + when any +* if (\(!E\|E == NULL\))@p { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +) + +@vfree depends on !patch@ +expression E; +position k != choice.kok; +position p; +@@ + +* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| + kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@p + +@pvfree depends on patch exists@ +expression E; +position k != choice.kok; +@@ + + E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| + kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E) ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position v != choice.vok; +position p; +@@ + +* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|kvfree\)(E) + +@pkfree depends on patch exists@ +expression E; +position v != choice.vok; +@@ + + E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E) ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position p, k; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...)@k + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p + +@pkvfree depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@script: python depends on report@ +k << vfree.k; +p << vfree.p; +@@ + +coccilib.report.print_report(p[0], + f"WARNING: kmalloc is used to allocate this memory at line {k[0].line}") + +@script: python depends on org@ +k << vfree.k; +p << vfree.p; +@@ + +coccilib.org.print_todo(p[0], + f"WARNING: kmalloc is used to allocate this memory at line {k[0].line}") + +@script: python depends on report@ +v << kfree.v; +p << kfree.p; +@@ + +coccilib.report.print_report(p[0], + f"WARNING: vmalloc is used to allocate this memory at line {v[0].line}") + +@script: python depends on org@ +v << kfree.v; +p << kfree.p; +@@ + +coccilib.org.print_todo(p[0], + f"WARNING: vmalloc is used to allocate this memory at line {v[0].line}") + +@script: python depends on report@ +k << kvfree.k; +p << kvfree.p; +@@ + +coccilib.report.print_report(p[0], + f"WARNING: kvmalloc is used to allocate this memory at line {k[0].line}") + +@script: python depends on org@ +k << kvfree.k; +p << kvfree.p; +@@ + +coccilib.org.print_todo(p[0], + f"WARNING: kvmalloc is used to allocate this memory at line {k[0].line}") + +@script: python depends on report@ +p << opportunity.p; +@@ + +coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc") + +@script: python depends on org@ +p << opportunity.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc") -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Ping? _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- Changes in v2: - Lines are limited to 80 characters where possible - Confidence changed from High to Medium because of fs/btrfs/send.c:1119 false-positive - __vmalloc_area_node() explicitly excluded from analysis instead of !(file in "mm/vmalloc.c") condition Changes in v3: - prints style in org && report modes changed for python2 scripts/coccinelle/api/kvfree.cocci | 227 ++++++++++++++++++++++++++++ 1 file changed, 227 insertions(+) create mode 100644 scripts/coccinelle/api/kvfree.cocci diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci new file mode 100644 index 000000000000..7c396daeacad --- /dev/null +++ b/scripts/coccinelle/api/kvfree.cocci @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: Medium +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + +@initialize:python@ +@@ +# low-level memory api +filter = frozenset(['__vmalloc_area_node']) + +def relevant(p): + return not (filter & {el.current_element for el in p}) + +@choice@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\| + kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\| + kmalloc_array_node@kok\|kcalloc_node@kok\)(...) + ... + } else { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| + __vmalloc_node@vok\)(...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...) + ... when != E = E1 + when any + if (\(!E\|E == NULL\)) { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| + __vmalloc_node@vok\)(...) + ... + } +) + +@opportunity depends on !patch@ +expression E, E1, size; +position p : script:python() { relevant(p) }; +@@ + +( +* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... when != E = E1 + when != size = E1 + when any +* if (\(!E\|E == NULL\))@p { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +) + +@vfree depends on !patch@ +expression E; +position k != choice.kok; +position p; +@@ + +* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| + kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@p + +@pvfree depends on patch exists@ +expression E; +position k != choice.kok; +@@ + + E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| + kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E) ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position v != choice.vok; +position p; +@@ + +* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|kvfree\)(E) + +@pkfree depends on patch exists@ +expression E; +position v != choice.vok; +@@ + + E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E) ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position p, k; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...)@k + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p + +@pkvfree depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@script: python depends on report@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.org.print_todo(p[0], + +@script: python depends on report@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +p << opportunity.p; +@@ + +coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc") + +@script: python depends on org@ +p << opportunity.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc") -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
> + > +@script: python depends on org@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) > +coccilib.org.print_todo(p[0], Just noticed this error. I will resend the patch in 5mins. Regards, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- Changes in v2: - Lines are limited to 80 characters where possible - Confidence changed from High to Medium because of fs/btrfs/send.c:1119 false-positive - __vmalloc_area_node() explicitly excluded from analysis instead of !(file in "mm/vmalloc.c") condition Changes in v3: - prints style in org && report modes changed for python2 Changes in v4: - missing msg argument to print_todo fixed scripts/coccinelle/api/kvfree.cocci | 227 ++++++++++++++++++++++++++++ 1 file changed, 227 insertions(+) create mode 100644 scripts/coccinelle/api/kvfree.cocci diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci new file mode 100644 index 000000000000..d73578c5549c --- /dev/null +++ b/scripts/coccinelle/api/kvfree.cocci @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: Medium +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + +@initialize:python@ +@@ +# low-level memory api +filter = frozenset(['__vmalloc_area_node']) + +def relevant(p): + return not (filter & {el.current_element for el in p}) + +@choice@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\| + kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\| + kmalloc_array_node@kok\|kcalloc_node@kok\)(...) + ... + } else { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| + __vmalloc_node@vok\)(...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...) + ... when != E = E1 + when any + if (\(!E\|E == NULL\)) { + ... + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| + __vmalloc_node@vok\)(...) + ... + } +) + +@opportunity depends on !patch@ +expression E, E1, size; +position p : script:python() { relevant(p) }; +@@ + +( +* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) + ... when != E = E1 + when != size = E1 + when any +* if (\(!E\|E == NULL\))@p { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) + ... + } +) + +@vfree depends on !patch@ +expression E; +position k != choice.kok; +position p; +@@ + +* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| + kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@p + +@pvfree depends on patch exists@ +expression E; +position k != choice.kok; +@@ + + E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| + kcalloc_node@k\)(...) + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E) ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position v != choice.vok; +position p; +@@ + +* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|kvfree\)(E) + +@pkfree depends on patch exists@ +expression E; +position v != choice.vok; +@@ + + E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) + ... when != !is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E) ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position p, k; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...)@k + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p + +@pkvfree depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@script: python depends on report@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +p << opportunity.p; +@@ + +coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc") + +@script: python depends on org@ +p << opportunity.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc") -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Actually, the message looks fine. Sorry for the noise about that. However there is a problem in the kfree case: > +@kfree depends on !patch@ > +expression E; > +position v != choice.vok; > +position p; > +@@ > + > +* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| > + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| > + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) > + ... when != !is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|kvfree\)(E) p is not used. The last line should be (E)@p. julia _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Thu, 30 Jul 2020, Denis Efremov wrote: > Check that alloc and free types of functions match each other. Do the checks for the opportunities for kvmalloc really belong in this rule? That issue is not mentioned in the commit log or the description of the semantic patch. With the current patch mode, I got some changes in a recent linux-next. Have you sent patches for these issues? julia > > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > Changes in v2: > - Lines are limited to 80 characters where possible > - Confidence changed from High to Medium because of > fs/btrfs/send.c:1119 false-positive > - __vmalloc_area_node() explicitly excluded from analysis > instead of !(file in "mm/vmalloc.c") condition > Changes in v3: > - prints style in org && report modes changed for python2 > Changes in v4: > - missing msg argument to print_todo fixed > > scripts/coccinelle/api/kvfree.cocci | 227 ++++++++++++++++++++++++++++ > 1 file changed, 227 insertions(+) > create mode 100644 scripts/coccinelle/api/kvfree.cocci > > diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci > new file mode 100644 > index 000000000000..d73578c5549c > --- /dev/null > +++ b/scripts/coccinelle/api/kvfree.cocci > @@ -0,0 +1,227 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/// > +/// Check that kvmalloc'ed memory is freed by kfree functions, > +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree > +/// functions. > +/// > +// Confidence: Medium > +// Copyright: (C) 2020 Denis Efremov ISPRAS > +// Options: --no-includes --include-headers > +// > + > +virtual patch > +virtual report > +virtual org > +virtual context > + > +@initialize:python@ > +@@ > +# low-level memory api > +filter = frozenset(['__vmalloc_area_node']) > + > +def relevant(p): > + return not (filter & {el.current_element for el in p}) > + > +@choice@ > +expression E, E1; > +position kok, vok; > +@@ > + > +( > + if (...) { > + ... > + E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\| > + kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\| > + kmalloc_array_node@kok\|kcalloc_node@kok\)(...) > + ... > + } else { > + ... > + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| > + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| > + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| > + __vmalloc_node@vok\)(...) > + ... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...) > + ... when != E = E1 > + when any > + if (\(!E\|E == NULL\)) { > + ... > + E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\| > + vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\| > + vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\| > + __vmalloc_node@vok\)(...) > + ... > + } > +) > + > +@opportunity depends on !patch@ > +expression E, E1, size; > +position p : script:python() { relevant(p) }; > +@@ > + > +( > +* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p { > + ... > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) > + ... > + } else { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) > + ... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...) > + ... when != E = E1 > + when != size = E1 > + when any > +* if (\(!E\|E == NULL\))@p { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > + __vmalloc_node_range\|__vmalloc_node\)(..., size, ...) > + ... > + } > +) > + > +@vfree depends on !patch@ > +expression E; > +position k != choice.kok; > +position p; > +@@ > + > +* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| > + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| > + kcalloc_node@k\)(...) > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +* \(vfree\|vfree_atomic\|kvfree\)(E)@p > + > +@pvfree depends on patch exists@ > +expression E; > +position k != choice.kok; > +@@ > + > + E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\| > + kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\| > + kcalloc_node@k\)(...) > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +- \(vfree\|vfree_atomic\|kvfree\)(E) > ++ kfree(E) > + > +@kfree depends on !patch@ > +expression E; > +position v != choice.vok; > +position p; > +@@ > + > +* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| > + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| > + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) > + ... when != !is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|kvfree\)(E) > + > +@pkfree depends on patch exists@ > +expression E; > +position v != choice.vok; > +@@ > + > + E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\| > + vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\| > + __vmalloc_node_range@v\|__vmalloc_node@v\)(...) > + ... when != !is_vmalloc_addr(E) > + when any > +- \(kfree\|kvfree\)(E) > ++ vfree(E) > + > +@kvfree depends on !patch@ > +expression E; > +position p, k; > +@@ > + > +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > + kvmalloc_array\)(...)@k > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p > + > +@pkvfree depends on patch exists@ > +expression E; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > + kvmalloc_array\)(...) > + ... when != is_vmalloc_addr(E) > + when any > +- \(kfree\|vfree\)(E) > ++ kvfree(E) > + > +@script: python depends on report@ > +k << vfree.k; > +p << vfree.p; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +k << vfree.k; > +p << vfree.p; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +k << kvfree.k; > +p << kvfree.p; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +k << kvfree.k; > +p << kvfree.p; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +p << opportunity.p; > +@@ > + > +coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc") > + > +@script: python depends on org@ > +p << opportunity.p; > +@@ > + > +coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc") > -- > 2.26.2 > > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
> With the current patch mode, I got some changes in a recent linux-next. > Have you sent patches for these issues? For mellanox, I've sent these patches: https://lkml.org/lkml/2020/6/5/901 https://lkml.org/lkml/2020/6/1/713 They were accepted. I see two new places in mellanox driver in linux-next. It looks like this is new code that is not yet merged to the linux master branch. diff -u -p a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c @@ -228,8 +228,8 @@ static int rx_fs_create(struct mlx5e_pri fs_prot->miss_rule = miss_rule; out: - kfree(flow_group_in); - kfree(spec); + kvfree(flow_group_in); + kvfree(spec); return err; } diff -u -p a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c @@ -191,7 +191,7 @@ static int accel_fs_tcp_create_groups(st ft->g = kcalloc(MLX5E_ACCEL_FS_TCP_NUM_GROUPS, sizeof(*ft->g), GFP_KERNEL); in = kvzalloc(inlen, GFP_KERNEL); if (!in || !ft->g) { - kvfree(ft->g); + kfree(ft->g); kvfree(in); return -ENOMEM; } I will send the fixes when the code will be merged to the linux master branch. Maybe it will be fixed already in net-next at that time. > > Do the checks for the opportunities for kvmalloc really belong in this > rule? That issue is not mentioned in the commit log or the description of > the semantic patch. I added this at the last moment. It was easy enough to add it based on existing patterns. I will add description for this warnings. Or do you want me to single out this warning to a separate rule? Regards, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
> > > > Do the checks for the opportunities for kvmalloc really belong in this > > rule? That issue is not mentioned in the commit log or the description of > > the semantic patch. > > I added this at the last moment. It was easy enough to add it based on existing > patterns. I will add description for this warnings. Or do you want me to single > out this warning to a separate rule? It seems like a different issue. A separate rule might be better. Also, there is no patch variant, so if one runs the patch mode on this script, where the patch mode is useful, then one will miss the kvmalloc suggestions completely. Coccicheck has a mode where is first tries patch and then report; I think 0-day uses this. julia _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- Changes in v2: - Lines are limited to 80 characters where possible - Confidence changed from High to Medium because of fs/btrfs/send.c:1119 false-positive - __vmalloc_area_node() explicitly excluded from analysis instead of !(file in "mm/vmalloc.c") condition Changes in v3: - prints style in org && report modes changed for python2 Changes in v4: - missing msg argument to print_todo fixed Changes in v5: - fix position p in kfree rule - move @kok and @v positions in choice rule after the arguments - remove kvmalloc suggestions scripts/coccinelle/api/kvfree.cocci | 182 ++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 scripts/coccinelle/api/kvfree.cocci diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci new file mode 100644 index 000000000000..e83b1ebbad7e --- /dev/null +++ b/scripts/coccinelle/api/kvfree.cocci @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: Medium +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + +@initialize:python@ +@@ +# low-level memory api +filter = frozenset(['__vmalloc_area_node']) + +def relevant(p): + return not (filter & {el.current_element for el in p}) + +@choice@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| + kmalloc_node\|kzalloc_node\|kmalloc_array\| + kmalloc_array_node\|kcalloc_node\)(...)@kok + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...) + ... when != E = E1 + when any + if (\(!E\|E == NULL\)) { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +) + +@vfree depends on !patch@ +expression E; +position k != choice.kok; +position p; +@@ + +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| + kcalloc_node\)(...)@k + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@p + +@pvfree depends on patch exists@ +expression E; +position k != choice.kok; +@@ + + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| + kcalloc_node\)(...)@k + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E) ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position v != choice.vok; +position p; +@@ + +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(...)@v + ... when != !is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|kvfree\)(E)@p + +@pkfree depends on patch exists@ +expression E; +position v != choice.vok; +@@ + + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(...)@v + ... when != !is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E) ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position p, k; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...)@k + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p + +@pkvfree depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@script: python depends on report@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- Changes in v2: - Lines are limited to 80 characters where possible - Confidence changed from High to Medium because of fs/btrfs/send.c:1119 false-positive - __vmalloc_area_node() explicitly excluded from analysis instead of !(file in "mm/vmalloc.c") condition Changes in v3: - prints style in org && report modes changed for python2 Changes in v4: - missing msg argument to print_todo fixed Changes in v5: - fix position p in kfree rule - move @kok and @v positions in choice rule after the arguments - remove kvmalloc suggestions Changes in v6: - more asterisks added in context mode - second @kok added to the choice rule scripts/coccinelle/api/kvfree.cocci | 182 ++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 scripts/coccinelle/api/kvfree.cocci diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci new file mode 100644 index 000000000000..f43cda5b0d5b --- /dev/null +++ b/scripts/coccinelle/api/kvfree.cocci @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: Medium +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + +@initialize:python@ +@@ +# low-level memory api +filter = frozenset(['__vmalloc_area_node']) + +def relevant(p): + return not (filter & {el.current_element for el in p}) + +@choice@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| + kmalloc_node\|kzalloc_node\|kmalloc_array\| + kmalloc_array_node\|kcalloc_node\)(...)@kok + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok + ... when != E = E1 + when any + if (\(!E\|E == NULL\)) { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +) + +@vfree depends on !patch@ +expression E; +position k != choice.kok; +position p; +@@ + +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\| +* kcalloc_node\)(...)@k + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@p + +@pvfree depends on patch exists@ +expression E; +position k != choice.kok; +@@ + + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| + kcalloc_node\)(...)@k + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E) ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position v != choice.vok; +position p; +@@ + +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| +* __vmalloc_node_range\|__vmalloc_node\)(...)@v + ... when != !is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|kvfree\)(E)@p + +@pkfree depends on patch exists@ +expression E; +position v != choice.vok; +@@ + + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(...)@v + ... when != !is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E) ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position p, k; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| +* kvmalloc_array\)(...)@k + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p + +@pkvfree depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@script: python depends on report@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << vfree.k; +p << vfree.p; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +v << kfree.v; +p << kfree.p; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) +coccilib.org.print_todo(p[0], msg) + +@script: python depends on report@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.report.print_report(p[0], msg) + +@script: python depends on org@ +k << kvfree.k; +p << kvfree.p; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) +coccilib.org.print_todo(p[0], msg) -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[-- Attachment #1: Type: text/plain, Size: 921 bytes --] On Sun, 2 Aug 2020, Markus Elfring wrote: > … > > +++ b/scripts/coccinelle/api/kvfree.cocci > … > > +@choice@ > > +expression E, E1; > > +position kok, vok; > > +@@ > > + > > +( > … > > +| > > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > > + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok > > + ... when != E = E1 > > + when any > > + if (\(!E\|E == NULL\)) { > … > > Can the SmPL code exclusion specification be incomplete here? > > How do you think about to check also if the memory is passed to any function > (or macro) calls before the shown detection of a null pointer? It seems both extremely unlikely and not relevant to the question at hand. Passing E to another function will not change the value of E itself. Passing &E to another function could change E, but it would be very unusual to do that, and doesn't seem worth checking for. julia [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
> +@initialize:python@ > +@@ > +# low-level memory api > +filter = frozenset(['__vmalloc_area_node']) > + > +def relevant(p): > + return not (filter & {el.current_element for el in p}) Is this used? Otherwise, I think it would be good to not warn about a use of kvfree if that use is reachable from a kvmalloc. There seems to be such a false positive in fs/btrfs/send.c, on line 1118. It also seems that when there are both a kmalloc and a vmalloc, there is no warning if kfree or vfree is used. Is that intentional? julia > + > +@choice@ > +expression E, E1; > +position kok, vok; > +@@ > + > +( > + if (...) { > + ... > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| > + kmalloc_node\|kzalloc_node\|kmalloc_array\| > + kmalloc_array_node\|kcalloc_node\)(...)@kok > + ... > + } else { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > + ... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok > + ... when != E = E1 > + when any > + if (\(!E\|E == NULL\)) { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > + ... > + } > +) > + > +@vfree depends on !patch@ > +expression E; > +position k != choice.kok; > +position p; > +@@ > + > +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > +* kcalloc_node\)(...)@k > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +* \(vfree\|vfree_atomic\|kvfree\)(E)@p > + > +@pvfree depends on patch exists@ > +expression E; > +position k != choice.kok; > +@@ > + > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > + kcalloc_node\)(...)@k > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +- \(vfree\|vfree_atomic\|kvfree\)(E) > ++ kfree(E) > + > +@kfree depends on !patch@ > +expression E; > +position v != choice.vok; > +position p; > +@@ > + > +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > +* __vmalloc_node_range\|__vmalloc_node\)(...)@v > + ... when != !is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|kvfree\)(E)@p > + > +@pkfree depends on patch exists@ > +expression E; > +position v != choice.vok; > +@@ > + > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > + __vmalloc_node_range\|__vmalloc_node\)(...)@v > + ... when != !is_vmalloc_addr(E) > + when any > +- \(kfree\|kvfree\)(E) > ++ vfree(E) > + > +@kvfree depends on !patch@ > +expression E; > +position p, k; > +@@ > + > +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > +* kvmalloc_array\)(...)@k > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p > + > +@pkvfree depends on patch exists@ > +expression E; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > + kvmalloc_array\)(...) > + ... when != is_vmalloc_addr(E) > + when any > +- \(kfree\|vfree\)(E) > ++ kvfree(E) > + > +@script: python depends on report@ > +k << vfree.k; > +p << vfree.p; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +k << vfree.k; > +p << vfree.p; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line) > +coccilib.org.print_todo(p[0], msg) > + > +@script: python depends on report@ > +k << kvfree.k; > +p << kvfree.p; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.report.print_report(p[0], msg) > + > +@script: python depends on org@ > +k << kvfree.k; > +p << kvfree.p; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line) > +coccilib.org.print_todo(p[0], msg) > -- > 2.26.2 > > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On 8/2/20 11:24 PM, Julia Lawall wrote: >> +@initialize:python@ >> +@@ >> +# low-level memory api >> +filter = frozenset(['__vmalloc_area_node']) >> + >> +def relevant(p): >> + return not (filter & {el.current_element for el in p}) > > Is this used? I'll remove it in v8. Or do you want me to add iterate_dir_item() in the list? > > Otherwise, I think it would be good to not warn about a use of kvfree > if that use is reachable from a kvmalloc. There seems to be such a false > positive in fs/btrfs/send.c, on line 1118. I don't know how to handle this case without position filter. It's too complex. In iterate_dir_item() there is: buf = kmalloc(buf_len, GFP_KERNEL); while(...) { if (...) { if (is_vmalloc_addr(buf)) { vfree(buf); ... } else { char *tmp = krealloc(buf, ...); if (!tmp) kfree(buf); ... } if (!buf) { buf = kvmalloc(buf_len, GFP_KERNEL); ... } } } kvfree(buf); Adding "when != kvfree(E)" is not enough: * E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| * kvmalloc_array\)(...)@k ... when != is_vmalloc_addr(E) + when != kvfree(E) when any * \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p > > It also seems that when there are both a kmalloc and a vmalloc, there is > no warning if kfree or vfree is used. Is that intentional? > No, I will try to address it in v8. Regards, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Is there a difference from cocci point of view between: ... when != !is_vmalloc_addr(E) and ... when != is_vmalloc_addr(E) Should the latter one be used in most cases? Thanks, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Mon, 3 Aug 2020, Denis Efremov wrote: > Is there a difference from cocci point of view between: > > ... when != !is_vmalloc_addr(E) This will only reject cases where the ! is present. Coccinelle doesn't apply isomorphisms to the C source code, so it doesn't detect that eg if (A) B else C could be rewritten as if (!A) C ese B So when != !A would only match when the code is in the second form. > > and > > ... when != is_vmalloc_addr(E) > > Should the latter one be used in most cases? This matches both a call to is_vmalloc_addr and a negated call, so it is more general. julia _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Mon, 3 Aug 2020, Denis Efremov wrote: > > > On 8/2/20 11:24 PM, Julia Lawall wrote: > >> +@initialize:python@ > >> +@@ > >> +# low-level memory api > >> +filter = frozenset(['__vmalloc_area_node']) > >> + > >> +def relevant(p): > >> + return not (filter & {el.current_element for el in p}) > > > > Is this used? > > I'll remove it in v8. Or do you want me to add iterate_dir_item() in the list? What is that? > > > > > Otherwise, I think it would be good to not warn about a use of kvfree > > if that use is reachable from a kvmalloc. There seems to be such a false > > positive in fs/btrfs/send.c, on line 1118. > > I don't know how to handle this case without position filter. > It's too complex. In iterate_dir_item() there is: > buf = kmalloc(buf_len, GFP_KERNEL); > while(...) { > if (...) { > if (is_vmalloc_addr(buf)) { > vfree(buf); > ... > } else { > char *tmp = krealloc(buf, ...); > > if (!tmp) > kfree(buf); > ... > } > if (!buf) { > buf = kvmalloc(buf_len, GFP_KERNEL); > ... > } > } > } > kvfree(buf); > > Adding "when != kvfree(E)" is not enough: > * E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > * kvmalloc_array\)(...)@k > ... when != is_vmalloc_addr(E) > + when != kvfree(E) > when any > * \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p Why not just @ok exists@ position p; expression E; @@ E = kvalloc(...) ... kvfree@p(...) Probably that is what you mean by a position filter, but why not add a position filter? julia > > > > It also seems that when there are both a kmalloc and a vmalloc, there is > > no warning if kfree or vfree is used. Is that intentional? > > > > No, I will try to address it in v8. > > Regards, > Denis > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- Changes in v2: - Lines are limited to 80 characters where possible - Confidence changed from High to Medium because of fs/btrfs/send.c:1119 false-positive - __vmalloc_area_node() explicitly excluded from analysis instead of !(file in "mm/vmalloc.c") condition Changes in v3: - prints style in org && report modes changed for python2 Changes in v4: - missing msg argument to print_todo fixed Changes in v5: - fix position p in kfree rule - move @kok and @v positions in choice rule after the arguments - remove kvmalloc suggestions Changes in v6: - more asterisks added in context mode - second @kok added to the choice rule Changes in v7: - file renamed to kfree_mismatch.cocci - python function relevant() removed - additional rule for filtering free positions added - btrfs false-positive fixed - confidence level changed to high - kvfree_switch rule added - names for position variables changed to @a (alloc) and @f (free) scripts/coccinelle/api/kfree_mismatch.cocci | 229 ++++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci b/scripts/coccinelle/api/kfree_mismatch.cocci new file mode 100644 index 000000000000..9e9ef9fd7a25 --- /dev/null +++ b/scripts/coccinelle/api/kfree_mismatch.cocci @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + +@alloc@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| + kmalloc_node\|kzalloc_node\|kmalloc_array\| + kmalloc_array_node\|kcalloc_node\)(...)@kok + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok + ... when != E = E1 + when any + if (E == NULL) { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +) + +@free@ +expression E; +position fok; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... + kvfree(E)@fok + +@vfree depends on !patch@ +expression E; +position a != alloc.kok; +position f != free.fok; +@@ + +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\| +* kcalloc_node\)(...)@a + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@f + +@depends on patch exists@ +expression E; +position a != alloc.kok; +position f != free.fok; +@@ + + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| + kcalloc_node\)(...)@a + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E)@f ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position a != alloc.vok; +position f != free.fok; +@@ + +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| +* __vmalloc_node_range\|__vmalloc_node\)(...)@a + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|kvfree\)(E)@f + +@depends on patch exists@ +expression E; +position a != alloc.vok; +position f != free.fok; +@@ + + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(...)@a + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E)@f ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position a, f; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| +* kvmalloc_array\)(...)@a + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@f + +@depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@kvfree_switch depends on !patch@ +expression alloc.E; +position f != free.fok; +@@ + + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@f + +@depends on patch exists@ +expression alloc.E; +position f != free.fok; +@@ + + ... when != is_vmalloc_addr(E) + when any +( +- \(kfree\|vfree\)(E)@f ++ kvfree(E) +| +- kzfree(E)@f ++ kvfree_sensitive(E) +) + +@script: python depends on report@ +a << vfree.a; +f << vfree.f; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +a << vfree.a; +f << vfree.f; +@@ + +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.org.print_todo(f[0], msg) + +@script: python depends on report@ +a << kfree.a; +f << kfree.f; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +a << kfree.a; +f << kfree.f; +@@ + +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.org.print_todo(f[0], msg) + +@script: python depends on report@ +a << kvfree.a; +f << kvfree.f; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +a << kvfree.a; +f << kvfree.f; +@@ + +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.org.print_todo(f[0], msg) + +@script: python depends on report@ +ka << alloc.kok; +va << alloc.vok; +f << kvfree_switch.f; +@@ + +msg = "WARNING: kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +ka << alloc.kok; +va << alloc.vok; +f << kvfree_switch.f; +@@ + +msg = "WARNING: kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) +coccilib.org.print_todo(f[0], msg) + -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Hi, On 8/3/20 9:34 PM, Denis Efremov wrote: > Check that alloc and free types of functions match each other. Julia, I've just send the patches to fix all the warnings emitted by the script. [1] https://lore.kernel.org/patchwork/patch/1309731/ [2] https://lore.kernel.org/patchwork/patch/1309273/ [3] https://lore.kernel.org/patchwork/patch/1309275/ Other inconsistencies and bugs detected by this script: 1e814d630fd1 drm/amd/display: Use kfree() to free rgb_user in calculate_user_regamma_ramp() 842540075974 drm/amd/display: Use kvfree() to free coeff in build_regamma() f5e383ac8b58 iommu/pamu: Use kzfree() in fsl_pamu_probe() 360000b26e37 net/mlx5: Use kfree(ft->g) in arfs_create_groups() 114427b8927a drm/panfrost: Use kvfree() to free bo->sgts 742532d11d83 f2fs: use kfree() instead of kvfree() to free superblock data 47a357de2b6b net/mlx5: DR, Fix freeing in dr_create_rc_qp() a8c73c1a614f io_uring: use kvfree() in io_sqe_buffer_register() 7f89cc07d22a cxgb4: Use kfree() instead kvfree() where appropriate bb2359f4dbe9 bpf: Change kvfree to kfree in generic_map_lookup_batch() > Changes in v2: > - Lines are limited to 80 characters where possible > - Confidence changed from High to Medium because of > fs/btrfs/send.c:1119 false-positive > - __vmalloc_area_node() explicitly excluded from analysis > instead of !(file in "mm/vmalloc.c") condition > Changes in v3: > - prints style in org && report modes changed for python2 > Changes in v4: > - missing msg argument to print_todo fixed > Changes in v5: > - fix position p in kfree rule > - move @kok and @v positions in choice rule after the arguments > - remove kvmalloc suggestions > Changes in v6: > - more asterisks added in context mode > - second @kok added to the choice rule > Changes in v7: > - file renamed to kfree_mismatch.cocci > - python function relevant() removed > - additional rule for filtering free positions added > - btrfs false-positive fixed > - confidence level changed to high > - kvfree_switch rule added > - names for position variables changed to @a (alloc) and @f (free) Is there something I can improve in this cocci script to be accepted? Thanks, Denis _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[See below for a comment] On Mon, 3 Aug 2020, Denis Efremov wrote: > Check that alloc and free types of functions match each other. > > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > Changes in v2: > - Lines are limited to 80 characters where possible > - Confidence changed from High to Medium because of > fs/btrfs/send.c:1119 false-positive > - __vmalloc_area_node() explicitly excluded from analysis > instead of !(file in "mm/vmalloc.c") condition > Changes in v3: > - prints style in org && report modes changed for python2 > Changes in v4: > - missing msg argument to print_todo fixed > Changes in v5: > - fix position p in kfree rule > - move @kok and @v positions in choice rule after the arguments > - remove kvmalloc suggestions > Changes in v6: > - more asterisks added in context mode > - second @kok added to the choice rule > Changes in v7: > - file renamed to kfree_mismatch.cocci > - python function relevant() removed > - additional rule for filtering free positions added > - btrfs false-positive fixed > - confidence level changed to high > - kvfree_switch rule added > - names for position variables changed to @a (alloc) and @f (free) > > scripts/coccinelle/api/kfree_mismatch.cocci | 229 ++++++++++++++++++++ > 1 file changed, 229 insertions(+) > create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci > > diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci b/scripts/coccinelle/api/kfree_mismatch.cocci > new file mode 100644 > index 000000000000..9e9ef9fd7a25 > --- /dev/null > +++ b/scripts/coccinelle/api/kfree_mismatch.cocci > @@ -0,0 +1,229 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/// > +/// Check that kvmalloc'ed memory is freed by kfree functions, > +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree > +/// functions. > +/// > +// Confidence: High > +// Copyright: (C) 2020 Denis Efremov ISPRAS > +// Options: --no-includes --include-headers > +// > + > +virtual patch > +virtual report > +virtual org > +virtual context > + > +@alloc@ > +expression E, E1; > +position kok, vok; > +@@ > + > +( > + if (...) { > + ... > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| > + kmalloc_node\|kzalloc_node\|kmalloc_array\| > + kmalloc_array_node\|kcalloc_node\)(...)@kok > + ... > + } else { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > + ... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok > + ... when != E = E1 > + when any > + if (E == NULL) { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > + ... > + } > +) > + > +@free@ > +expression E; > +position fok; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > + kvmalloc_array\)(...) > + ... > + kvfree(E)@fok > + > +@vfree depends on !patch@ > +expression E; > +position a != alloc.kok; > +position f != free.fok; > +@@ > + > +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > +* kcalloc_node\)(...)@a > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +* \(vfree\|vfree_atomic\|kvfree\)(E)@f > + > +@depends on patch exists@ > +expression E; > +position a != alloc.kok; > +position f != free.fok; > +@@ > + > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > + kcalloc_node\)(...)@a > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +- \(vfree\|vfree_atomic\|kvfree\)(E)@f > ++ kfree(E) > + > +@kfree depends on !patch@ > +expression E; > +position a != alloc.vok; > +position f != free.fok; > +@@ > + > +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > +* __vmalloc_node_range\|__vmalloc_node\)(...)@a > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|kvfree\)(E)@f > + > +@depends on patch exists@ > +expression E; > +position a != alloc.vok; > +position f != free.fok; > +@@ > + > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > + __vmalloc_node_range\|__vmalloc_node\)(...)@a > + ... when != is_vmalloc_addr(E) > + when any > +- \(kfree\|kvfree\)(E)@f > ++ vfree(E) > + > +@kvfree depends on !patch@ > +expression E; > +position a, f; > +@@ > + > +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > +* kvmalloc_array\)(...)@a > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@f > + > +@depends on patch exists@ > +expression E; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > + kvmalloc_array\)(...) > + ... when != is_vmalloc_addr(E) > + when any > +- \(kfree\|vfree\)(E) > ++ kvfree(E) > + > +@kvfree_switch depends on !patch@ > +expression alloc.E; > +position f != free.fok; > +@@ > + > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@f > + > +@depends on patch exists@ > +expression alloc.E; > +position f != free.fok; > +@@ > + > + ... when != is_vmalloc_addr(E) > + when any > +( > +- \(kfree\|vfree\)(E)@f > ++ kvfree(E) > +| > +- kzfree(E)@f > ++ kvfree_sensitive(E) > +) The above two rules refer to free.fok, but it is not clear why, because free.fok only occurs on a kvfree call. Is there a mistake, or is it just an unnecessary copy-pasted constraint? julia > + > +@script: python depends on report@ > +a << vfree.a; > +f << vfree.f; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +a << vfree.a; > +f << vfree.f; > +@@ > + > +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.org.print_todo(f[0], msg) > + > +@script: python depends on report@ > +a << kfree.a; > +f << kfree.f; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +a << kfree.a; > +f << kfree.f; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.org.print_todo(f[0], msg) > + > +@script: python depends on report@ > +a << kvfree.a; > +f << kvfree.f; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +a << kvfree.a; > +f << kvfree.f; > +@@ > + > +msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.org.print_todo(f[0], msg) > + > +@script: python depends on report@ > +ka << alloc.kok; > +va << alloc.vok; > +f << kvfree_switch.f; > +@@ > + > +msg = "WARNING: kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +ka << alloc.kok; > +va << alloc.vok; > +f << kvfree_switch.f; > +@@ > + > +msg = "WARNING: kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) > +coccilib.org.print_todo(f[0], msg) > + > -- > 2.26.2 > > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Check that alloc and free types of functions match each other. Signed-off-by: Denis Efremov <efremov@linux.com> --- Changes in v2: - Lines are limited to 80 characters where possible - Confidence changed from High to Medium because of fs/btrfs/send.c:1119 false-positive - __vmalloc_area_node() explicitly excluded from analysis instead of !(file in "mm/vmalloc.c") condition Changes in v3: - prints style in org && report modes changed for python2 Changes in v4: - missing msg argument to print_todo fixed Changes in v5: - fix position p in kfree rule - move @kok and @v positions in choice rule after the arguments - remove kvmalloc suggestions Changes in v6: - more asterisks added in context mode - second @kok added to the choice rule Changes in v7: - file renamed to kfree_mismatch.cocci - python function relevant() removed - additional rule for filtering free positions added - btrfs false-positive fixed - confidence level changed to high - kvfree_switch rule added - names for position variables changed to @a (alloc) and @f (free) Changes in v8: - kzfree() replaced with kfree_sensitive() - "position f != free.fok;" simplified to "position f;" in patch and kvfree_switch rules scripts/coccinelle/api/kfree_mismatch.cocci | 229 ++++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci b/scripts/coccinelle/api/kfree_mismatch.cocci new file mode 100644 index 000000000000..843b794fac7b --- /dev/null +++ b/scripts/coccinelle/api/kfree_mismatch.cocci @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check that kvmalloc'ed memory is freed by kfree functions, +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree +/// functions. +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// + +virtual patch +virtual report +virtual org +virtual context + +@alloc@ +expression E, E1; +position kok, vok; +@@ + +( + if (...) { + ... + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| + kmalloc_node\|kzalloc_node\|kmalloc_array\| + kmalloc_array_node\|kcalloc_node\)(...)@kok + ... + } else { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +| + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok + ... when != E = E1 + when any + if (E == NULL) { + ... + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| + vzalloc_node\|vmalloc_exec\|vmalloc_32\| + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| + __vmalloc_node\)(...)@vok + ... + } +) + +@free@ +expression E; +position fok; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... + kvfree(E)@fok + +@vfree depends on !patch@ +expression E; +position a != alloc.kok; +position f != free.fok; +@@ + +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\| +* kcalloc_node\)(...)@a + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +* \(vfree\|vfree_atomic\|kvfree\)(E)@f + +@depends on patch exists@ +expression E; +position a != alloc.kok; +position f != free.fok; +@@ + + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| + kcalloc_node\)(...)@a + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } + when != is_vmalloc_addr(E) + when any +- \(vfree\|vfree_atomic\|kvfree\)(E)@f ++ kfree(E) + +@kfree depends on !patch@ +expression E; +position a != alloc.vok; +position f != free.fok; +@@ + +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| +* __vmalloc_node_range\|__vmalloc_node\)(...)@a + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kfree_sensitive\|kvfree\)(E)@f + +@depends on patch exists@ +expression E; +position a != alloc.vok; +position f != free.fok; +@@ + + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| + __vmalloc_node_range\|__vmalloc_node\)(...)@a + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|kvfree\)(E)@f ++ vfree(E) + +@kvfree depends on !patch@ +expression E; +position a, f; +@@ + +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| +* kvmalloc_array\)(...)@a + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kfree_sensitive\|vfree\|vfree_atomic\)(E)@f + +@depends on patch exists@ +expression E; +@@ + + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| + kvmalloc_array\)(...) + ... when != is_vmalloc_addr(E) + when any +- \(kfree\|vfree\)(E) ++ kvfree(E) + +@kvfree_switch depends on !patch@ +expression alloc.E; +position f; +@@ + + ... when != is_vmalloc_addr(E) + when any +* \(kfree\|kfree_sensitive\|vfree\|vfree_atomic\)(E)@f + +@depends on patch exists@ +expression alloc.E; +position f; +@@ + + ... when != is_vmalloc_addr(E) + when any +( +- \(kfree\|vfree\)(E)@f ++ kvfree(E) +| +- kfree_sensitive(E)@f ++ kvfree_sensitive(E) +) + +@script: python depends on report@ +a << vfree.a; +f << vfree.f; +@@ + +msg = "WARNING kmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +a << vfree.a; +f << vfree.f; +@@ + +msg = "WARNING kmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.org.print_todo(f[0], msg) + +@script: python depends on report@ +a << kfree.a; +f << kfree.f; +@@ + +msg = "WARNING vmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +a << kfree.a; +f << kfree.f; +@@ + +msg = "WARNING vmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.org.print_todo(f[0], msg) + +@script: python depends on report@ +a << kvfree.a; +f << kvfree.f; +@@ + +msg = "WARNING kvmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +a << kvfree.a; +f << kvfree.f; +@@ + +msg = "WARNING kvmalloc is used to allocate this memory at line %s" % (a[0].line) +coccilib.org.print_todo(f[0], msg) + +@script: python depends on report@ +ka << alloc.kok; +va << alloc.vok; +f << kvfree_switch.f; +@@ + +msg = "WARNING kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) +coccilib.report.print_report(f[0], msg) + +@script: python depends on org@ +ka << alloc.kok; +va << alloc.vok; +f << kvfree_switch.f; +@@ + +msg = "WARNING kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) +coccilib.org.print_todo(f[0], msg) + -- 2.26.2 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Fri, 16 Oct 2020, Denis Efremov wrote: > Check that alloc and free types of functions match each other. > > Signed-off-by: Denis Efremov <efremov@linux.com> Applied, thanks. > --- > Changes in v2: > - Lines are limited to 80 characters where possible > - Confidence changed from High to Medium because of > fs/btrfs/send.c:1119 false-positive > - __vmalloc_area_node() explicitly excluded from analysis > instead of !(file in "mm/vmalloc.c") condition > Changes in v3: > - prints style in org && report modes changed for python2 > Changes in v4: > - missing msg argument to print_todo fixed > Changes in v5: > - fix position p in kfree rule > - move @kok and @v positions in choice rule after the arguments > - remove kvmalloc suggestions > Changes in v6: > - more asterisks added in context mode > - second @kok added to the choice rule > Changes in v7: > - file renamed to kfree_mismatch.cocci > - python function relevant() removed > - additional rule for filtering free positions added > - btrfs false-positive fixed > - confidence level changed to high > - kvfree_switch rule added > - names for position variables changed to @a (alloc) and @f (free) > Changes in v8: > - kzfree() replaced with kfree_sensitive() > - "position f != free.fok;" simplified to "position f;" in patch > and kvfree_switch rules > > scripts/coccinelle/api/kfree_mismatch.cocci | 229 ++++++++++++++++++++ > 1 file changed, 229 insertions(+) > create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci > > diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci b/scripts/coccinelle/api/kfree_mismatch.cocci > new file mode 100644 > index 000000000000..843b794fac7b > --- /dev/null > +++ b/scripts/coccinelle/api/kfree_mismatch.cocci > @@ -0,0 +1,229 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/// > +/// Check that kvmalloc'ed memory is freed by kfree functions, > +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree > +/// functions. > +/// > +// Confidence: High > +// Copyright: (C) 2020 Denis Efremov ISPRAS > +// Options: --no-includes --include-headers > +// > + > +virtual patch > +virtual report > +virtual org > +virtual context > + > +@alloc@ > +expression E, E1; > +position kok, vok; > +@@ > + > +( > + if (...) { > + ... > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| > + kmalloc_node\|kzalloc_node\|kmalloc_array\| > + kmalloc_array_node\|kcalloc_node\)(...)@kok > + ... > + } else { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > + ... > + } > +| > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\| > + kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok > + ... when != E = E1 > + when any > + if (E == NULL) { > + ... > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\| > + vzalloc_node\|vmalloc_exec\|vmalloc_32\| > + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\| > + __vmalloc_node\)(...)@vok > + ... > + } > +) > + > +@free@ > +expression E; > +position fok; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > + kvmalloc_array\)(...) > + ... > + kvfree(E)@fok > + > +@vfree depends on !patch@ > +expression E; > +position a != alloc.kok; > +position f != free.fok; > +@@ > + > +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > +* kcalloc_node\)(...)@a > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +* \(vfree\|vfree_atomic\|kvfree\)(E)@f > + > +@depends on patch exists@ > +expression E; > +position a != alloc.kok; > +position f != free.fok; > +@@ > + > + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\| > + kzalloc_node\|kmalloc_array\|kmalloc_array_node\| > + kcalloc_node\)(...)@a > + ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...); ... } > + when != is_vmalloc_addr(E) > + when any > +- \(vfree\|vfree_atomic\|kvfree\)(E)@f > ++ kfree(E) > + > +@kfree depends on !patch@ > +expression E; > +position a != alloc.vok; > +position f != free.fok; > +@@ > + > +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > +* __vmalloc_node_range\|__vmalloc_node\)(...)@a > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kfree_sensitive\|kvfree\)(E)@f > + > +@depends on patch exists@ > +expression E; > +position a != alloc.vok; > +position f != free.fok; > +@@ > + > + E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\| > + vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\| > + __vmalloc_node_range\|__vmalloc_node\)(...)@a > + ... when != is_vmalloc_addr(E) > + when any > +- \(kfree\|kvfree\)(E)@f > ++ vfree(E) > + > +@kvfree depends on !patch@ > +expression E; > +position a, f; > +@@ > + > +* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > +* kvmalloc_array\)(...)@a > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kfree_sensitive\|vfree\|vfree_atomic\)(E)@f > + > +@depends on patch exists@ > +expression E; > +@@ > + > + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\| > + kvmalloc_array\)(...) > + ... when != is_vmalloc_addr(E) > + when any > +- \(kfree\|vfree\)(E) > ++ kvfree(E) > + > +@kvfree_switch depends on !patch@ > +expression alloc.E; > +position f; > +@@ > + > + ... when != is_vmalloc_addr(E) > + when any > +* \(kfree\|kfree_sensitive\|vfree\|vfree_atomic\)(E)@f > + > +@depends on patch exists@ > +expression alloc.E; > +position f; > +@@ > + > + ... when != is_vmalloc_addr(E) > + when any > +( > +- \(kfree\|vfree\)(E)@f > ++ kvfree(E) > +| > +- kfree_sensitive(E)@f > ++ kvfree_sensitive(E) > +) > + > +@script: python depends on report@ > +a << vfree.a; > +f << vfree.f; > +@@ > + > +msg = "WARNING kmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +a << vfree.a; > +f << vfree.f; > +@@ > + > +msg = "WARNING kmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.org.print_todo(f[0], msg) > + > +@script: python depends on report@ > +a << kfree.a; > +f << kfree.f; > +@@ > + > +msg = "WARNING vmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +a << kfree.a; > +f << kfree.f; > +@@ > + > +msg = "WARNING vmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.org.print_todo(f[0], msg) > + > +@script: python depends on report@ > +a << kvfree.a; > +f << kvfree.f; > +@@ > + > +msg = "WARNING kvmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +a << kvfree.a; > +f << kvfree.f; > +@@ > + > +msg = "WARNING kvmalloc is used to allocate this memory at line %s" % (a[0].line) > +coccilib.org.print_todo(f[0], msg) > + > +@script: python depends on report@ > +ka << alloc.kok; > +va << alloc.vok; > +f << kvfree_switch.f; > +@@ > + > +msg = "WARNING kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) > +coccilib.report.print_report(f[0], msg) > + > +@script: python depends on org@ > +ka << alloc.kok; > +va << alloc.vok; > +f << kvfree_switch.f; > +@@ > + > +msg = "WARNING kmalloc (line %s) && vmalloc (line %s) are used to allocate this memory" % (ka[0].line, va[0].line) > +coccilib.org.print_todo(f[0], msg) > + > -- > 2.26.2 > > _______________________________________________ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci