mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Chad Aldreda <ca@modelmiser.com>
To: Julia Lawall <Julia.Lawall@inria.fr>,
	Nicolas Palix <nicolas.palix@imag.fr>
Cc: cocci@inria.fr, linux-kernel@vger.kernel.org
Subject: [PATCH v2] coccinelle: api: add a checker for device nodes in devm memory
Date: Tue, 10 Nov 2026 17:56:12 +0000	[thread overview]
Message-ID: <20261110175607.1617450-2-ca@modelmiser.com> (raw)
In-Reply-To: <20261106193517.3267013-2-ca@modelmiser.com>

A struct cdev, miscdevice, video_device or v4l2_device embedded in
memory from devm_kzalloc() and friends is freed when the driver unbinds
(Documentation/driver-api/driver-model/devres.rst), but a file that user
space holds open on the node outlives the unbind. Its later file
operations use the freed memory, and for two kinds of node so does the
core on close(). For a cdev, __fput() calls cdev_put() on it after the
driver's release(). For a video_device, v4l2_release() reads it and the
v4l2_device it points to, then drops a reference on it; at the last
reference v4l2_device_release() follows it to the v4l2_device again.
For a miscdevice, nothing on close() touches the miscdevice except the
driver's release().

Report such allocations in two ways: when the allocation and a
registration whose argument uses it (x, &x->f, ...) are in one function;
and by type, when the allocated struct embeds the node directly and is
defined in the .c file or in a header it includes with #include "..."
(spatch's default). A node registered through a pointer copied out of
the allocation first is found only by the type rule; one nested in
another member or in an array only by the first rule.

With Coccinelle 1.3.3, on 7.4-rc1 it reports 124 sites under drivers/,
47 of them only through a v4l2_device. Known false positives: drivers
that cannot be unbound while a file is open short of removing the
device, such as open-dice (platform_driver_probe() sets
suppress_bind_attrs, and the open file pins the module); nodes no file
can be opened on, such as the miscdevice in dw-xdata-pcie, which has no
file_operations, or a v4l2_device that no device node points to; for the
type rule, an allocation whose node is never registered; and a
registration of memory only reached through the allocation, such as
misc_register(x->ptr). Coccinelle 1.1.1 found 116 and 115 of the 124 in
two runs, and nothing else: it loses some structs defined in headers
when run with --dir, and cannot parse one probe function.

Fixes sent for sites it reports:

  misc: nsm: https://lore.kernel.org/r/20261107114433.3789830-2-ca@modelmiser.com
  ptp: vmclock: https://lore.kernel.org/r/20261107150344.3946529-2-ca@modelmiser.com
  platform/chrome: cros_hps_i2c: https://lore.kernel.org/r/20261110123230.1251547-2-ca@modelmiser.com
  platform/x86: dell-smo8800: https://lore.kernel.org/r/20261107204950.88952-2-ca@modelmiser.com
  ACPI: pfr_update: https://lore.kernel.org/r/20261108233900.4043941-4-ca@modelmiser.com
  ACPI: pfr_telemetry: https://lore.kernel.org/r/20261110144436.1529000-2-ca@modelmiser.com

The script supports the context, org and report modes. It has no patch
mode: the fix is a lifetime change (a kref, or a reference held by the
open file) that differs per driver.

The script and this message were written with an AI assistant (Claude)
and checked by hand. The six sites with fixes linked above were
reproduced in QEMU under KASAN before each fix was sent; the other sites
were not tested.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Chad Aldreda <ca@modelmiser.com>
---
v2, for Julia Lawall's points:
- Accepted fixes: links in the message; their status is at the end of
  this note.
- -I: two things lost ipu6 line 990 for us, and -I fixes neither.
  Coccinelle 1.1.1 misses it with --dir, with or without -I; and v1's
  Options line (--include-headers-for-types), which make coccicheck
  applies and a bare spatch run ignores, makes 1.3.3 miss it too. v2
  drops that line; with 1.3.3 (git 11b93adb6), spatch --dir drivers and
  make coccicheck M=drivers, which passes -I, give the same 124 sites,
  990 among them.
- Header cut to two lines, and depends on org || report dropped.

v2, for Markus Elfring's points: the two allocation rules are one rule
with a disjunction (x is now an identifier, so x->f = devm_k*() is
reported by the type rule); the argument pattern is <+... x ...+>;
context mode added; one report helper; f-strings; "devm" without the
underscore; the close() paths per node type are in the message. With the
same Coccinelle and options, the report over drivers/ is the same line
for line before and after these changes.

Counts are for Coccinelle 1.3.3; v1's used 1.1.1. The 47 replaces v1's
50, which was measured with v1's script, 1.1.1 and the old Options line.

v1: https://lore.kernel.org/r/20261106193517.3267013-2-ca@modelmiser.com

On your question about accepted fixes: Rafael Wysocki applied the
pfr_update fix to linux-pm (bleeding-edge) for 7.5, as dacb7cef6f32.
The cros_hps_i2c fix is at v2 after Tzung-Bi Shih's review, and
pfr_telemetry at v2 after Rafael asked me to address an automated
review of it; nsm, vmclock and dell-smo8800 have had no reply yet.

 .../coccinelle/api/devm_file_reachable.cocci  | 114 ++++++++++++++++++
 1 file changed, 114 insertions(+)
 create mode 100644 scripts/coccinelle/api/devm_file_reachable.cocci

diff --git a/scripts/coccinelle/api/devm_file_reachable.cocci b/scripts/coccinelle/api/devm_file_reachable.cocci
new file mode 100644
index 0000000000..09d139eb6d
--- /dev/null
+++ b/scripts/coccinelle/api/devm_file_reachable.cocci
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Find a cdev, miscdevice, video_device or v4l2_device in devm memory,
+/// which a file still open after unbind can use.
+///
+// Confidence: Moderate
+// Comments: Drivers that cannot be unbound while a file is open, and nodes
+// without file operations, are false positives.
+
+virtual context
+virtual org
+virtual report
+
+@initialize:python@
+@@
+seen = set()
+
+def warn(print_fn, p, msg):
+    key = (p.file, p.line, p.column)
+    if key not in seen:
+        seen.add(key)
+        print_fn(p, f"WARNING: {msg}")
+
+@alloc exists@
+identifier x;
+type T;
+expression e, e2;
+identifier fn =~ "^devm_k[mzcr]";
+position p, q;
+@@
+
+(
+x = fn@p(...);
+|
+T *x = fn@p(...);
+)
+... when != x = e
+(
+misc_register@q(<+... x ...+>)
+|
+video_register_device@q(<+... x ...+>, ...)
+|
+v4l2_device_register@q(e2, <+... x ...+>)
+|
+cdev_device_add@q(<+... x ...+>, ...)
+|
+cdev_add@q(<+... x ...+>, ...)
+|
+__video_register_device@q(<+... x ...+>, ...)
+|
+video_register_device_no_warn@q(<+... x ...+>, ...)
+)
+
+@embeds@
+identifier S, f;
+type T = {struct cdev, struct miscdevice, struct video_device,
+          struct v4l2_device};
+@@
+
+struct S {
+  ...
+  T f;
+  ...
+};
+
+@bytype@
+identifier embeds.S;
+struct S *x;
+identifier fn =~ "^devm_k[mzcr]";
+position p;
+@@
+
+x = fn@p(...)
+
+@depends on context@
+identifier alloc.fn;
+position alloc.p;
+@@
+
+* fn@p(...)
+
+@depends on context@
+identifier bytype.fn;
+position bytype.p;
+@@
+
+* fn@p(...)
+
+@script:python depends on org@
+p << alloc.p;
+q << alloc.q;
+@@
+warn(coccilib.org.print_todo, p[0],
+     f"devm memory registered at line {q[0].line} as a device node or a v4l2_device")
+
+@script:python depends on report@
+p << alloc.p;
+q << alloc.q;
+@@
+warn(coccilib.report.print_report, p[0],
+     f"devm memory registered at line {q[0].line} as a device node or a v4l2_device")
+
+@script:python depends on org@
+p << bytype.p;
+S << embeds.S;
+@@
+warn(coccilib.org.print_todo, p[0],
+     f"devm allocation of struct {S}, which embeds a device node or a v4l2_device")
+
+@script:python depends on report@
+p << bytype.p;
+S << embeds.S;
+@@
+warn(coccilib.report.print_report, p[0],
+     f"devm allocation of struct {S}, which embeds a device node or a v4l2_device")
-- 
2.43.0



      parent reply	other threads:[~2026-11-10 17:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-06 19:35 [PATCH] coccinelle: api: add a checker for device nodes in devm_ memory Chad Aldreda
2026-11-06 20:20 ` Julia Lawall
2026-11-06 21:07   ` Chad Aldreda
2026-11-07  9:10   ` [cocci] " Markus Elfring
2026-11-07 10:55 ` Markus Elfring
2026-11-07 13:40 ` Markus Elfring
2026-11-07 14:37 ` Markus Elfring
2026-11-08 20:10 ` Julia Lawall
2026-11-08 20:36   ` Chad Aldreda
2026-11-10 17:56 ` Chad Aldreda [this message]

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=20261110175607.1617450-2-ca@modelmiser.com \
    --to=ca@modelmiser.com \
    --cc=Julia.Lawall@inria.fr \
    --cc=cocci@inria.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolas.palix@imag.fr \
    /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®