From: Julia Lawall <julia.lawall@lip6.fr>
To: Nicolai Stange <nicstange@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Jonathan Corbet <corbet@lwn.net>, Jan Kara <jack@suse.com>,
Andrew Morton <akpm@linux-foundation.org>,
Julia Lawall <Julia.Lawall@lip6.fr>,
Gilles Muller <Gilles.Muller@lip6.fr>,
Nicolas Palix <nicolas.palix@imag.fr>,
Michal Marek <mmarek@suse.com>,
linux-kernel@vger.kernel.org, cocci@systeme.lip6.fr
Subject: Re: [PATCH v6 4/8] debugfs, coccinelle: check for obsolete DEFINE_SIMPLE_ATTRIBUTE() usage
Date: Tue, 22 Mar 2016 14:18:22 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.10.1603221417430.3301@hadrien> (raw)
In-Reply-To: <1458652280-19785-5-git-send-email-nicstange@gmail.com>
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 <nicstange@gmail.com>
In terms of the structure of the semantic patch:
Acked-by: Julia Lawall <Julia.Lawall@lip6.fr>
> ---
> .../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
>
>
next prev parent reply other threads:[~2016-03-22 13:18 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-22 13:11 [PATCH v6 0/8] fix debugfs file removal races Nicolai Stange
2016-03-22 13:11 ` [PATCH v6 1/8] debugfs: prevent access to possibly dead file_operations at file open Nicolai Stange
2016-03-22 13:11 ` [PATCH v6 2/8] debugfs: prevent access to removed files' private data Nicolai Stange
2016-05-18 14:48 ` Sasha Levin
2016-05-18 15:01 ` Nicolai Stange
2016-05-18 15:18 ` Sasha Levin
2016-05-18 16:05 ` Greg Kroah-Hartman
2016-05-20 16:57 ` Sasha Levin
2016-05-21 17:57 ` Nicolai Stange
2016-05-22 13:28 ` Nicolai Stange
2016-05-18 16:32 ` Nicolai Stange
2016-05-20 16:55 ` Sasha Levin
2016-03-22 13:11 ` [PATCH v6 3/8] debugfs: add support for self-protecting attribute file fops Nicolai Stange
2016-03-22 13:11 ` [PATCH v6 4/8] debugfs, coccinelle: check for obsolete DEFINE_SIMPLE_ATTRIBUTE() usage Nicolai Stange
2016-03-22 13:18 ` Julia Lawall [this message]
2016-03-22 13:11 ` [PATCH v6 5/8] debugfs: unproxify integer attribute files Nicolai Stange
2016-03-22 13:11 ` [PATCH v6 6/8] debugfs: unproxify files created through debugfs_create_bool() Nicolai Stange
2016-03-22 13:11 ` [PATCH v6 7/8] debugfs: unproxify files created through debugfs_create_blob() Nicolai Stange
2016-03-22 13:11 ` [PATCH v6 8/8] debugfs: unproxify files created through debugfs_create_u32_array() Nicolai Stange
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.10.1603221417430.3301@hadrien \
--to=julia.lawall@lip6.fr \
--cc=Gilles.Muller@lip6.fr \
--cc=akpm@linux-foundation.org \
--cc=cocci@systeme.lip6.fr \
--cc=corbet@lwn.net \
--cc=gregkh@linuxfoundation.org \
--cc=jack@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=mmarek@suse.com \
--cc=nicolas.palix@imag.fr \
--cc=nicstange@gmail.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=viro@zeniv.linux.org.uk \
/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
all inboxes | Powered by JetHome®