Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 4 Jun 2020 22:57:18 +0200 (CEST)
From:   Julia Lawall <julia.lawall@inria.fr>
To:     Denis Efremov <efremov@linux.com>
cc:     Joe Perches <joe@perches.com>, cocci@systeme.lip6.fr,
        linux-kernel@vger.kernel.org
Subject: Re: [Cocci] [PATCH v2] coccinelle: api: add kzfree script
In-Reply-To: <20200604204846.15897-1-efremov@linux.com>
Message-ID: <alpine.DEB.2.21.2006042254240.2577@hadrien>
References: <20200604140805.111613-1-efremov@linux.com> <20200604204846.15897-1-efremov@linux.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On Thu, 4 Jun 2020, Denis Efremov wrote:

> Check for memset()/memset_explicit() with 0 followed by
> kfree()/vfree()/kvfree().
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> Changes in v2:
> - memset_explicit() added
> - kvfree_sensitive() added
> - forall added to r1
> - ... between memset and kfree added
>   Unfortunately, it doesn't work as I would expect it to in "patch"
>   mode. I've added my comment about it in the rule. It can be safely
>   removed from the patch if I misunderstood something.
>
> Another "strange" behaviour that I faced that r2 rule works only if I
> write 2 expression lines:
> expression *E;
> expression size;
> If I try to use a single line "expression *E, size;" then r2 matches nothing.

The parser for metavariables is not so smart.  Everything to the left of
the first metavariable name is the type.  Everything after is the list of
metavariables of that type.  So if you put them together you require size
to be a pointer.

On the other hand, do you really require E to be a pointer?  If you do
that, it will have to find the type of E.  If E refers to a structure
field, then the type might not be available in the current function, and
you may need command line argments like --all-includes or
--recursive-includes.  Is avoiding transforming the case where E is not
verified to be a pointer a concern?

julia

>
>  scripts/coccinelle/api/kzfree.cocci | 65 +++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 scripts/coccinelle/api/kzfree.cocci
>
> diff --git a/scripts/coccinelle/api/kzfree.cocci b/scripts/coccinelle/api/kzfree.cocci
> new file mode 100644
> index 000000000000..5c7e4bb13bb7
> --- /dev/null
> +++ b/scripts/coccinelle/api/kzfree.cocci
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Use kzfree, kvfree_sensitive rather than memset or
> +/// memset_explicit with 0 followed by kfree
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: kzfree, kvfree_sensitive
> +//
> +
> +virtual context
> +virtual patch
> +virtual org
> +virtual report
> +
> +
> +// Ignore kzfree definition
> +// Ignore kasan test
> +@r depends on !patch && !(file in "lib/test_kasan.c") && !(file in "mm/slab_common.c") forall@
> +expression *E;
> +position p;
> +@@
> +
> +* \(memset\|memset_explicit\)(E, 0, ...);
> +  ... when != E
> +      when strict
> +* \(kfree\|vfree\|kvfree\)(E)@p;
> +
> +@r1 depends on patch && !(file in "lib/test_kasan.c") && !(file in "mm/slab_common.c")@
> +expression *E;
> +expression size;
> +@@
> +
> +- \(memset\|memset_explicit\)(E, 0, size);
> +/// Unfortunately, it doesn't work as in !patch mode.
> +/// spatch (v1.0.8) should patch 4 functions in linux 5.7 with this rule
> +/// and uncommented "when" lines. With only "... when != E" line 2 functions
> +/// are patched, none with "when strict". 3 functions patch is produced by the
> +/// rule with "when" lines commented out.
> +//  ... when != E
> +//      when strict
> +(
> +- kfree(E);
> ++ kzfree(E);
> +|
> +- vfree(E);
> ++ kvfree_sensitive(E, size);
> +|
> +- kvfree(E);
> ++ kvfree_sensitive(E, size);
> +)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for kzfree/kvfree_sensitive")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for kzfree/kvfree_sensitive")
> --
> 2.26.2
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>