linux-kernel - Re: [PATCH v6 4/8] debugfs, coccinelle: check for obsolete DEFINE_SIMPLE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1603221417430.3301@hadrien>
Date:	Tue, 22 Mar 2016 14:18:22 +0100 (CET)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	Nicolai Stange <nicstange@...il.com>
cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Jonathan Corbet <corbet@....net>, Jan Kara <jack@...e.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Julia Lawall <Julia.Lawall@...6.fr>,
	Gilles Muller <Gilles.Muller@...6.fr>,
	Nicolas Palix <nicolas.palix@...g.fr>,
	Michal Marek <mmarek@...e.com>, linux-kernel@...r.kernel.org,
	cocci@...teme.lip6.fr
Subject: Re: [PATCH v6 4/8] debugfs, coccinelle: check for obsolete
 DEFINE_SIMPLE_ATTRIBUTE() usage



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@...il.com>

In terms of the structure of the semantic patch:
Acked-by: Julia Lawall <Julia.Lawall@...6.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)
> +
> +
> +@...text_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);
> +
> +
> +@...ch_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)
> +
> +@...ch_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);
> +
> +
> +@...ipt: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)
> +
> +@...ipt: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
>
>