| 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
| ||
|
Message-Id: <20210517193810.2793785-1-Julia.Lawall@inria.fr>
Date: Mon, 17 May 2021 21:38:10 +0200
From: Julia Lawall <Julia.Lawall@...ia.fr>
To: Julia Lawall <Julia.Lawall@...ia.fr>,
Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc: kernel-janitors@...r.kernel.org,
Gilles Muller <Gilles.Muller@...ia.fr>,
Nicolas Palix <nicolas.palix@...g.fr>,
Michal Marek <michal.lkml@...kovi.net>, cocci@...teme.lip6.fr,
linux-kernel@...r.kernel.org,
"Rafael J . Wysocki" <rafael@...nel.org>,
Johan Hovold <johan@...nel.org>,
Zhang Qilong <zhangqilong3@...wei.com>,
Jakub Kicinski <kuba@...nel.org>,
Jonathan Cameron <jic23@...nel.org>,
"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>
Subject: [PATCH v6] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
pm_runtime_get_sync keeps a reference count on failure, which can lead
to leaks. pm_runtime_resume_and_get drops the reference count in the
failure case. This rule very conservatively follows the definition of
pm_runtime_resume_and_get to address the cases where the reference
count is unlikely to be needed in the failure case. Specifically, the
change is only done when pm_runtime_get_sync is followed immediately
by an if and when the branch of the if is immediately a call to
pm_runtime_put_noidle (like in the definition of
pm_runtime_resume_and_get) or something that is likely a print
statement followed by a pm_runtime_put_noidle call.
The change is furthermore only done when the ret variable is only used
in a ret < 0 test and possibly in the associated branch. No change is
made if ret is used in the else branch (the rule bad) or after the if.
This is because pm_runtime_resume_and_get only returns 0 (success) or
a negative value (failure), while pm_runtime_get_sync may also return
1 in the success case. Thus more attention is required to make the
change in a case where a 1 value might be observed.
The patch case appears somewhat more complicated than the others,
because it aolso deals with the cases where {}s need to be removed.
pm_runtime_resume_and_get was introduced in
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
deal with usage counter")
Signed-off-by: Julia Lawall <Julia.Lawall@...ia.fr>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
v6: don't touch code that reuses the value of ret, as suggested by Mauro Carvalho Chehab
v5: print a message with the new function name, as suggested by Markus Elfring
v4: s/pm_runtime_resume_and_get/pm_runtime_put_noidle/ as noted by John Hovold
v3: add the people who signed off on commit dd8088d5a896, expand the log message
v2: better keyword
scripts/coccinelle/api/pm_runtime_resume_and_get.cocci | 185 +++++++++++++++++
1 file changed, 185 insertions(+)
diff --git a/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
new file mode 100644
index 000000000000..b5abb3783d3d
--- /dev/null
+++ b/scripts/coccinelle/api/pm_runtime_resume_and_get.cocci
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use pm_runtime_resume_and_get.
+/// pm_runtime_get_sync keeps a reference count on failure,
+/// which can lead to leaks. pm_runtime_resume_and_get
+/// drops the reference count in the failure case.
+/// This rule addresses the cases where the reference count
+/// is unlikely to be needed in the failure case.
+///
+// Confidence: High
+// Copyright: (C) 2021 Julia Lawall, Inria
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --include-headers --no-includes
+// Keywords: pm_runtime_get_sync
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@bad exists@
+expression ret;
+statement S1;
+position p;
+@@
+
+ ret = pm_runtime_get_sync(...);
+ if@p (ret < 0) S1
+ else {<+... ret ...+>}
+
+@r0 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+- if (ret < 0)
+- pm_runtime_put_noidle(e);
+ ... when != ret
+? ret = e1
+
+@r1 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+statement S1,S2;
+position p != bad.p;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0)
+- {
+- pm_runtime_put_noidle(e);
+ S1
+- }
+ else S2
+ ... when != ret
+? ret = e1
+
+@r2 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+statement S;
+position p != bad.p;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0) {
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+ ... when != ret
+? ret = e1
+
+@r3 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+identifier f;
+constant char[] c;
+position p != bad.p;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0)
+- {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+- }
+ else S
+ ... when != ret
+? ret = e1
+
+@r4 depends on patch && !context && !org && !report@
+expression ret,e,e1;
+identifier f;
+constant char[] c;
+position p != bad.p;
+statement S;
+@@
+
+- ret = pm_runtime_get_sync(e);
++ ret = pm_runtime_resume_and_get(e);
+ if@p (ret < 0) {
+ f(...,c,...);
+- pm_runtime_put_noidle(e);
+ ...
+ } else S
+ ... when != ret
+? ret = e1
+
+// ----------------------------------------------------------------------------
+
+@...context depends on !patch && (context || org || report)@
+statement S;
+expression e, ret, e1;
+position j0, j1;
+position p != bad.p;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if@p (ret < 0) {
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+ ... when != ret
+ when forall
+? ret = e1
+
+@...context depends on !patch && (context || org || report)@
+identifier f;
+statement S;
+constant char []c;
+expression e, ret, e1;
+position j0, j1;
+position p != bad.p;
+@@
+
+* ret@j0 = pm_runtime_get_sync(e);
+ if@p (ret < 0) {
+ f(...,c,...);
+* pm_runtime_put_noidle@j1(e);
+ ...
+ } else S
+ ... when != ret
+ when forall
+? ret = e1
+
+// ----------------------------------------------------------------------------
+
+@...ipt:python r2_org depends on org@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_resume_and_get"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+@...ipt:python r3_org depends on org@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_resume_and_get"
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+
+// ----------------------------------------------------------------------------
+
+@...ipt:python r2_report depends on report@
+j0 << r2_context.j0;
+j1 << r2_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_resume_and_get on line %s." % (j0[0].line)
+coccilib.report.print_report(j0[0], msg)
+
+@...ipt:python r3_report depends on report@
+j0 << r3_context.j0;
+j1 << r3_context.j1;
+@@
+
+msg = "WARNING: opportunity for pm_runtime_resume_and_get on %s." % (j0[0].line)
+coccilib.report.print_report(j0[0], msg)
+
Powered by blists - more mailing lists