From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759235AbcCVNSg (ORCPT ); Tue, 22 Mar 2016 09:18:36 -0400 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:53362 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758421AbcCVNS0 (ORCPT ); Tue, 22 Mar 2016 09:18:26 -0400 X-IronPort-AV: E=Sophos;i="5.24,377,1454972400"; d="scan'208";a="209473263" Date: Tue, 22 Mar 2016 14:18:22 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: Nicolai Stange cc: Greg Kroah-Hartman , Rasmus Villemoes , "Paul E. McKenney" , Alexander Viro , Jonathan Corbet , Jan Kara , Andrew Morton , Julia Lawall , Gilles Muller , Nicolas Palix , Michal Marek , linux-kernel@vger.kernel.org, cocci@systeme.lip6.fr Subject: Re: [PATCH v6 4/8] debugfs, coccinelle: check for obsolete DEFINE_SIMPLE_ATTRIBUTE() usage In-Reply-To: <1458652280-19785-5-git-send-email-nicstange@gmail.com> Message-ID: References: <1458652280-19785-1-git-send-email-nicstange@gmail.com> <1458652280-19785-5-git-send-email-nicstange@gmail.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) 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 On Tue, 22 Mar 2016, Nicolai Stange wrote: > In order to protect against file removal races, debugfs files created via > debugfs_create_file() now get wrapped by a struct file_operations at their > opening. > > If the original struct file_operations are known to be safe against removal > races by themselves already, the proxy creation may be bypassed by creating > the files through debugfs_create_file_unsafe(). > > In order to help debugfs users who use the common > DEFINE_SIMPLE_ATTRIBUTE() + debugfs_create_file() > idiom to transition to removal safe struct file_operations, the helper > macro DEFINE_DEBUGFS_ATTRIBUTE() has been introduced. > > Thus, the preferred strategy is to use > DEFINE_DEBUGFS_ATTRIBUTE() + debugfs_create_file_unsafe() > now. > > Introduce a Coccinelle script that searches for > DEFINE_SIMPLE_ATTRIBUTE()-defined struct file_operations handed into > debugfs_create_file(). Suggest to turn these usages into the > DEFINE_DEBUGFS_ATTRIBUTE() + debugfs_create_file_unsafe() > pattern. > > Signed-off-by: Nicolai Stange In terms of the structure of the semantic patch: Acked-by: Julia Lawall > --- > .../api/debugfs/debugfs_simple_attr.cocci | 67 ++++++++++++++++++++++ > 1 file changed, 67 insertions(+) > create mode 100644 scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > > diff --git a/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > new file mode 100644 > index 0000000..85cf540 > --- /dev/null > +++ b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > @@ -0,0 +1,67 @@ > +/// Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE > +/// for debugfs files. > +/// > +//# Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() > +//# imposes some significant overhead as compared to > +//# DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe(). > +// > +// Copyright (C): 2016 Nicolai Stange > +// Options: --no-includes > +// > + > +virtual context > +virtual patch > +virtual org > +virtual report > + > +@dsa@ > +declarer name DEFINE_SIMPLE_ATTRIBUTE; > +identifier dsa_fops; > +expression dsa_get, dsa_set, dsa_fmt; > +position p; > +@@ > +DEFINE_SIMPLE_ATTRIBUTE@p(dsa_fops, dsa_get, dsa_set, dsa_fmt); > + > +@dcf@ > +expression name, mode, parent, data; > +identifier dsa.dsa_fops; > +@@ > +debugfs_create_file(name, mode, parent, data, &dsa_fops) > + > + > +@context_dsa depends on context && dcf@ > +declarer name DEFINE_DEBUGFS_ATTRIBUTE; > +identifier dsa.dsa_fops; > +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt; > +@@ > +* DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > + > + > +@patch_dcf depends on patch expression@ > +expression name, mode, parent, data; > +identifier dsa.dsa_fops; > +@@ > +- debugfs_create_file(name, mode, parent, data, &dsa_fops) > ++ debugfs_create_file_unsafe(name, mode, parent, data, &dsa_fops) > + > +@patch_dsa depends on patch_dcf && patch@ > +identifier dsa.dsa_fops; > +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt; > +@@ > +- DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > ++ DEFINE_DEBUGFS_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > + > + > +@script:python depends on org && dcf@ > +fops << dsa.dsa_fops; > +p << dsa.p; > +@@ > +msg="%s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops) > +coccilib.org.print_todo(p[0], msg) > + > +@script:python depends on report && dcf@ > +fops << dsa.dsa_fops; > +p << dsa.p; > +@@ > +msg="WARNING: %s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops) > +coccilib.report.print_report(p[0], msg) > -- > 2.7.4 > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: julia.lawall@lip6.fr (Julia Lawall) Date: Tue, 22 Mar 2016 14:18:22 +0100 (CET) Subject: [Cocci] [PATCH v6 4/8] debugfs, coccinelle: check for obsolete DEFINE_SIMPLE_ATTRIBUTE() usage In-Reply-To: <1458652280-19785-5-git-send-email-nicstange@gmail.com> References: <1458652280-19785-1-git-send-email-nicstange@gmail.com> <1458652280-19785-5-git-send-email-nicstange@gmail.com> Message-ID: To: cocci@systeme.lip6.fr List-Id: cocci@systeme.lip6.fr On Tue, 22 Mar 2016, Nicolai Stange wrote: > In order to protect against file removal races, debugfs files created via > debugfs_create_file() now get wrapped by a struct file_operations at their > opening. > > If the original struct file_operations are known to be safe against removal > races by themselves already, the proxy creation may be bypassed by creating > the files through debugfs_create_file_unsafe(). > > In order to help debugfs users who use the common > DEFINE_SIMPLE_ATTRIBUTE() + debugfs_create_file() > idiom to transition to removal safe struct file_operations, the helper > macro DEFINE_DEBUGFS_ATTRIBUTE() has been introduced. > > Thus, the preferred strategy is to use > DEFINE_DEBUGFS_ATTRIBUTE() + debugfs_create_file_unsafe() > now. > > Introduce a Coccinelle script that searches for > DEFINE_SIMPLE_ATTRIBUTE()-defined struct file_operations handed into > debugfs_create_file(). Suggest to turn these usages into the > DEFINE_DEBUGFS_ATTRIBUTE() + debugfs_create_file_unsafe() > pattern. > > Signed-off-by: Nicolai Stange In terms of the structure of the semantic patch: Acked-by: Julia Lawall > --- > .../api/debugfs/debugfs_simple_attr.cocci | 67 ++++++++++++++++++++++ > 1 file changed, 67 insertions(+) > create mode 100644 scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > > diff --git a/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > new file mode 100644 > index 0000000..85cf540 > --- /dev/null > +++ b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > @@ -0,0 +1,67 @@ > +/// Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE > +/// for debugfs files. > +/// > +//# Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() > +//# imposes some significant overhead as compared to > +//# DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe(). > +// > +// Copyright (C): 2016 Nicolai Stange > +// Options: --no-includes > +// > + > +virtual context > +virtual patch > +virtual org > +virtual report > + > + at dsa@ > +declarer name DEFINE_SIMPLE_ATTRIBUTE; > +identifier dsa_fops; > +expression dsa_get, dsa_set, dsa_fmt; > +position p; > +@@ > +DEFINE_SIMPLE_ATTRIBUTE at p(dsa_fops, dsa_get, dsa_set, dsa_fmt); > + > + at dcf@ > +expression name, mode, parent, data; > +identifier dsa.dsa_fops; > +@@ > +debugfs_create_file(name, mode, parent, data, &dsa_fops) > + > + > + at context_dsa depends on context && dcf@ > +declarer name DEFINE_DEBUGFS_ATTRIBUTE; > +identifier dsa.dsa_fops; > +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt; > +@@ > +* DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > + > + > + at patch_dcf depends on patch expression@ > +expression name, mode, parent, data; > +identifier dsa.dsa_fops; > +@@ > +- debugfs_create_file(name, mode, parent, data, &dsa_fops) > ++ debugfs_create_file_unsafe(name, mode, parent, data, &dsa_fops) > + > + at patch_dsa depends on patch_dcf && patch@ > +identifier dsa.dsa_fops; > +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt; > +@@ > +- DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > ++ DEFINE_DEBUGFS_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > + > + > + at script:python depends on org && dcf@ > +fops << dsa.dsa_fops; > +p << dsa.p; > +@@ > +msg="%s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops) > +coccilib.org.print_todo(p[0], msg) > + > + at script:python depends on report && dcf@ > +fops << dsa.dsa_fops; > +p << dsa.p; > +@@ > +msg="WARNING: %s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops) > +coccilib.report.print_report(p[0], msg) > -- > 2.7.4 > >