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:   Wed, 17 Jun 2020 22:30:43 +0200 (CEST)
From:   Julia Lawall <julia.lawall@inria.fr>
To:     Denis Efremov <efremov@linux.com>
cc:     Kees Cook <keescook@chromium.org>, cocci@systeme.lip6.fr,
        linux-kernel@vger.kernel.org
Subject: Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to
 detect missed overlow checks
In-Reply-To: <20200615102045.4558-1-efremov@linux.com>
Message-ID: <alpine.DEB.2.22.394.2006172229550.3083@hadrien>
References: <20200615102045.4558-1-efremov@linux.com>
User-Agent: Alpine 2.22 (DEB 394 2020-01-19)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On Mon, 15 Jun 2020, Denis Efremov wrote:

> Detect an opencoded expression that is used before or after
> array_size()/array3_size()/struct_size() to compute the same size.

This would benefit from the assignemnt operator metavariables as well.

Also, it could be better to put the python rules up next the SmPL pattern
matching rules that they are associated with.

julia


>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  scripts/coccinelle/misc/array_size_dup.cocci | 347 +++++++++++++++++++
>  1 file changed, 347 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
>
> diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci
> new file mode 100644
> index 000000000000..08919a938754
> --- /dev/null
> +++ b/scripts/coccinelle/misc/array_size_dup.cocci
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for array_size(), array3_size(), struct_size() duplicates.
> +/// Three types of patterns for these functions:
> +///  1. An opencoded expression is used before array_size() to compute the same size
> +///  2. An opencoded expression is used after array_size() to compute the same size
> +///  3. Consecutive calls of array_size() with the same values
> +/// From security point of view only first case is relevant. These functions
> +/// perform arithmetic overflow check. Thus, if we use an opencoded expression
> +/// before a call to the *_size() function we can miss an overflow.
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers --no-loops
> +
> +virtual context
> +virtual report
> +virtual org
> +
> +@as@
> +expression E1, E2;
> +@@
> +
> +array_size(E1, E2)
> +
> +@as_next@
> +expression subE1 <= as.E1;
> +expression as.E1;
> +expression subE2 <= as.E2;
> +expression as.E2;
> +expression E3;
> +position p1, p2;
> +@@
> +
> +* E1 * E2@p1
> +  ... when != \(E1\|E2\|subE1\|subE2\)=E3
> +      when != \(E1\|E2\|subE1\|subE2\)+=E3
> +      when != \(E1\|E2\|subE1\|subE2\)-=E3
> +      when != \(E1\|E2\|subE1\|subE2\)*=E3
> +      when != \(&E1\|&E2\|&subE1\|&subE2\)
> +* array_size(E1, E2)@p2
> +
> +@as_prev@
> +expression subE1 <= as.E1;
> +expression as.E1;
> +expression subE2 <= as.E2;
> +expression as.E2;
> +expression E3;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> +  ... when != \(E1\|E2\|subE1\|subE2\)=E3
> +      when != \(E1\|E2\|subE1\|subE2\)+=E3
> +      when != \(E1\|E2\|subE1\|subE2\)-=E3
> +      when != \(E1\|E2\|subE1\|subE2\)*=E3
> +      when != \(&E1\|&E2\|&subE1\|&subE2\)
> +* E1 * E2@p2
> +
> +@as_dup@
> +expression subE1 <= as.E1;
> +expression as.E1;
> +expression subE2 <= as.E2;
> +expression as.E2;
> +expression E3;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> +  ... when != \(E1\|E2\|subE1\|subE2\)=E3
> +      when != \(E1\|E2\|subE1\|subE2\)+=E3
> +      when != \(E1\|E2\|subE1\|subE2\)-=E3
> +      when != \(E1\|E2\|subE1\|subE2\)*=E3
> +      when != \(&E1\|&E2\|&subE1\|&subE2\)
> +* array_size(E1, E2)@p2
> +
> +@as3@
> +expression E1, E2, E3;
> +@@
> +
> +array3_size(E1, E2, E3)
> +
> +@as3_next@
> +expression subE1 <= as3.E1;
> +expression as3.E1;
> +expression subE2 <= as3.E2;
> +expression as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 * E3@p1
> +  ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> +      when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@as3_prev@
> +expression subE1 <= as3.E1;
> +expression as3.E1;
> +expression subE2 <= as3.E2;
> +expression as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> +  ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> +      when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* E1 * E2 * E3@p2
> +
> +@as3_dup@
> +expression subE1 <= as3.E1;
> +expression as3.E1;
> +expression subE2 <= as3.E2;
> +expression as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> +  ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> +      when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@ss@
> +expression E1, E2, E3;
> +@@
> +
> +struct_size(E1, E2, E3)
> +
> +@ss_next@
> +expression subE1 <= ss.E1;
> +expression ss.E1;
> +expression subE2 <= ss.E2;
> +expression ss.E2;
> +expression subE3 <= ss.E3;
> +expression ss.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 + E3@p1
> +  ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> +      when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@ss_prev@
> +expression subE1 <= ss.E1;
> +expression ss.E1;
> +expression subE2 <= ss.E2;
> +expression ss.E2;
> +expression subE3 <= ss.E3;
> +expression ss.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> +  ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> +      when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* E1 * E2 + E3@p2
> +
> +@ss_dup@
> +expression subE1 <= ss.E1;
> +expression ss.E1;
> +expression subE2 <= ss.E2;
> +expression ss.E2;
> +expression subE3 <= ss.E3;
> +expression ss.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> +  ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> +      when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> +      when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +msg = "WARNING: array_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +msg = "WARNING: array_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@script:python depends on report@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@script:python depends on report@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +msg = "WARNING: same array_size (line %s)" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +msg = "WARNING: same array_size (line %s)" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +
> +@script:python depends on report@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +msg = "WARNING: array3_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +msg = "WARNING: array3_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@script:python depends on report@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@script:python depends on report@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +msg = "WARNING: same array3_size (line %s)" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +msg = "WARNING: same array3_size (line %s)" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +
> +@script:python depends on report@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +msg = "WARNING: struct_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +msg = "WARNING: struct_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@script:python depends on report@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@script:python depends on report@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +msg = "WARNING: same struct_size (line %s)" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +msg = "WARNING: same struct_size (line %s)" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> --
> 2.26.2
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>