Re: [PATCH] coccinelle: api: add a checker for device nodes in devm_ memory

[Julia Lawall]

On Fri, 6 Nov 2026, Chad Aldreda wrote:

> A struct cdev, struct miscdevice, struct video_device or struct
> v4l2_device embedded in memory from devm_kzalloc() and friends is freed
> when the driver unbinds, but a file that user space holds open on the
> node outlives the unbind. Its later file operations are a use-after-free
> when they use that memory, and for a cdev or video_device so is its
> close(), where the core touches the object and the v4l2_device the
> video_device points to.
>
> Add a script that reports such allocations: by two rules when the
> allocation, as an assignment or a declaration initializer, and a
> registration of it or of an address taken inside it are written directly
> in one function, and by a third, type-based rule when the struct is
> defined in the same .c file and the node is a direct member of it. A
> registration in a helper function on a struct defined in a header is not
> seen, so cx231xx is reported only through its v4l2_device.
>
> On 7.4-rc1 it reports 101 sites under drivers/, 50 of which are found
> only through a v4l2_device. Not all are actionable: some drivers cannot
> be unbound, and some miscdevice drivers never use the allocation after
> open(); the header lists the false-positive classes and the misses.
>
> Tested with spatch 1.1.1, with make coccicheck MODE=report and MODE=org,
> and on small test files for each allocation form, a struct in a header,
> a registration in a helper, a node in an array member, a registered
> pointer to separate memory, a v4l2_device, an embedded node that is
> never registered and a kzalloc() control.

Thanks for this initiative.

Have you sent any patches based on the results?  I'm not an expert on the
problem being addressed, so it would be helpful to know that the semantic
patch has led to patches that have been accepted.

>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Chad Aldreda <ca@xxxxxxxxxxxxxx>
> ---
> The type rule uses --include-headers-for-types, which reports the same
> 101 sites on every run. With --include-headers it also sees structs
> defined in headers: run on one file at a time, that adds 22 sites on
> 7.4-rc1 (xilinx-vipp and ultrasoc-smb among them). Under --dir, as make
> coccicheck runs it, some are lost. spatch --include-headers --dir
> drivers/media/pci/intel/ipu6 reports nothing, even as a single process,
> while the same command on drivers/media/pci/intel/ipu6/ipu6-isys.c alone
> reports line 990. With six jobs the rest vary as well: five runs over
> drivers/ found 15 to 17 of the 22, a different set each time.
>
> So I kept the deterministic option. Is the --dir difference expected,
> and is there a better way to give the type rule header-defined structs?

The problem you have when you work on a subdirectory is that it doesn't
know where to find the header files.  It should work better with a -I
option.

>
>  .../coccinelle/api/devm_file_reachable.cocci  | 164 ++++++++++++++++++
>  1 file changed, 164 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..3847710798
> --- /dev/null
> +++ b/scripts/coccinelle/api/devm_file_reachable.cocci
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/// Find a struct cdev, struct miscdevice, struct video_device or struct
> +/// v4l2_device embedded in memory from the devm_ allocators.  The memory
> +/// is freed when the driver unbinds, but a file that is open on the node
> +/// outlives the unbind: for a cdev or a video_device the core touches the
> +/// object, and the v4l2_device the video_device points to, on every
> +/// close, and for a miscdevice the driver's file operations usually reach
> +/// it through file->private_data.  Allocate such state with kzalloc() and
> +/// free it on the last reference; see
> +/// Documentation/driver-api/driver-model/devres.rst.
> +///
> +/// The first rules need the allocation and a registration of it, or of an
> +/// address taken inside it, written directly in one function; an
> +/// allocation inside a condition or behind a cast or a wrapper function
> +/// is not seen by them. The type rule catches the rest, but with the
> +/// options below it sees only a struct defined in the .c file itself, not
> +/// in a header, and only a node that is a direct, non-array member of it,
> +/// not one nested in another member. So a registration in a helper
> +/// function, or through any alias of x such as vfd = &x->vdev, is missed
> +/// when the struct is in a header or the node is not such a member. Not
> +/// checked: a struct media_device in devm_ memory, which v4l2_release()
> +/// also reaches; a struct v4l2_subdev with a device node, whose close
> +/// reaches it; a static node whose file operations reach devm_ state
> +/// through a global or drvdata pointer; memory freed by devm_add_action()
> +/// rather than allocated by devm_k*(); a typedef'd struct; and a file or
> +/// function spatch cannot parse.
> +/// False positives: a driver that cannot be unbound (suppress_bind_attrs,
> +/// platform_driver_probe()) on a device that is never removed, a driver
> +/// whose remove() waits for the last close, a miscdevice driver whose file
> +/// operations do not use the allocation after open(), a registration of
> +/// an address that only uses the allocation to reach separate memory
> +/// (&x->ptr->misc, &table[x->idx]), a v4l2_device that no video_device
> +/// points to, and, for the type rule, a struct whose embedded node is not
> +/// registered from this allocation.  When an allocation has several
> +/// registrations, the report names the first one found.

Could you rewrite this in a much more concise format?  If you look at the
other semantic patches, you will see that they only have a line or two.
We don't need a description of what each rule does.


> +///
> +// Confidence: Moderate
> +// Options: --include-headers-for-types
> +
> +virtual org
> +virtual report
> +
> +@initialize:python@
> +@@
> +seen = set()
> +
> +@alloc depends on org || report exists@

Since you only support org and report, you don't really need depends on
org || report everywhere.

julia

> +expression x, e, e2;
> +identifier fn =~ "^devm_k[mzcr]";
> +position p, q;
> +@@
> +
> +x = fn@p(...);
> +... when != x = e
> +(
> +misc_register@q(\(x\|&<+... x ...+>\))
> +|
> +cdev_add@q(\(x\|&<+... x ...+>\), ...)
> +|
> +cdev_device_add@q(\(x\|&<+... x ...+>\), ...)
> +|
> +video_register_device@q(\(x\|&<+... x ...+>\), ...)
> +|
> +video_register_device_no_warn@q(\(x\|&<+... x ...+>\), ...)
> +|
> +__video_register_device@q(\(x\|&<+... x ...+>\), ...)
> +|
> +v4l2_device_register@q(e2, \(x\|&<+... x ...+>\))
> +)
> +
> +@allocdecl depends on org || report exists@
> +identifier x;
> +type T;
> +expression e, e2;
> +identifier fn =~ "^devm_k[mzcr]";
> +position p, q;
> +@@
> +
> +T *x = fn@p(...);
> +... when != x = e
> +(
> +misc_register@q(\(x\|&<+... x ...+>\))
> +|
> +cdev_add@q(\(x\|&<+... x ...+>\), ...)
> +|
> +cdev_device_add@q(\(x\|&<+... x ...+>\), ...)
> +|
> +video_register_device@q(\(x\|&<+... x ...+>\), ...)
> +|
> +video_register_device_no_warn@q(\(x\|&<+... x ...+>\), ...)
> +|
> +__video_register_device@q(\(x\|&<+... x ...+>\), ...)
> +|
> +v4l2_device_register@q(e2, \(x\|&<+... x ...+>\))
> +)
> +
> +@embeds depends on org || report@
> +identifier S, f;
> +type T = {struct cdev, struct miscdevice, struct video_device,
> +          struct v4l2_device};
> +@@
> +
> +struct S {
> +  ...
> +  T f;
> +  ...
> +};
> +
> +@bytype depends on org || report@
> +identifier embeds.S;
> +struct S *x;
> +identifier fn =~ "^devm_k[mzcr]";
> +position p;
> +@@
> +
> +x = fn@p(...)
> +
> +@script:python depends on org@
> +p << alloc.p;
> +q << alloc.q;
> +@@
> +if (p[0].file, p[0].line, p[0].column) not in seen:
> +    seen.add((p[0].file, p[0].line, p[0].column))
> +    coccilib.org.print_todo(p[0], "WARNING: devm_ memory registered at line %s as a device node or a v4l2_device; an open file outlives unbind" % q[0].line)
> +
> +@script:python depends on report@
> +p << alloc.p;
> +q << alloc.q;
> +@@
> +if (p[0].file, p[0].line, p[0].column) not in seen:
> +    seen.add((p[0].file, p[0].line, p[0].column))
> +    coccilib.report.print_report(p[0], "WARNING: devm_ memory registered at line %s as a device node or a v4l2_device; an open file outlives unbind" % q[0].line)
> +
> +@script:python depends on org@
> +p << allocdecl.p;
> +q << allocdecl.q;
> +@@
> +if (p[0].file, p[0].line, p[0].column) not in seen:
> +    seen.add((p[0].file, p[0].line, p[0].column))
> +    coccilib.org.print_todo(p[0], "WARNING: devm_ memory registered at line %s as a device node or a v4l2_device; an open file outlives unbind" % q[0].line)
> +
> +@script:python depends on report@
> +p << allocdecl.p;
> +q << allocdecl.q;
> +@@
> +if (p[0].file, p[0].line, p[0].column) not in seen:
> +    seen.add((p[0].file, p[0].line, p[0].column))
> +    coccilib.report.print_report(p[0], "WARNING: devm_ memory registered at line %s as a device node or a v4l2_device; an open file outlives unbind" % q[0].line)
> +
> +@script:python depends on org@
> +p << bytype.p;
> +S << embeds.S;
> +@@
> +if (p[0].file, p[0].line, p[0].column) not in seen:
> +    seen.add((p[0].file, p[0].line, p[0].column))
> +    coccilib.org.print_todo(p[0], "WARNING: devm_ allocation of struct %s, which embeds a cdev, miscdevice, video_device or v4l2_device" % S)
> +
> +@script:python depends on report@
> +p << bytype.p;
> +S << embeds.S;
> +@@
> +if (p[0].file, p[0].line, p[0].column) not in seen:
> +    seen.add((p[0].file, p[0].line, p[0].column))
> +    coccilib.report.print_report(p[0], "WARNING: devm_ allocation of struct %s, which embeds a cdev, miscdevice, video_device or v4l2_device" % S)
> --
> 2.43.0
>
>
>