Thread (6 messages) 6 messages, 3 authors, 18d ago

Re: [PATCH v3] rust: add procedural macro for declaring configfs attributes

From: sashiko-bot@kernel.org
Date: 2026-06-12 13:41:01

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent execution of the generated `configfs_attrs!` block causes a data race on a shared `static` array.
--

commit d9a7547ab4c8957df6d5328eba0e3a712923192f
Author: Malte Wechter [off-list ref]

rust: add procedural macro for declaring configfs attributes

This commit replaces the old declarative configfs_attrs macro with a new
procedural macro to improve readability and maintainability.
quoted hunk ↗ jump to hunk
diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
new file mode 100644
index 0000000000000..81037bc38188b
--- /dev/null
+++ b/rust/macros/configfs_attrs.rs
[ ... ]
+    let mut attrs = Vec::new();
+    for (attr_idx, (name, id)) in cfs_attrs.attributes.iter().enumerate() {
+        let name_with_attr = format_ident!("{}_ATTR_{}", name.to_string().to_uppercase(), attr_idx);
[ ... ]
+        attrs.push(quote! {
+        static #name_with_attr: kernel::configfs::Attribute<#id, #data_ty, #data_ty> =
+            // SAFETY: We are expanding `configfs_attrs`.
+            unsafe {
+              kernel::configfs::Attribute::new(kernel::c_str!(::core::stringify!(#name)))
+            };
+
+          // SAFETY: By design of this macro, the name of the variable we
+          // invoke the `add` method on below, is not visible outside of
+          // the macro expansion. The macro does not operate concurrently
+          // on this variable, and thus we have exclusive access to the
+          // variable.
+          unsafe { #data_attr_ident.add::<#attr_idx, #id, _>(&#name_with_attr) }
[Severity: High]
This is a pre-existing issue, but does this generated code cause a data race 
on the static attribute list array?

Because the generated attribute list is a static variable, its state is shared
across all invocations of the block. If the macro is used inside a callback
like make_group, every time a new directory is created via mkdir, the add
method is executed at runtime, mutating the globally shared UnsafeCell-backed
array via a raw pointer.

While make_group itself is serialized per-subsystem by su_mutex, concurrent
VFS reads on attributes of previously created directories do not hold this
mutex. Could make_group mutate the static array at the exact same time a
concurrent attribute read accesses it?

Could this array be fully initialized statically at compile time instead?
+        });
+    }
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-configfs-syn-v3-1-3292fbc5cc32@gmail.com?part=1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help