cocci.inria.fr archive mirror
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@inria.fr>
To: Denis Efremov <efremov@linux.com>
Cc: linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>,
	cocci@systeme.lip6.fr
Subject: Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
Date: Wed, 17 Jun 2020 22:30:43 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2006172229550.3083@hadrien> (raw)
In-Reply-To: <20200615102045.4558-1-efremov@linux.com>



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
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

  parent reply	other threads:[~2020-06-17 20:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-15 10:20 [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks Denis Efremov
2020-06-15 18:23 ` Kees Cook
2020-06-15 18:35   ` Denis Efremov
2020-06-15 18:46     ` Gustavo A. R. Silva
2020-06-17  9:32       ` Denis Efremov
2020-06-17 10:55       ` Denis Efremov
2020-06-17 20:08         ` Julia Lawall
2020-06-17 20:15           ` Julia Lawall
2020-06-17 18:15 ` Kees Cook
2020-06-17 18:54   ` Julia Lawall
2020-06-18 19:52     ` Kees Cook
2020-06-18 19:56       ` Julia Lawall
2020-06-18 20:48         ` Kees Cook
2020-06-18 21:08           ` Julia Lawall
2020-06-17 20:30 ` Julia Lawall [this message]
2020-06-17 20:50   ` Denis Efremov
2020-06-17 20:52     ` Julia Lawall
2020-06-18 10:23 ` [Cocci] [PATCH v2] " Denis Efremov
2020-06-19 13:13 ` [Cocci] [PATCH v3] coccinelle: misc: add array_size_dup script to detect missed overflow checks Denis Efremov
2020-06-21 20:53   ` Julia Lawall
2020-06-21 20:56   ` Julia Lawall
2020-06-22 12:12     ` Denis Efremov
2020-06-22 12:16     ` Denis Efremov
2020-06-22 12:19       ` Julia Lawall
2020-06-22 22:10 ` [Cocci] [PATCH v4] " Denis Efremov
2020-06-24 19:42   ` Julia Lawall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.22.394.2006172229550.3083@hadrien \
    --to=julia.lawall@inria.fr \
    --cc=cocci@systeme.lip6.fr \
    --cc=efremov@linux.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).