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 <[email protected]> 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. > 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/[email protected]?part=1
