Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Thu, 18 Jun 2020, Kees Cook wrote: > On Thu, Jun 18, 2020 at 09:56:18PM +0200, Julia Lawall wrote: > > @@ > > identifier i,fld; > > expression e; > > @@ > > > > \(\(i\|e.fld\|e->fld\) \& E\) > > > > The e will match all of the variants you are concerned about. > > Ah, I see! Okay, that's good. And the "& E" part is to effectively > collect it into E (as in, both the left and right of the & must match). Yes > So to do the matching from earlier: > > @@ > identifier i, fld; > expression e, ARG1, ARG2; > @@ > > array_size(\(\(i\|e.fld\|e->fld\) \& ARG1\), ARG2); Yes, that would be fine. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Thu, Jun 18, 2020 at 09:56:18PM +0200, Julia Lawall wrote: > @@ > identifier i,fld; > expression e; > @@ > > \(\(i\|e.fld\|e->fld\) \& E\) > > The e will match all of the variants you are concerned about. Ah, I see! Okay, that's good. And the "& E" part is to effectively collect it into E (as in, both the left and right of the & must match). So to do the matching from earlier: @@ identifier i, fld; expression e, ARG1, ARG2; @@ array_size(\(\(i\|e.fld\|e->fld\) \& ARG1\), ARG2); ? -- Kees Cook ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Thu, 18 Jun 2020, Kees Cook wrote: > On Wed, Jun 17, 2020 at 08:54:03PM +0200, Julia Lawall wrote: > > > > > > On Wed, 17 Jun 2020, Kees Cook wrote: > > > > > On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: > > > > +@as@ > > > > +expression E1, E2; > > > > +@@ > > > > + > > > > +array_size(E1, E2) > > > > > > BTW, is there a way yet in Coccinelle to match a fully qualified (?) > > > identifier? For example, if I have two lines in C: > > > > > > A) > > > array_size(variable, 5); > > > B) > > > array_size(instance->member.size, 5); > > > C) > > > array_size(instance->member.size + 1, 5); > > > D) > > > array_size(function_call(variable), 5); > > > > > > > > > This matches A, B, C, and D: > > > > > > @@ > > > expression ARG1; > > > expression ARG2; > > > @@ > > > > > > array_size(ARG1, ARG2); > > > > > > > > > This matches only A: > > > > > > @@ > > > identifier ARG1; > > > expression ARG2; > > > @@ > > > > > > array_size(ARG1, ARG2); > > > > > > > > > How do I get something to match A and B but not C and D (i.e. I do not > > > want to match any operations, function calls, etc, only a variable, > > > which may be identified through dereference, array index, or struct > > > member access.) > > > > \(i\|e.fld\|e->fld\) > > > > would probably do what you want. It will also match cases where e is a > > function/macr call, but that is unlikely. > > > > If you want a single metavariable that contains the whole thing, you can > > have an expression metavariable E and then write: > > > > \(\(i\|e.fld\|e->fld\) \& E\) > > Can you give an example of how that would look for an @@ section? > > The problem I have is that I don't know the depth or combination of such > metavariables. There are a lot of combinations: > > a > a.b > a.b.c > a.b.c.d > a.b.c->d > a.b->c > a.b->c.d > a.b->c->d > a->b > a->b.c > a->b.c.d > a->b.c->d > a->b->c > a->b->c.d > a->b->c->d > ... @@ identifier i,fld; expression e; @@ \(\(i\|e.fld\|e->fld\) \& E\) The e will match all of the variants you are concerned about. julia > > > -- > Kees Cook > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Wed, Jun 17, 2020 at 08:54:03PM +0200, Julia Lawall wrote: > > > On Wed, 17 Jun 2020, Kees Cook wrote: > > > On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: > > > +@as@ > > > +expression E1, E2; > > > +@@ > > > + > > > +array_size(E1, E2) > > > > BTW, is there a way yet in Coccinelle to match a fully qualified (?) > > identifier? For example, if I have two lines in C: > > > > A) > > array_size(variable, 5); > > B) > > array_size(instance->member.size, 5); > > C) > > array_size(instance->member.size + 1, 5); > > D) > > array_size(function_call(variable), 5); > > > > > > This matches A, B, C, and D: > > > > @@ > > expression ARG1; > > expression ARG2; > > @@ > > > > array_size(ARG1, ARG2); > > > > > > This matches only A: > > > > @@ > > identifier ARG1; > > expression ARG2; > > @@ > > > > array_size(ARG1, ARG2); > > > > > > How do I get something to match A and B but not C and D (i.e. I do not > > want to match any operations, function calls, etc, only a variable, > > which may be identified through dereference, array index, or struct > > member access.) > > \(i\|e.fld\|e->fld\) > > would probably do what you want. It will also match cases where e is a > function/macr call, but that is unlikely. > > If you want a single metavariable that contains the whole thing, you can > have an expression metavariable E and then write: > > \(\(i\|e.fld\|e->fld\) \& E\) Can you give an example of how that would look for an @@ section? The problem I have is that I don't know the depth or combination of such metavariables. There are a lot of combinations: a a.b a.b.c a.b.c.d a.b.c->d a.b->c a.b->c.d a.b->c->d a->b a->b.c a->b.c.d a->b.c->d a->b->c a->b->c.d a->b->c->d ... -- Kees Cook ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Wed, 17 Jun 2020, Denis Efremov wrote: > > > On 6/17/20 11:30 PM, Julia Lawall wrote: > > > > > > 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. > > > > Thanks, I will send v2. > Here is the KSPP ticket with patches https://github.com/KSPP/linux/issues/83 Thanks! julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On 6/17/20 11:30 PM, Julia Lawall wrote: > > > 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. > Thanks, I will send v2. Here is the KSPP ticket with patches https://github.com/KSPP/linux/issues/83 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
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 > Signed-off-by: Denis Efremov > --- > 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 ..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 != \(\|\|\|\) > +* 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@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 != \(\|\|\|\) > +* 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 != \(\|\|\|\|\|\) > +* 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@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 != \(\|\|\|\|\|\) > +* 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
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Wed, 17 Jun 2020, Julia Lawall wrote: > > > On Wed, 17 Jun 2020, Denis Efremov wrote: > > > > > > > > > Awesome! I'll take a look into this. :) > > > > > Here is another script for your #83 ticket. > > Currently, it issues 598 warnings. > > > > // SPDX-License-Identifier: GPL-2.0-only > > /// > > /// Check for missing overflow checks in allocation functions. > > /// Low confidence because it's pointless to check for overflow > > /// relatively small allocations. > > /// > > // Confidence: Low > > // Copyright: (C) 2020 Denis Efremov ISPRAS > > // Options: --no-includes --include-headers > > > > virtual patch > > virtual context > > virtual org > > virtual report > > > > @depends on patch@ > > expression E1, E2, E3, E4, size; > > @@ > > > > ( > > - size = E1 * E2; > > + size = array_size(E1, E2); > > | > > - size = E1 * E2 * E3; > > + size = array3_size(E1, E2, E3); > > | > > - size = E1 * E2 + E3; > > + size = struct_size(E1, E2, E3); > > Should the arguments be checked to see if they have something to do with > arrays and structures? Sorry for the noise, I see that this comment makes no sense. julia > > > ) > > ... when != size = E4 > > when != size += E4 > > when != size -= E4 > > when != size *= E4 > > Here you can have a metavariable > > assignment operator aop; > > and then say size aop E4 > > It doesn't really look like an assignment any more, but it could be a > little safer. > > julia > > > when != > > \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| > > vmalloc\|vzalloc\|vzalloc_node\| > > kvmalloc\|kvzalloc\|kvzalloc_node\| > > sock_kmalloc\| > > f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| > > devm_kmalloc\|devm_kzalloc\) > > (..., size, ...) > > > > @r depends on !patch@ > > expression E1, E2, E3, E4, size; > > position p; > > @@ > > > > ( > > * size = E1 * E2;@p > > | > > * size = E1 * E2 * E3;@p > > | > > * size = E1 * E2 + E3;@p > > ) > > ... when != size = E4 > > when != size += E4 > > when != size -= E4 > > when != size *= E4 > > when != > > * \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| > > vmalloc\|vzalloc\|vzalloc_node\| > > kvmalloc\|kvzalloc\|kvzalloc_node\| > > sock_kmalloc\| > > f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| > > devm_kmalloc\|devm_kzalloc\) > > (..., size, ...) > > > > @script:python depends on report@ > > p << r.p; > > @@ > > > > coccilib.report.print_report(p[0], "WARNING: missing overflow check") > > > > @script:python depends on org@ > > p << r.p; > > @@ > > > > coccilib.org.print_todo(p[0], "WARNING: missing overflow check") > > ___ > > 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 > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Wed, 17 Jun 2020, Denis Efremov wrote: > > > > > Awesome! I'll take a look into this. :) > > > Here is another script for your #83 ticket. > Currently, it issues 598 warnings. > > // SPDX-License-Identifier: GPL-2.0-only > /// > /// Check for missing overflow checks in allocation functions. > /// Low confidence because it's pointless to check for overflow > /// relatively small allocations. > /// > // Confidence: Low > // Copyright: (C) 2020 Denis Efremov ISPRAS > // Options: --no-includes --include-headers > > virtual patch > virtual context > virtual org > virtual report > > @depends on patch@ > expression E1, E2, E3, E4, size; > @@ > > ( > - size = E1 * E2; > + size = array_size(E1, E2); > | > - size = E1 * E2 * E3; > + size = array3_size(E1, E2, E3); > | > - size = E1 * E2 + E3; > + size = struct_size(E1, E2, E3); Should the arguments be checked to see if they have something to do with arrays and structures? > ) > ... when != size = E4 > when != size += E4 > when != size -= E4 > when != size *= E4 Here you can have a metavariable assignment operator aop; and then say size aop E4 It doesn't really look like an assignment any more, but it could be a little safer. julia > when != > \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| > vmalloc\|vzalloc\|vzalloc_node\| > kvmalloc\|kvzalloc\|kvzalloc_node\| > sock_kmalloc\| > f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| > devm_kmalloc\|devm_kzalloc\) > (..., size, ...) > > @r depends on !patch@ > expression E1, E2, E3, E4, size; > position p; > @@ > > ( > * size = E1 * E2;@p > | > * size = E1 * E2 * E3;@p > | > * size = E1 * E2 + E3;@p > ) > ... when != size = E4 > when != size += E4 > when != size -= E4 > when != size *= E4 > when != > * \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| > vmalloc\|vzalloc\|vzalloc_node\| > kvmalloc\|kvzalloc\|kvzalloc_node\| > sock_kmalloc\| > f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| > devm_kmalloc\|devm_kzalloc\) > (..., size, ...) > > @script:python depends on report@ > p << r.p; > @@ > > coccilib.report.print_report(p[0], "WARNING: missing overflow check") > > @script:python depends on org@ > p << r.p; > @@ > > coccilib.org.print_todo(p[0], "WARNING: missing overflow check") > ___ > 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
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Wed, 17 Jun 2020, Kees Cook wrote: > On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: > > +@as@ > > +expression E1, E2; > > +@@ > > + > > +array_size(E1, E2) > > BTW, is there a way yet in Coccinelle to match a fully qualified (?) > identifier? For example, if I have two lines in C: > > A) > array_size(variable, 5); > B) > array_size(instance->member.size, 5); > C) > array_size(instance->member.size + 1, 5); > D) > array_size(function_call(variable), 5); > > > This matches A, B, C, and D: > > @@ > expression ARG1; > expression ARG2; > @@ > > array_size(ARG1, ARG2); > > > This matches only A: > > @@ > identifier ARG1; > expression ARG2; > @@ > > array_size(ARG1, ARG2); > > > How do I get something to match A and B but not C and D (i.e. I do not > want to match any operations, function calls, etc, only a variable, > which may be identified through dereference, array index, or struct > member access.) \(i\|e.fld\|e->fld\) would probably do what you want. It will also match cases where e is a function/macr call, but that is unlikely. If you want a single metavariable that contains the whole thing, you can have an expression metavariable E and then write: \(\(i\|e.fld\|e->fld\) \& E\) julia > > > -- > Kees Cook > ___ > 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
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: > +@as@ > +expression E1, E2; > +@@ > + > +array_size(E1, E2) BTW, is there a way yet in Coccinelle to match a fully qualified (?) identifier? For example, if I have two lines in C: A) array_size(variable, 5); B) array_size(instance->member.size, 5); C) array_size(instance->member.size + 1, 5); D) array_size(function_call(variable), 5); This matches A, B, C, and D: @@ expression ARG1; expression ARG2; @@ array_size(ARG1, ARG2); This matches only A: @@ identifier ARG1; expression ARG2; @@ array_size(ARG1, ARG2); How do I get something to match A and B but not C and D (i.e. I do not want to match any operations, function calls, etc, only a variable, which may be identified through dereference, array index, or struct member access.) -- Kees Cook ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
> > Awesome! I'll take a look into this. :) > Here is another script for your #83 ticket. Currently, it issues 598 warnings. // SPDX-License-Identifier: GPL-2.0-only /// /// Check for missing overflow checks in allocation functions. /// Low confidence because it's pointless to check for overflow /// relatively small allocations. /// // Confidence: Low // Copyright: (C) 2020 Denis Efremov ISPRAS // Options: --no-includes --include-headers virtual patch virtual context virtual org virtual report @depends on patch@ expression E1, E2, E3, E4, size; @@ ( - size = E1 * E2; + size = array_size(E1, E2); | - size = E1 * E2 * E3; + size = array3_size(E1, E2, E3); | - size = E1 * E2 + E3; + size = struct_size(E1, E2, E3); ) ... when != size = E4 when != size += E4 when != size -= E4 when != size *= E4 when != \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| vmalloc\|vzalloc\|vzalloc_node\| kvmalloc\|kvzalloc\|kvzalloc_node\| sock_kmalloc\| f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| devm_kmalloc\|devm_kzalloc\) (..., size, ...) @r depends on !patch@ expression E1, E2, E3, E4, size; position p; @@ ( * size = E1 * E2;@p | * size = E1 * E2 * E3;@p | * size = E1 * E2 + E3;@p ) ... when != size = E4 when != size += E4 when != size -= E4 when != size *= E4 when != * \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| vmalloc\|vzalloc\|vzalloc_node\| kvmalloc\|kvzalloc\|kvzalloc_node\| sock_kmalloc\| f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| devm_kmalloc\|devm_kzalloc\) (..., size, ...) @script:python depends on report@ p << r.p; @@ coccilib.report.print_report(p[0], "WARNING: missing overflow check") @script:python depends on org@ p << r.p; @@ coccilib.org.print_todo(p[0], "WARNING: missing overflow check") ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
> > > Awesome! I'll take a look into this. :) > It would be helpful to get a feedback from you after. What kind of warnings are helpful and what are not? "duplicate calls" and "opencoded expression after array_size()" look doubtful to me. I think that maintainers will not like these patches. Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On 6/15/20 13:35, Denis Efremov wrote: > > > On 6/15/20 9:23 PM, Kees Cook wrote: >> On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: >>> Detect an opencoded expression that is used before or after >>> array_size()/array3_size()/struct_size() to compute the same size. >>> >>> Cc: Kees Cook >>> Signed-off-by: Denis Efremov >> >> Oh, very cool! How much does this find currently? >> > > opencoded expression before the function call: > ./drivers/net/ethernet/cavium/liquidio/request_manager.c:98:34-59: WARNING: > array_size is used down the code (line 103) to compute the same size > ./drivers/media/test-drivers/vivid/vivid-core.c:1120:26-34: WARNING: > array_size is used down the code (line 1122) to compute the same size > ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5184:11-31: WARNING: array_size > is used down the code (line 5191) to compute the same size > ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5200:2-37: WARNING: array_size > is used down the code (line 5207) to compute the same size > ./fs/cifs/misc.c:853:17-39: WARNING: array_size is used down the code (line > 858) to compute the same size > ./fs/cifs/misc.c:863:17-38: WARNING: array_size is used down the code (line > 868) to compute the same size > ./drivers/scsi/fnic/fnic_trace.c:562:25-48: WARNING: array_size is used down > the code (line 566) to compute the same size > > opencoded expression after the function call: > ./net/ethtool/ioctl.c:1976:55-66: WARNING: array_size is already used (line > 1957) to compute the same size > ./net/ethtool/ioctl.c:1921:55-66: WARNING: array_size is already used (line > 1909) to compute the same size > ./drivers/net/ethernet/cavium/liquidio/request_manager.c:111:29-54: WARNING: > array_size is already used (line 103) to compute the same size > ./drivers/staging/rts5208/ms.c:2309:55-56: WARNING: array_size is already > used (line 2305) to compute the same size > ./drivers/video/fbdev/core/fbcon.c:642:52-53: WARNING: array3_size is already > used (line 638) to compute the same size > ./drivers/video/fbdev/core/fbcon.c:679:47-48: WARNING: array3_size is already > used (line 638) to compute the same size > ./drivers/usb/misc/sisusbvga/sisusb_con.c:1229:54-56: WARNING: array_size is > already used (line 1226) to compute the same size > ./fs/afs/cmservice.c:271:45-46: WARNING: array3_size is already used (line > 267) to compute the same size > ./drivers/mtd/ftl.c:270:49-65: WARNING: array_size is already used (line 266) > to compute the same size > ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1608:6-42: WARNING: array_size is > already used (line 1605) to compute the same size > ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1613:8-44: WARNING: array_size is > already used (line 1605) to compute the same size > ./drivers/net/ppp/bsd_comp.c:439:13-37: WARNING: array_size is already used > (line 409) to compute the same size > ./drivers/net/wireless/ath/ath5k/debug.c:957:20-21: WARNING: array_size is > already used (line 934) to compute the same size > ./drivers/scsi/fnic/fnic_trace.c:575:3-26: WARNING: array_size is already > used (line 566) to compute the same size > ./drivers/scsi/fnic/fnic_trace.c:592:32-53: WARNING: array_size is already > used (line 580) to compute the same size > ./drivers/scsi/fnic/fnic_trace.c:504:30-51: WARNING: array_size is already > used (line 492) to compute the same size > ./drivers/staging/rts5208/rtsx_chip.c:1475:17-18: WARNING: array_size is > already used (line 1458) to compute the same size > ./kernel/kexec_file.c:917:8-25: WARNING: array_size is already used (line > 913) to compute the same size > ./drivers/rapidio/devices/rio_mport_cdev.c:984:8-25: WARNING: array_size is > already used (line 978) to compute the same size > ./fs/reiserfs/bitmap.c:1463:22-37: WARNING: array_size is already used (line > 1459) to compute the same size > > duplicate calls: > ./drivers/media/test-drivers/vivid/vivid-core.c:1125:59-60: WARNING: same > array_size (line 1122) > ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:142:36-37: WARNING: same > array_size (line 138) > ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:129:41-42: WARNING: same > array3_size (line 123) > ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same > array3_size (line 123) > ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same > array3_size (line 129) > ./drivers/net/ethernet/cavium/liquidio/octeon_droq.c:289:27-28: WARNING: same > array_size (line 284) > ./drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:857:59-60: WARNING: same > struct_size (line 854) > ./fs/f2fs/super.c:3478:34-35: WARNING: same array_size (line 3478) > ./drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1637:45-46: WARNING: same > struct_size (line 1634) > ./drivers/net/ethernet/netronome/nfp/flower/cmsg.c:221:49-50: WARNING: same > struct_size (line 219) > ./drivers/staging/rts5208/rtsx_chip.c:1458:36-37: WARNING: same array_size > (line 1454) >
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On 6/15/20 9:23 PM, Kees Cook wrote: > On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: >> Detect an opencoded expression that is used before or after >> array_size()/array3_size()/struct_size() to compute the same size. >> >> Cc: Kees Cook >> Signed-off-by: Denis Efremov > > Oh, very cool! How much does this find currently? > opencoded expression before the function call: ./drivers/net/ethernet/cavium/liquidio/request_manager.c:98:34-59: WARNING: array_size is used down the code (line 103) to compute the same size ./drivers/media/test-drivers/vivid/vivid-core.c:1120:26-34: WARNING: array_size is used down the code (line 1122) to compute the same size ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5184:11-31: WARNING: array_size is used down the code (line 5191) to compute the same size ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5200:2-37: WARNING: array_size is used down the code (line 5207) to compute the same size ./fs/cifs/misc.c:853:17-39: WARNING: array_size is used down the code (line 858) to compute the same size ./fs/cifs/misc.c:863:17-38: WARNING: array_size is used down the code (line 868) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:562:25-48: WARNING: array_size is used down the code (line 566) to compute the same size opencoded expression after the function call: ./net/ethtool/ioctl.c:1976:55-66: WARNING: array_size is already used (line 1957) to compute the same size ./net/ethtool/ioctl.c:1921:55-66: WARNING: array_size is already used (line 1909) to compute the same size ./drivers/net/ethernet/cavium/liquidio/request_manager.c:111:29-54: WARNING: array_size is already used (line 103) to compute the same size ./drivers/staging/rts5208/ms.c:2309:55-56: WARNING: array_size is already used (line 2305) to compute the same size ./drivers/video/fbdev/core/fbcon.c:642:52-53: WARNING: array3_size is already used (line 638) to compute the same size ./drivers/video/fbdev/core/fbcon.c:679:47-48: WARNING: array3_size is already used (line 638) to compute the same size ./drivers/usb/misc/sisusbvga/sisusb_con.c:1229:54-56: WARNING: array_size is already used (line 1226) to compute the same size ./fs/afs/cmservice.c:271:45-46: WARNING: array3_size is already used (line 267) to compute the same size ./drivers/mtd/ftl.c:270:49-65: WARNING: array_size is already used (line 266) to compute the same size ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1608:6-42: WARNING: array_size is already used (line 1605) to compute the same size ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1613:8-44: WARNING: array_size is already used (line 1605) to compute the same size ./drivers/net/ppp/bsd_comp.c:439:13-37: WARNING: array_size is already used (line 409) to compute the same size ./drivers/net/wireless/ath/ath5k/debug.c:957:20-21: WARNING: array_size is already used (line 934) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:575:3-26: WARNING: array_size is already used (line 566) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:592:32-53: WARNING: array_size is already used (line 580) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:504:30-51: WARNING: array_size is already used (line 492) to compute the same size ./drivers/staging/rts5208/rtsx_chip.c:1475:17-18: WARNING: array_size is already used (line 1458) to compute the same size ./kernel/kexec_file.c:917:8-25: WARNING: array_size is already used (line 913) to compute the same size ./drivers/rapidio/devices/rio_mport_cdev.c:984:8-25: WARNING: array_size is already used (line 978) to compute the same size ./fs/reiserfs/bitmap.c:1463:22-37: WARNING: array_size is already used (line 1459) to compute the same size duplicate calls: ./drivers/media/test-drivers/vivid/vivid-core.c:1125:59-60: WARNING: same array_size (line 1122) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:142:36-37: WARNING: same array_size (line 138) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:129:41-42: WARNING: same array3_size (line 123) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 123) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 129) ./drivers/net/ethernet/cavium/liquidio/octeon_droq.c:289:27-28: WARNING: same array_size (line 284) ./drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:857:59-60: WARNING: same struct_size (line 854) ./fs/f2fs/super.c:3478:34-35: WARNING: same array_size (line 3478) ./drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1637:45-46: WARNING: same struct_size (line 1634) ./drivers/net/ethernet/netronome/nfp/flower/cmsg.c:221:49-50: WARNING: same struct_size (line 219) ./drivers/staging/rts5208/rtsx_chip.c:1458:36-37: WARNING: same array_size (line 1454) ./drivers/net/ethernet/neterion/vxge/vxge-config.c:2664:59-60: WARNING: same array_size (line 2654) ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: > Detect an opencoded expression that is used before or after > array_size()/array3_size()/struct_size() to compute the same size. > > Cc: Kees Cook > Signed-off-by: Denis Efremov Oh, very cool! How much does this find currently? -- Kees Cook ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
