Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751495AbdIWUn4 (ORCPT ); Sat, 23 Sep 2017 16:43:56 -0400 Received: from mail3-relais-sop.national.inria.fr ([192.134.164.104]:56324 "EHLO mail3-relais-sop.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750839AbdIWUnz (ORCPT ); Sat, 23 Sep 2017 16:43:55 -0400 X-IronPort-AV: E=Sophos;i="5.42,430,1500933600"; d="scan'208";a="238499770" Date: Sat, 23 Sep 2017 22:43:52 +0200 (CEST) From: Julia Lawall X-X-Sender: jll@hadrien To: Kees Cook cc: Thomas Gleixner , Gilles Muller , Nicolas Palix , Michal Marek , cocci@systeme.lip6.fr, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 01/31] coccinelle: Improve setup_timer.cocci matching In-Reply-To: <1505950075-50223-2-git-send-email-keescook@chromium.org> Message-ID: References: <1505950075-50223-1-git-send-email-keescook@chromium.org> <1505950075-50223-2-git-send-email-keescook@chromium.org> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5312 Lines: 230 On Wed, 20 Sep 2017, Kees Cook wrote: > This improves the patch mode of setup_timer.cocci. Several patterns > were missing: > - assignments-before-init_timer() cases > - limit the .data case removal to the specific struct timer_list instance > - handling calls by dereference (timer->field vs timer.field) > > Cc: Julia Lawall > Cc: Gilles Muller > Cc: Nicolas Palix > Cc: Michal Marek > Cc: cocci@systeme.lip6.fr > Signed-off-by: Kees Cook Acked-by: Julia Lawall Note that I proposed some changes on this rule as well, on August 23 (https://systeme.lip6.fr/pipermail/cocci/2017-August/004386.html). My changes are still orthogonal to the ones proposed here. Actually, my changes are in the part about matching, and this patch on covers the -D patch case (transformation). The matching rules should be extended in the same way that the patch rules are extended below, but it would be better to apply my patch first. julia > --- > scripts/coccinelle/api/setup_timer.cocci | 129 +++++++++++++++++++++++++------ > 1 file changed, 105 insertions(+), 24 deletions(-) > > diff --git a/scripts/coccinelle/api/setup_timer.cocci b/scripts/coccinelle/api/setup_timer.cocci > index eb6bd9e4ab1a..279767f3bbef 100644 > --- a/scripts/coccinelle/api/setup_timer.cocci > +++ b/scripts/coccinelle/api/setup_timer.cocci > @@ -2,6 +2,7 @@ > /// and data fields > // Confidence: High > // Copyright: (C) 2016 Vaishali Thakkar, Oracle. GPLv2 > +// Copyright: (C) 2017 Kees Cook, Google. GPLv2 > // Options: --no-includes --include-headers > // Keywords: init_timer, setup_timer > > @@ -10,60 +11,123 @@ virtual context > virtual org > virtual report > > +// Match the common cases first to avoid Coccinelle parsing loops with > +// "... when" clauses. > + > @match_immediate_function_data_after_init_timer > depends on patch && !context && !org && !report@ > expression e, func, da; > @@ > > --init_timer (&e); > -+setup_timer (&e, func, da); > +-init_timer > ++setup_timer > + ( \(&e\|e\) > ++, func, da > + ); > +( > +-\(e.function\|e->function\) = func; > +-\(e.data\|e->data\) = da; > +| > +-\(e.data\|e->data\) = da; > +-\(e.function\|e->function\) = func; > +) > + > +@match_immediate_function_data_before_init_timer > +depends on patch && !context && !org && !report@ > +expression e, func, da; > +@@ > > ( > +-\(e.function\|e->function\) = func; > +-\(e.data\|e->data\) = da; > +| > +-\(e.data\|e->data\) = da; > +-\(e.function\|e->function\) = func; > +) > +-init_timer > ++setup_timer > + ( \(&e\|e\) > ++, func, da > + ); > + > +@match_function_and_data_after_init_timer > +depends on patch && !context && !org && !report@ > +expression e, e2, e3, e4, e5, func, da; > +@@ > + > +-init_timer > ++setup_timer > + ( \(&e\|e\) > ++, func, da > + ); > + ... when != func = e2 > + when != da = e3 > +( > -e.function = func; > +... when != da = e4 > -e.data = da; > | > +-e->function = func; > +... when != da = e4 > +-e->data = da; > +| > -e.data = da; > +... when != func = e5 > -e.function = func; > +| > +-e->data = da; > +... when != func = e5 > +-e->function = func; > ) > > -@match_function_and_data_after_init_timer > +@match_function_and_data_before_init_timer > depends on patch && !context && !org && !report@ > -expression e1, e2, e3, e4, e5, a, b; > +expression e, e2, e3, e4, e5, func, da; > @@ > - > --init_timer (&e1); > -+setup_timer (&e1, a, b); > - > -... when != a = e2 > - when != b = e3 > ( > --e1.function = a; > -... when != b = e4 > --e1.data = b; > +-e.function = func; > +... when != da = e4 > +-e.data = da; > | > --e1.data = b; > -... when != a = e5 > --e1.function = a; > +-e->function = func; > +... when != da = e4 > +-e->data = da; > +| > +-e.data = da; > +... when != func = e5 > +-e.function = func; > +| > +-e->data = da; > +... when != func = e5 > +-e->function = func; > ) > +... when != func = e2 > + when != da = e3 > +-init_timer > ++setup_timer > + ( \(&e\|e\) > ++, func, da > + ); > > @r1 exists@ > +expression t; > identifier f; > position p; > @@ > > f(...) { ... when any > - init_timer@p(...) > + init_timer@p(\(&t\|t\)) > ... when any > } > > @r2 exists@ > +expression r1.t; > identifier g != r1.f; > -struct timer_list t; > expression e8; > @@ > > g(...) { ... when any > - t.data = e8 > + \(t.data\|t->data\) = e8 > ... when any > } > > @@ -77,14 +141,31 @@ p << r1.p; > cocci.include_match(False) > > @r3 depends on patch && !context && !org && !report@ > -expression e6, e7, c; > +expression r1.t, func, e7; > position r1.p; > @@ > > --init_timer@p (&e6); > -+setup_timer (&e6, c, 0UL); > -... when != c = e7 > --e6.function = c; > +( > +-init_timer@p(&t); > ++setup_timer(&t, func, 0UL); > +... when != func = e7 > +-t.function = func; > +| > +-t.function = func; > +... when != func = e7 > +-init_timer@p(&t); > ++setup_timer(&t, func, 0UL); > +| > +-init_timer@p(t); > ++setup_timer(t, func, 0UL); > +... when != func = e7 > +-t->function = func; > +| > +-t->function = func; > +... when != func = e7 > +-init_timer@p(t); > ++setup_timer(t, func, 0UL); > +) > > // ---------------------------------------------------------------------------- > > -- > 2.7.4 > >