debugfs, coccinelle: check for ->vm_ops setting ->mmap() implementations

From: Nicolai Stange
Date: Tue Aug 02 2016 - 12:33:59 EST


While debugfs files may provide their custom ->mmap() implementations now,
they must not set the vm_area_struct's ->vm_ops for the following reason:
its methods can be invoked at any time by the MM subsystem and thus, they
are subject to file removal races.

Further explanation: for the struct file_operations, this issue has been
resolved by installing some protecting proxies from the debugfs core.
However, we certainly don't want to do this for the vm_operations_struct:
first, there isn't any real demand currently and second, we would probably
have to SIGSEGV userspace under certain conditions (->fault() invoked on
stale file).

Thus, don't support custom ->vm_ops for debugfs files. Introduce a
Coccinelle script checking for this forbidden usage pattern: moan if a
struct file_operations with a ->mmap() writing to vma->vm_ops is handed
to debugfs_create_file().

Signed-off-by: Nicolai Stange <nicstange@xxxxxxxxx>

diff --git a/scripts/coccinelle/api/debugfs/debugfs_mmap_vm_ops.cocci b/scripts/coccinelle/api/debugfs/debugfs_mmap_vm_ops.cocci
new file mode 100644
index 0000000..c53286b
--- /dev/null
+++ b/scripts/coccinelle/api/debugfs/debugfs_mmap_vm_ops.cocci
@@ -0,0 +1,66 @@
+/// Don't set vma->vm_ops from a debugfs file's ->mmap() implementation.
+///
+//# Rationale: While a debugfs file's struct file_operations is
+//# protected against file removal races through a proxy wrapper
+//# automatically provided by the debugfs core, anything installed at
+//# vma->vm_ops from ->mmap() isn't: the mm subsystem may and will
+//# invoke its members at any time.
+//
+// Copyright (C): 2016 Nicolai Stange
+// Options: --no-includes
+//
+
+virtual context
+virtual report
+virtual org
+
+@unsupp_mmap_impl@
+identifier mmap_impl;
+identifier filp, vma;
+expression e;
+position p;
+@@
+
+int mmap_impl(struct file *filp, struct vm_area_struct *vma)
+{
+ ...
+ vma->vm_ops@p = e
+ ...
+}
+
+@unsupp_fops@
+identifier fops;
+identifier unsupp_mmap_impl.mmap_impl;
+@@
+struct file_operations fops = {
+ .mmap = mmap_impl,
+};
+
+@unsupp_fops_usage@
+expression name, mode, parent, data;
+identifier unsupp_fops.fops;
+@@
+debugfs_create_file(name, mode, parent, data, &fops)
+
+
+@context_unsupp_mmap_impl depends on context && unsupp_fops_usage@
+identifier unsupp_mmap_impl.mmap_impl;
+identifier unsupp_mmap_impl.filp, unsupp_mmap_impl.vma;
+expression unsupp_mmap_impl.e;
+@@
+int mmap_impl(struct file *filp, struct vm_area_struct *vma)
+{
+ ...
+* vma->vm_ops = e
+ ...
+}
+
+@script:python depends on org && unsupp_fops_usage@
+p << unsupp_mmap_impl.p;
+@@
+coccilib.org.print_todo(p[0], "a debugfs file's ->mmap() must not set ->vm_ops")
+
+@script:python depends on report && unsupp_fops_usage@
+p << unsupp_mmap_impl.p;
+@@
+coccilib.report.print_report(p[0], "a debugfs file's ->mmap() must not set ->vm_ops")
--
2.9.2

--8<---------------cut here---------------end--------------->8---

Thanks,

Nicolai