[PATCH] coccinelle: locks: Add must_hold.cocci

[Marco Elver]

Clang's context analysis treats lockdep_assert_held*() as an assumption
that the lock is held. A function that asserts instead of being
annotated with __must_hold() therefore passes the analysis, but its
callers are never checked.

Add a script that reports functions which assert on all paths that a
lock is held, where the lock can be named in a function attribute (it is
reachable from a parameter or is a global lock), and which lack a
__must_hold*() or __releases*() annotation.

For example, on v7.4-rc2:

  kernel/locking/rwsem.c:396:1-20: rwsem_del_waiter() asserts &sem->wait_lock is held but lacks __must_hold(&sem->wait_lock)
  kernel/locking/rwsem.c:437:1-20: rwsem_mark_wake() asserts &sem->wait_lock is held but lacks __must_hold(&sem->wait_lock)
  kernel/futex/pi.c:919:1-20: fixup_pi_state_owner() asserts q->lock_ptr is held but lacks __must_hold(q->lock_ptr)
  drivers/nvme/host/fabrics.c:650:1-20: nvmf_lookup_transport() asserts &nvmf_transports_rwsem is held but lacks __must_hold(&nvmf_transports_rwsem)

Limitations:

 - Annotations on a prototype in another file are not seen; non-static
   functions are reported with "check prototype".
 - Annotations are detected textually and are not matched against the
   asserted lock.
 - Functions that require a lock without asserting it are not found.

Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
---
 scripts/coccinelle/locks/must_hold.cocci | 100 +++++++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 scripts/coccinelle/locks/must_hold.cocci

diff --git a/scripts/coccinelle/locks/must_hold.cocci b/scripts/coccinelle/locks/must_hold.cocci
new file mode 100644
index 000000000000..ba45544afc46
--- /dev/null
+++ b/scripts/coccinelle/locks/must_hold.cocci
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Find functions that unconditionally assert with lockdep that a lock is
+/// held, but lack the corresponding __must_hold() annotation. For such
+/// functions the compiler's context analysis trusts the assertion instead of
+/// checking the callers.
+///
+/// Only locks that can be named in a function attribute are reported, i.e.
+/// those reachable from a parameter or a global lock (&lock). Assertions on
+/// locks reached through local aliases are skipped.
+//
+// Confidence: Moderate
+// Comments: Annotations on a prototype in another file are not seen; such
+//	     functions are reported with "check prototype".
+// Options: --no-includes --include-headers
+
+virtual report
+virtual org
+
+@initialize:python@
+@@
+
+import re
+
+def squash(s):
+	return re.sub(r'\s+', '', s)
+
+def missing_must_hold(f, ps, E, pf, p):
+	e = squash(E)
+	m = re.match(r'[&(]*(\w+)', e)
+	if not m:
+		return None
+	params = re.findall(r'(\w+)\s*(?:\[[^\]]*\]\s*)*(?:,|$)', str(ps))
+	if m.group(1) not in params and not re.fullmatch(r'&\w+', e):
+		return None	# Local alias: cannot be named in an attribute.
+
+	with open(pf[0].file, errors='replace') as fp:
+		lines = fp.readlines()
+	first, last = int(pf[0].line), int(p[0].line)
+
+	# Attributes between the declarator and the assertion.
+	hdr = ''.join(lines[first - 1:last - 1])
+	if re.search(r'__(must_hold|must_hold_shared|releases|releases_shared|'
+		     r'context_unsafe|no_context_analysis)\b', hdr):
+		return None
+
+	# Attributes on a prototype in the same file.
+	for d in re.finditer(r'\b%s\s*\([^;{}]*\)([^;{}]*);' % re.escape(f),
+			     ''.join(lines)):
+		if '__must_hold' in d.group(1):
+			return None
+
+	static = 'static' in ''.join(lines[max(first - 3, 0):first])
+	shared = 'lockdep_assert_held_read' in lines[last - 1]
+	return "%s() asserts %s is held but lacks __must_hold%s(%s)%s" % (
+		f, e, "_shared" if shared else "", e,
+		"" if static else "; check prototype")
+
+@r@
+identifier f;
+parameter list ps;
+expression E;
+position pf, p;
+@@
+
+f@pf(ps)
+{
+	...
+(
+	lockdep_assert_held@p(E);
+|
+	lockdep_assert_held_write@p(E);
+|
+	lockdep_assert_held_read@p(E);
+)
+	...
+}
+
+@script:python depends on report@
+f << r.f;
+ps << r.ps;
+E << r.E;
+pf << r.pf;
+p << r.p;
+@@
+
+msg = missing_must_hold(f, ps, E, pf, p)
+if msg:
+	coccilib.report.print_report(p[0], msg)
+
+@script:python depends on org@
+f << r.f;
+ps << r.ps;
+E << r.E;
+pf << r.pf;
+p << r.p;
+@@
+
+msg = missing_must_hold(f, ps, E, pf, p)
+if msg:
+	coccilib.org.print_todo(p[0], msg)
-- 
2.56.0.542.g3ef89b7020-goog


--kN9fNHIrqDuj/ERC--