diff --git a/MAINTAINERS b/MAINTAINERS index 7895e77..63a8878 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6603,6 +6603,7 @@ S: Supported F: fs/configfs/ F: include/linux/configfs.h F: samples/configfs/ +F: tools/testing/selftests/filesystems/configfs/ CONFIGFS [RUST] M: Andreas Hindborg diff --git a/drivers/acpi/acpi_configfs.c b/drivers/acpi/acpi_configfs.c index 12ffec7..6071699 100644 --- a/drivers/acpi/acpi_configfs.c +++ b/drivers/acpi/acpi_configfs.c @@ -91,7 +91,7 @@ static ssize_t acpi_table_aml_read(struct config_item *cfg, CONFIGFS_BIN_ATTR(acpi_table_, aml, NULL, MAX_ACPI_TABLE_SIZE); -static struct configfs_bin_attribute *acpi_table_bin_attrs[] = { +static const struct configfs_bin_attribute *const acpi_table_bin_attrs[] = { &acpi_table_attr_aml, NULL, }; diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c index 052cce9..842c180 100644 --- a/drivers/gpu/drm/xe/xe_configfs.c +++ b/drivers/gpu/drm/xe/xe_configfs.c @@ -886,7 +886,7 @@ static struct configfs_item_operations xe_config_device_ops = { }; static bool xe_config_device_is_visible(struct config_item *item, - struct configfs_attribute *attr, int n) + const struct configfs_attribute *attr, int n) { struct xe_config_group_device *dev = to_xe_config_group_device(item); @@ -981,7 +981,7 @@ static struct configfs_attribute *xe_config_sriov_attrs[] = { }; static bool xe_config_sriov_is_visible(struct config_item *item, - struct configfs_attribute *attr, int n) + const struct configfs_attribute *attr, int n) { struct xe_config_group_device *dev = to_xe_config_group_device(item->ci_parent); diff --git a/drivers/virt/coco/guest/report.c b/drivers/virt/coco/guest/report.c index b254a14..ad400fb 100644 --- a/drivers/virt/coco/guest/report.c +++ b/drivers/virt/coco/guest/report.c @@ -356,7 +356,7 @@ static struct configfs_attribute *tsm_report_attrs[] = { NULL, }; -static struct configfs_bin_attribute *tsm_report_bin_attrs[] = { +static const struct configfs_bin_attribute *const tsm_report_bin_attrs[] = { [TSM_REPORT_INBLOB] = &tsm_report_attr_inblob, [TSM_REPORT_OUTBLOB] = &tsm_report_attr_outblob, [TSM_REPORT_AUXBLOB] = &tsm_report_attr_auxblob, @@ -381,7 +381,7 @@ static struct configfs_item_operations tsm_report_item_ops = { }; static bool tsm_report_is_visible(struct config_item *item, - struct configfs_attribute *attr, int n) + const struct configfs_attribute *attr, int n) { guard(rwsem_read)(&tsm_rwsem); if (!provider.ops) @@ -394,7 +394,7 @@ static bool tsm_report_is_visible(struct config_item *item, } static bool tsm_report_is_bin_visible(struct config_item *item, - struct configfs_bin_attribute *attr, int n) + const struct configfs_bin_attribute *attr, int n) { guard(rwsem_read)(&tsm_rwsem); if (!provider.ops) diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h index 860f1b6..5f627e5 100644 --- a/fs/configfs/configfs_internal.h +++ b/fs/configfs/configfs_internal.h @@ -104,17 +104,17 @@ static inline struct config_item * to_item(struct dentry * dentry) return ((struct config_item *) sd->s_element); } -static inline struct configfs_attribute * to_attr(struct dentry * dentry) +static inline const struct configfs_attribute * to_attr(struct dentry * dentry) { struct configfs_dirent * sd = dentry->d_fsdata; - return ((struct configfs_attribute *) sd->s_element); + return ((const struct configfs_attribute *) sd->s_element); } -static inline struct configfs_bin_attribute *to_bin_attr(struct dentry *dentry) +static inline const struct configfs_bin_attribute *to_bin_attr(struct dentry *dentry) { - struct configfs_attribute *attr = to_attr(dentry); + const struct configfs_attribute *attr = to_attr(dentry); - return container_of(attr, struct configfs_bin_attribute, cb_attr); + return container_of_const(attr, struct configfs_bin_attribute, cb_attr); } static inline struct config_item *configfs_get_config_item(struct dentry *dentry) diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index f85b945..0c80fee 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -470,7 +470,7 @@ static struct dentry * configfs_lookup(struct inode *dir, */ if ((sd->s_type & CONFIGFS_NOT_PINNED) && !strcmp(configfs_get_name(sd), dentry->d_name.name)) { - struct configfs_attribute *attr = sd->s_element; + const struct configfs_attribute *attr = sd->s_element; umode_t mode = (attr->ca_mode & S_IALLUGO) | S_IFREG; dentry->d_fsdata = configfs_get(sd); @@ -631,8 +631,8 @@ static int populate_attrs(struct config_item *item) { const struct config_item_type *t = item->ci_type; const struct configfs_group_operations *ops; - struct configfs_attribute *attr; - struct configfs_bin_attribute *bin_attr; + const struct configfs_attribute *attr; + const struct configfs_bin_attribute *bin_attr; int error = 0; int i; @@ -641,8 +641,8 @@ static int populate_attrs(struct config_item *item) ops = t->ct_group_ops; - if (t->ct_attrs) { - for (i = 0; (attr = t->ct_attrs[i]) != NULL; i++) { + if (t->ct_attrs_const) { + for (i = 0; (attr = t->ct_attrs_const[i]) != NULL; i++) { if (ops && ops->is_visible && !ops->is_visible(item, attr, i)) continue; diff --git a/fs/configfs/file.c b/fs/configfs/file.c index a48cece..6460b00 100644 --- a/fs/configfs/file.c +++ b/fs/configfs/file.c @@ -41,8 +41,8 @@ struct configfs_buffer { struct config_item *item; struct module *owner; union { - struct configfs_attribute *attr; - struct configfs_bin_attribute *bin_attr; + const struct configfs_attribute *attr; + const struct configfs_bin_attribute *bin_attr; }; }; @@ -291,7 +291,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type { struct dentry *dentry = file->f_path.dentry; struct configfs_fragment *frag = to_frag(file); - struct configfs_attribute *attr; + const struct configfs_attribute *attr; struct configfs_buffer *buffer; int error; diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c index 6ba125d..c92a052 100644 --- a/fs/configfs/inode.c +++ b/fs/configfs/inode.c @@ -178,7 +178,7 @@ struct inode *configfs_create(struct dentry *dentry, umode_t mode) */ const unsigned char * configfs_get_name(struct configfs_dirent *sd) { - struct configfs_attribute *attr; + const struct configfs_attribute *attr; BUG_ON(!sd || !sd->s_element); diff --git a/include/linux/configfs.h b/include/linux/configfs.h index ef65c75..5bead91 100644 --- a/include/linux/configfs.h +++ b/include/linux/configfs.h @@ -66,8 +66,11 @@ struct config_item_type { struct module *ct_owner; const struct configfs_item_operations *ct_item_ops; const struct configfs_group_operations *ct_group_ops; - struct configfs_attribute **ct_attrs; - struct configfs_bin_attribute **ct_bin_attrs; + union { + struct configfs_attribute **ct_attrs; + const struct configfs_attribute *const *ct_attrs_const; + }; + const struct configfs_bin_attribute *const *ct_bin_attrs; }; /** @@ -160,41 +163,41 @@ struct configfs_bin_attribute { ssize_t (*write)(struct config_item *, const void *, size_t); }; -#define CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz) \ -static struct configfs_bin_attribute _pfx##attr_##_name = { \ - .cb_attr = { \ - .ca_name = __stringify(_name), \ - .ca_mode = S_IRUGO | S_IWUSR, \ - .ca_owner = THIS_MODULE, \ - }, \ - .cb_private = _priv, \ - .cb_max_size = _maxsz, \ - .read = _pfx##_name##_read, \ - .write = _pfx##_name##_write, \ +#define CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz) \ +static const struct configfs_bin_attribute _pfx##attr_##_name = { \ + .cb_attr = { \ + .ca_name = __stringify(_name), \ + .ca_mode = S_IRUGO | S_IWUSR, \ + .ca_owner = THIS_MODULE, \ + }, \ + .cb_private = _priv, \ + .cb_max_size = _maxsz, \ + .read = _pfx##_name##_read, \ + .write = _pfx##_name##_write, \ } -#define CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz) \ -static struct configfs_bin_attribute _pfx##attr_##_name = { \ - .cb_attr = { \ - .ca_name = __stringify(_name), \ - .ca_mode = S_IRUGO, \ - .ca_owner = THIS_MODULE, \ - }, \ - .cb_private = _priv, \ - .cb_max_size = _maxsz, \ - .read = _pfx##_name##_read, \ +#define CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz) \ +static const struct configfs_bin_attribute _pfx##attr_##_name = { \ + .cb_attr = { \ + .ca_name = __stringify(_name), \ + .ca_mode = S_IRUGO, \ + .ca_owner = THIS_MODULE, \ + }, \ + .cb_private = _priv, \ + .cb_max_size = _maxsz, \ + .read = _pfx##_name##_read, \ } -#define CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz) \ -static struct configfs_bin_attribute _pfx##attr_##_name = { \ - .cb_attr = { \ - .ca_name = __stringify(_name), \ - .ca_mode = S_IWUSR, \ - .ca_owner = THIS_MODULE, \ - }, \ - .cb_private = _priv, \ - .cb_max_size = _maxsz, \ - .write = _pfx##_name##_write, \ +#define CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz) \ +static const struct configfs_bin_attribute _pfx##attr_##_name = { \ + .cb_attr = { \ + .ca_name = __stringify(_name), \ + .ca_mode = S_IWUSR, \ + .ca_owner = THIS_MODULE, \ + }, \ + .cb_private = _priv, \ + .cb_max_size = _maxsz, \ + .write = _pfx##_name##_write, \ } /* @@ -220,8 +223,8 @@ struct configfs_group_operations { struct config_group *(*make_group)(struct config_group *group, const char *name); void (*disconnect_notify)(struct config_group *group, struct config_item *item); void (*drop_item)(struct config_group *group, struct config_item *item); - bool (*is_visible)(struct config_item *item, struct configfs_attribute *attr, int n); - bool (*is_bin_visible)(struct config_item *item, struct configfs_bin_attribute *attr, + bool (*is_visible)(struct config_item *item, const struct configfs_attribute *attr, int n); + bool (*is_bin_visible)(struct config_item *item, const struct configfs_bin_attribute *attr, int n); }; diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs index cd082b8..dbed728 100644 --- a/rust/kernel/configfs.rs +++ b/rust/kernel/configfs.rs @@ -135,8 +135,9 @@ pub struct Subsystem { // SAFETY: We do not provide any operations on `Subsystem`. unsafe impl Sync for Subsystem {} -// SAFETY: Ownership of `Subsystem` can safely be transferred to other threads. -unsafe impl Send for Subsystem {} +// SAFETY: Ownership of `Subsystem` can safely be transferred to other threads +// if its data can be transferred as well. +unsafe impl Send for Subsystem {} impl Subsystem { /// Create an initializer for a [`Subsystem`]. @@ -249,6 +250,13 @@ pub struct Group { data: Data, } +// SAFETY: We do not provide any operations on `Group`. +unsafe impl Sync for Group {} + +// SAFETY: Ownership of `Group` can safely be transferred to other threads if +// its data can be transferred as well. +unsafe impl Send for Group {} + impl Group { /// Create an initializer for a new group. /// @@ -297,26 +305,39 @@ unsafe impl HasGroup for Group { /// /// `this` must be a valid pointer. /// -/// If `this` does not represent the root group of a configfs subsystem, -/// `this` must be a pointer to a `bindings::config_group` embedded in a -/// `Group`. +/// If `this` is the `su_group` field of a `bindings::configfs_subsystem`, that +/// `configfs_subsystem` must be embedded in a `Subsystem`. /// -/// Otherwise, `this` must be a pointer to a `bindings::config_group` that -/// is embedded in a `bindings::configfs_subsystem` that is embedded in a -/// `Subsystem`. +/// Otherwise, `this` must be a pointer to a `bindings::config_group` embedded +/// in a `Group`. unsafe fn get_group_data<'a, Parent>(this: *mut bindings::config_group) -> &'a Parent { // SAFETY: `this` is a valid pointer. - let is_root = unsafe { (*this).cg_subsys.is_null() }; + let subsys = unsafe { (*this).cg_subsys }; + + // `link_group()` in `fs/configfs/dir.c` assigns `cg_subsys` for every + // `config_group` attached anywhere in a registered subsystem, including + // the subsystem's own `su_group` (which gets a pointer to itself). The + // only `config_group` with a NULL `cg_subsys` is the configfs root, and + // userspace cannot trigger callbacks on it. The group is therefore the + // subsystem's `su_group` iff it equals `&cg_subsys->su_group`. + let is_root = !subsys.is_null() + && core::ptr::eq( + this.cast_const(), + // SAFETY: For every `config_group` the configfs core dispatches a + // callback on, `cg_subsys` was set by `link_group()` at registration time + // and points to a valid `configfs_subsystem` that outlives the callback. + unsafe { &raw const (*subsys).su_group }, + ); - if !is_root { - // SAFETY: By C API contact,`this` was returned from a call to - // `make_group`. The pointer is known to be embedded within a - // `Group`. - unsafe { &(*Group::::container_of(this)).data } - } else { - // SAFETY: By C API contract, `this` is a pointer to the - // `bindings::config_group` field within a `Subsystem`. + if is_root { + // SAFETY: By the above, `this` is the `su_group` field of a + // `configfs_subsystem` that, by function safety requirements, is + // embedded in a `Subsystem`. unsafe { &(*Subsystem::container_of(this)).data } + } else { + // SAFETY: By function safety requirements, `this` is a + // `config_group` embedded in a `Group`. + unsafe { &(*Group::::container_of(this)).data } } } @@ -325,7 +346,9 @@ struct GroupOperationsVTable(PhantomData<(Parent, Child)>); impl GroupOperationsVTable where Parent: GroupOperations, - Child: 'static, + // We transfer `Arc>` across a thread boundary in `make_group` + // and `drop_item`. + Arc>: Send, { /// # Safety /// @@ -344,8 +367,9 @@ where this: *mut bindings::config_group, name: *const kernel::ffi::c_char, ) -> *mut bindings::config_group { - // SAFETY: By function safety requirements of this function, this call - // is safe. + // SAFETY: By function safety requirements, `this` points to a configfs + // group containing `Parent`. The `GroupOperations` bound guarantees + // that `Parent: Sync`, so it is safe to share it with this thread. let parent_data = unsafe { get_group_data(this) }; let group_init = match Parent::make_group( @@ -390,8 +414,9 @@ where this: *mut bindings::config_group, item: *mut bindings::config_item, ) { - // SAFETY: By function safety requirements of this function, this call - // is safe. + // SAFETY: By function safety requirements, `this` points to a configfs + // group containing `Parent`. The `GroupOperations` bound guarantees + // that `Parent: Sync`, so it is safe to share it with this thread. let parent_data = unsafe { get_group_data(this) }; // SAFETY: By function safety requirements, `item` is embedded in a @@ -403,7 +428,9 @@ where if Parent::HAS_DROP_ITEM { // SAFETY: We called `into_raw` to produce `r_child_group_ptr` in - // `make_group`. + // `make_group`. This function may be executing on a different + // thread than `into_raw`. As `Arc>: Send` this + // ownership transfer is safe. let arc: Arc> = unsafe { Arc::from_raw(r_child_group_ptr.cast_mut()) }; Parent::drop_item(parent_data, arc.as_arc_borrow()); @@ -434,11 +461,13 @@ struct ItemOperationsVTable(PhantomData<(Container, Data)>); impl ItemOperationsVTable, Data> where Data: 'static, + // We transfer `Arc>` across a thread boundary in `release`. + Arc>: Send, { /// # Safety /// - /// `this` must be a pointer to a `bindings::config_group` embedded in a - /// `Group`. + /// `this` must be a pointer to the `cg_item` field of a `bindings::config_group` embedded in a + /// valid `Group`. /// /// This function will destroy the pointee of `this`. The pointee of `this` /// must not be accessed after the function returns. @@ -450,8 +479,9 @@ where // embedded within a `Group`. let r_group_ptr = unsafe { Group::::container_of(c_group_ptr) }; - // SAFETY: We called `into_raw` on `r_group_ptr` in - // `make_group`. + // SAFETY: We called `into_raw` on `r_group_ptr` in `make_group`. This + // function may be running on a different thread than the thread that + // called `into_raw`. As `Arc>: Send`, this is safe. let pin_self: Arc> = unsafe { Arc::from_raw(r_group_ptr.cast_mut()) }; drop(pin_self); } @@ -483,12 +513,12 @@ impl ItemOperationsVTable, Data> { /// /// Implement this trait on structs that embed a [`Subsystem`] or a [`Group`]. #[vtable] -pub trait GroupOperations { +pub trait GroupOperations: Sync { /// The child data object type. /// /// This group will create subgroups (subdirectories) backed by this kind of /// object. - type Child: 'static; + type Child: 'static + Send; /// Creates a new subgroup. /// @@ -555,8 +585,10 @@ where // `config_group`. unsafe { container_of!(item, bindings::config_group, cg_item) }; - // SAFETY: The function safety requirements for this function satisfy - // the conditions for this call. + // SAFETY: By function safety requirements, `c_group` points to a + // configfs group containing `Data`. The `AttributeOperations` bound + // guarantees that `Data: Sync`, so it is safe to share it with this + // thread. let data: &Data = unsafe { get_group_data(c_group) }; // SAFETY: By function safety requirements, `page` is writable for `PAGE_SIZE`. @@ -589,8 +621,10 @@ where // `config_group`. unsafe { container_of!(item, bindings::config_group, cg_item) }; - // SAFETY: The function safety requirements for this function satisfy - // the conditions for this call. + // SAFETY: By function safety requirements, `c_group` points to a + // configfs group containing `Data`. The `AttributeOperations` bound + // guarantees that `Data: Sync`, so it is safe to share it with this + // thread. let data: &Data = unsafe { get_group_data(c_group) }; let ret = O::store( @@ -643,7 +677,7 @@ where pub trait AttributeOperations { /// The type of the object that contains the field that is backing the /// attribute for this operation. - type Data; + type Data: Sync; /// Renders the value of an attribute. /// @@ -749,15 +783,18 @@ macro_rules! impl_item_type { ) -> Self where Data: GroupOperations, - Child: 'static, + Arc>: Send, + Arc>: Send, { Self { item_type: Opaque::new(bindings::config_item_type { ct_owner: owner.as_ptr(), - ct_group_ops: GroupOperationsVTable::::vtable_ptr().cast_mut(), - ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr().cast_mut(), - ct_attrs: core::ptr::from_ref(attributes).cast_mut().cast(), - ct_bin_attrs: core::ptr::null_mut(), + ct_group_ops: GroupOperationsVTable::::vtable_ptr(), + ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr(), + __bindgen_anon_1: bindings::config_item_type__bindgen_ty_1 { + ct_attrs_const: core::ptr::from_ref(attributes).cast(), + }, + ct_bin_attrs: core::ptr::null(), }), _p: PhantomData, } @@ -767,14 +804,19 @@ macro_rules! impl_item_type { pub const fn new( owner: &'static ThisModule, attributes: &'static AttributeList, - ) -> Self { + ) -> Self + where + Arc>: Send, + { Self { item_type: Opaque::new(bindings::config_item_type { ct_owner: owner.as_ptr(), - ct_group_ops: core::ptr::null_mut(), - ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr().cast_mut(), - ct_attrs: core::ptr::from_ref(attributes).cast_mut().cast(), - ct_bin_attrs: core::ptr::null_mut(), + ct_group_ops: core::ptr::null(), + ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr(), + __bindgen_anon_1: bindings::config_item_type__bindgen_ty_1 { + ct_attrs_const: core::ptr::from_ref(attributes).cast(), + }, + ct_bin_attrs: core::ptr::null(), }), _p: PhantomData, } diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c index c1b108e..6e3a8e0 100644 --- a/samples/configfs/configfs_sample.c +++ b/samples/configfs/configfs_sample.c @@ -84,7 +84,7 @@ CONFIGFS_ATTR_RO(childless_, showme); CONFIGFS_ATTR(childless_, storeme); CONFIGFS_ATTR_RO(childless_, description); -static struct configfs_attribute *childless_attrs[] = { +static const struct configfs_attribute *const childless_attrs[] = { &childless_attr_showme, &childless_attr_storeme, &childless_attr_description, @@ -92,7 +92,7 @@ static struct configfs_attribute *childless_attrs[] = { }; static const struct config_item_type childless_type = { - .ct_attrs = childless_attrs, + .ct_attrs_const = childless_attrs, .ct_owner = THIS_MODULE, }; @@ -148,7 +148,7 @@ static ssize_t simple_child_storeme_store(struct config_item *item, CONFIGFS_ATTR(simple_child_, storeme); -static struct configfs_attribute *simple_child_attrs[] = { +static const struct configfs_attribute *const simple_child_attrs[] = { &simple_child_attr_storeme, NULL, }; @@ -164,7 +164,7 @@ static const struct configfs_item_operations simple_child_item_ops = { static const struct config_item_type simple_child_type = { .ct_item_ops = &simple_child_item_ops, - .ct_attrs = simple_child_attrs, + .ct_attrs_const = simple_child_attrs, .ct_owner = THIS_MODULE, }; @@ -205,7 +205,7 @@ static ssize_t simple_children_description_show(struct config_item *item, CONFIGFS_ATTR_RO(simple_children_, description); -static struct configfs_attribute *simple_children_attrs[] = { +static const struct configfs_attribute *const simple_children_attrs[] = { &simple_children_attr_description, NULL, }; @@ -230,7 +230,7 @@ static const struct configfs_group_operations simple_children_group_ops = { static const struct config_item_type simple_children_type = { .ct_item_ops = &simple_children_item_ops, .ct_group_ops = &simple_children_group_ops, - .ct_attrs = simple_children_attrs, + .ct_attrs_const = simple_children_attrs, .ct_owner = THIS_MODULE, }; @@ -283,7 +283,7 @@ static ssize_t group_children_description_show(struct config_item *item, CONFIGFS_ATTR_RO(group_children_, description); -static struct configfs_attribute *group_children_attrs[] = { +static const struct configfs_attribute *const group_children_attrs[] = { &group_children_attr_description, NULL, }; @@ -298,7 +298,7 @@ static const struct configfs_group_operations group_children_group_ops = { static const struct config_item_type group_children_type = { .ct_group_ops = &group_children_group_ops, - .ct_attrs = group_children_attrs, + .ct_attrs_const = group_children_attrs, .ct_owner = THIS_MODULE, }; @@ -314,6 +314,126 @@ static struct configfs_subsystem group_children_subsys = { /* ----------------------------------------------------------------- */ /* + * 04-symlink-children + * + * This example has children that are valid sources for symlink(2). A + * child accepts a link to any other config_item and reports how many + * links it currently holds, so ->allow_link() and ->drop_link() are + * observable from userspace. + */ + +struct symlink_child { + struct config_item item; + int nlinks; +}; + +static inline struct symlink_child *to_symlink_child(struct config_item *item) +{ + return container_of(item, struct symlink_child, item); +} + +static ssize_t symlink_child_nlinks_show(struct config_item *item, char *page) +{ + return sprintf(page, "%d\n", to_symlink_child(item)->nlinks); +} + +CONFIGFS_ATTR_RO(symlink_child_, nlinks); + +static struct configfs_attribute *symlink_child_attrs[] = { + &symlink_child_attr_nlinks, + NULL, +}; + +/* + * The VFS holds the source item's directory locked across symlink(2) and + * unlink(2), so ->nlinks needs no lock of its own. + */ +static int symlink_child_allow_link(struct config_item *src, + struct config_item *target) +{ + to_symlink_child(src)->nlinks++; + + return 0; +} + +static void symlink_child_drop_link(struct config_item *src, + struct config_item *target) +{ + to_symlink_child(src)->nlinks--; +} + +static void symlink_child_release(struct config_item *item) +{ + kfree(to_symlink_child(item)); +} + +static const struct configfs_item_operations symlink_child_item_ops = { + .release = symlink_child_release, + .allow_link = symlink_child_allow_link, + .drop_link = symlink_child_drop_link, +}; + +static const struct config_item_type symlink_child_type = { + .ct_item_ops = &symlink_child_item_ops, + .ct_attrs = symlink_child_attrs, + .ct_owner = THIS_MODULE, +}; + +static struct config_item *symlink_children_make_item( + struct config_group *group, const char *name) +{ + struct symlink_child *symlink_child; + + symlink_child = kzalloc_obj(*symlink_child, GFP_KERNEL); + if (!symlink_child) + return ERR_PTR(-ENOMEM); + + config_item_init_type_name(&symlink_child->item, name, + &symlink_child_type); + + return &symlink_child->item; +} + +static ssize_t symlink_children_description_show(struct config_item *item, + char *page) +{ + return sprintf(page, +"[04-symlink-children]\n" +"\n" +"This subsystem allows the creation of child config_items that\n" +"symlink(2) can point at other config_items from. Each child\n" +"reports the number of links it holds.\n"); +} + +CONFIGFS_ATTR_RO(symlink_children_, description); + +static struct configfs_attribute *symlink_children_attrs[] = { + &symlink_children_attr_description, + NULL, +}; + +static const struct configfs_group_operations symlink_children_group_ops = { + .make_item = symlink_children_make_item, +}; + +static const struct config_item_type symlink_children_type = { + .ct_group_ops = &symlink_children_group_ops, + .ct_attrs = symlink_children_attrs, + .ct_owner = THIS_MODULE, +}; + +static struct configfs_subsystem symlink_children_subsys = { + .su_group = { + .cg_item = { + .ci_namebuf = "04-symlink-children", + .ci_type = &symlink_children_type, + }, + }, +}; + +/* ----------------------------------------------------------------- */ + +/* * We're now done with our subsystem definitions. * For convenience in this module, here's a list of them all. It * allows the init function to easily register them. Most modules @@ -324,6 +444,7 @@ static struct configfs_subsystem *example_subsys[] = { &childless_subsys.subsys, &simple_children_subsys, &group_children_subsys, + &symlink_children_subsys, NULL, }; diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 32dde7b..7e8d579 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -34,6 +34,7 @@ TARGETS += exec TARGETS += fchmodat2 TARGETS += filesystems TARGETS += filesystems/binderfs +TARGETS += filesystems/configfs TARGETS += filesystems/epoll TARGETS += filesystems/eventfd TARGETS += filesystems/failfs diff --git a/tools/testing/selftests/filesystems/configfs/.gitignore b/tools/testing/selftests/filesystems/configfs/.gitignore new file mode 100644 index 0000000..accfb6b --- /dev/null +++ b/tools/testing/selftests/filesystems/configfs/.gitignore @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +configfs_test diff --git a/tools/testing/selftests/filesystems/configfs/Makefile b/tools/testing/selftests/filesystems/configfs/Makefile new file mode 100644 index 0000000..40a91ed --- /dev/null +++ b/tools/testing/selftests/filesystems/configfs/Makefile @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2026 Meta Platforms, Inc. and affiliates +# Copyright (c) 2026 Breno Leitao + +CFLAGS += -Wall -Werror -pthread +TEST_GEN_PROGS := configfs_test + +include ../../lib.mk diff --git a/tools/testing/selftests/filesystems/configfs/config b/tools/testing/selftests/filesystems/configfs/config new file mode 100644 index 0000000..5ea17df --- /dev/null +++ b/tools/testing/selftests/filesystems/configfs/config @@ -0,0 +1,5 @@ +CONFIG_CONFIGFS_FS=y +CONFIG_MODULES=y +CONFIG_MODULE_UNLOAD=y +CONFIG_SAMPLES=y +CONFIG_SAMPLE_CONFIGFS=m diff --git a/tools/testing/selftests/filesystems/configfs/configfs_test.c b/tools/testing/selftests/filesystems/configfs/configfs_test.c new file mode 100644 index 0000000..c6a1049 --- /dev/null +++ b/tools/testing/selftests/filesystems/configfs/configfs_test.c @@ -0,0 +1,481 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Exercise the configfs userspace interface through the subsystems + * registered by samples/configfs. + * + * Copyright (c) 2026 Meta Platforms, Inc. and affiliates + * Copyright (c) 2026 Breno Leitao + */ +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "kselftest_harness.h" + +/* Private to fs/configfs/mount.c. */ +#define CONFIGFS_MAGIC 0x62656570 + +#define SAMPLE_MODULE "configfs_sample" + +#define CHILDLESS "01-childless" +#define SIMPLE "02-simple-children" +#define GROUPS "03-group-children" +#define SYMLINKS "04-symlink-children" + +#define ITEM_A SIMPLE "/kselftest-a" +#define ITEM_B SIMPLE "/kselftest-b" +#define GROUP GROUPS "/kselftest-group" +#define GROUP_ITEM GROUP "/kselftest-a" +#define LINK_SRC SYMLINKS "/kselftest-src" +#define LINK LINK_SRC "/kselftest-link" + +#define RACE_ITERATIONS 20000 + +static const char * const test_links[] = { + LINK, +}; + +/* Deepest first, so one pass empties the tree. */ +static const char * const test_dirs[] = { + GROUP_ITEM, + GROUP, + LINK_SRC, + ITEM_A, + ITEM_B, +}; + +static void drop_test_dirs(void) +{ + size_t i; + + /* Links first: they hold both their source and their target. */ + for (i = 0; i < ARRAY_SIZE(test_links); i++) + unlink(test_links[i]); + + for (i = 0; i < ARRAY_SIZE(test_dirs); i++) + rmdir(test_dirs[i]); +} + +static ssize_t read_attr(const char *path, char *buf, size_t len) +{ + ssize_t ret; + int fd; + + fd = open(path, O_RDONLY); + if (fd < 0) + return -1; + + ret = read(fd, buf, len - 1); + close(fd); + if (ret < 0) + return -1; + + buf[ret] = '\0'; + return ret; +} + +static ssize_t write_attr(const char *path, const char *val) +{ + ssize_t ret; + int fd, err; + + fd = open(path, O_WRONLY); + if (fd < 0) + return -1; + + ret = write(fd, val, strlen(val)); + err = errno; + close(fd); + errno = err; + + return ret; +} + +FIXTURE(configfs) { + char mnt[sizeof(P_tmpdir "/configfs_XXXXXX")]; + bool mounted; +}; + +FIXTURE_SETUP(configfs) +{ + char tmpl[] = P_tmpdir "/configfs_XXXXXX"; + + if (geteuid()) + SKIP(return, "need root to load modules and mount configfs"); + + ASSERT_EQ(system("modprobe -q " SAMPLE_MODULE), 0) + TH_LOG(SAMPLE_MODULE " missing, is CONFIG_SAMPLE_CONFIGFS=m?"); + + ASSERT_EQ(unshare(CLONE_NEWNS), 0); + ASSERT_EQ(mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL), 0); + + ASSERT_NE(mkdtemp(tmpl), NULL); + strcpy(self->mnt, tmpl); + + ASSERT_EQ(mount("configfs", self->mnt, "configfs", 0, NULL), 0); + ASSERT_EQ(chdir(self->mnt), 0); + self->mounted = true; + + /* configfs items outlive the mount, so a killed run leaves some. */ + drop_test_dirs(); +} + +FIXTURE_TEARDOWN(configfs) +{ + if (self->mounted) { + drop_test_dirs(); + EXPECT_EQ(chdir("/"), 0); + EXPECT_EQ(umount2(self->mnt, MNT_DETACH), 0); + } + + if (self->mnt[0]) + EXPECT_EQ(rmdir(self->mnt), 0); +} + +TEST_F(configfs, mount_and_subsystems) +{ + const char * const subsys[] = { CHILDLESS, SIMPLE, GROUPS, SYMLINKS }; + struct statfs sfs; + struct stat st; + size_t i; + + ASSERT_EQ(statfs(".", &sfs), 0); + EXPECT_EQ(sfs.f_type, CONFIGFS_MAGIC); + + for (i = 0; i < ARRAY_SIZE(subsys); i++) { + ASSERT_EQ(stat(subsys[i], &st), 0) + TH_LOG("%s is missing", subsys[i]); + EXPECT_TRUE(S_ISDIR(st.st_mode)); + } +} + +TEST_F(configfs, mkdir_at_root) +{ + /* The root has no ->mkdir(); only subsystems register there. */ + ASSERT_EQ(mkdir("kselftest-root", 0755), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, rmdir_subsystem) +{ + ASSERT_EQ(rmdir(CHILDLESS), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, mkdir_without_group_ops) +{ + /* 01-childless has attributes but no ->make_item()/->make_group(). */ + ASSERT_EQ(mkdir(CHILDLESS "/kselftest-a", 0755), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, attr_store_and_show) +{ + char buf[64]; + + ASSERT_GT(write_attr(CHILDLESS "/storeme", "42"), 0); + ASSERT_GT(read_attr(CHILDLESS "/storeme", buf, sizeof(buf)), 0); + EXPECT_STREQ(buf, "42\n"); +} + +TEST_F(configfs, attr_store_rejects_garbage) +{ + ASSERT_EQ(write_attr(CHILDLESS "/storeme", "not-a-number"), -1); + EXPECT_EQ(errno, EINVAL); +} + +TEST_F(configfs, attr_show_runs_on_every_open) +{ + char first[64], second[64]; + + /* 01-childless/showme increments the value it just returned. */ + ASSERT_GT(read_attr(CHILDLESS "/showme", first, sizeof(first)), 0); + ASSERT_GT(read_attr(CHILDLESS "/showme", second, sizeof(second)), 0); + EXPECT_EQ(atoi(second), atoi(first) + 1); +} + +TEST_F(configfs, attr_read_only) +{ + ASSERT_EQ(open(CHILDLESS "/description", O_WRONLY), -1); + EXPECT_EQ(errno, EACCES); +} + +TEST_F(configfs, attr_unlink) +{ + /* ->unlink() only accepts the symlinks configfs itself created. */ + ASSERT_EQ(unlink(CHILDLESS "/storeme"), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, attr_read_length) +{ + char buf[8192]; + struct stat st; + ssize_t n; + int fd; + + fd = open(CHILDLESS "/description", O_RDONLY); + ASSERT_GE(fd, 0); + + /* Attributes report a page, whatever ->show() ends up producing. */ + ASSERT_EQ(fstat(fd, &st), 0); + EXPECT_EQ(st.st_size, sysconf(_SC_PAGESIZE)); + + n = read(fd, buf, sizeof(buf)); + ASSERT_GT(n, 0); + EXPECT_LT(n, st.st_size); + EXPECT_EQ(read(fd, buf, sizeof(buf)), 0); + + EXPECT_EQ(close(fd), 0); +} + +TEST_F(configfs, attr_write_is_not_incremental) +{ + char buf[64]; + int fd; + + /* + * Every write hands the whole buffer to ->store() and the file + * position is ignored, so the second write replaces the first. + */ + fd = open(CHILDLESS "/storeme", O_WRONLY); + ASSERT_GE(fd, 0); + ASSERT_EQ(write(fd, "1", 1), 1); + ASSERT_EQ(write(fd, "2", 1), 1); + EXPECT_EQ(close(fd), 0); + + ASSERT_GT(read_attr(CHILDLESS "/storeme", buf, sizeof(buf)), 0); + EXPECT_STREQ(buf, "2\n"); +} + +TEST_F(configfs, item_create_and_drop) +{ + struct stat st; + + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + EXPECT_EQ(stat(ITEM_A "/storeme", &st), 0); + + /* The item carries its own attributes, not the subsystem's. */ + ASSERT_EQ(stat(ITEM_A "/description", &st), -1); + EXPECT_EQ(errno, ENOENT); + + ASSERT_EQ(rmdir(ITEM_A), 0); + ASSERT_EQ(stat(ITEM_A, &st), -1); + EXPECT_EQ(errno, ENOENT); +} + +TEST_F(configfs, item_create_twice) +{ + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + ASSERT_EQ(mkdir(ITEM_A, 0755), -1); + EXPECT_EQ(errno, EEXIST); +} + +TEST_F(configfs, item_has_no_children) +{ + /* ->make_item() produces an item, so it cannot nest. */ + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + ASSERT_EQ(mkdir(ITEM_A "/kselftest-b", 0755), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, item_attrs_are_private) +{ + char buf[64]; + + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + ASSERT_EQ(mkdir(ITEM_B, 0755), 0); + + ASSERT_GT(write_attr(ITEM_A "/storeme", "11"), 0); + ASSERT_GT(write_attr(ITEM_B "/storeme", "22"), 0); + + ASSERT_GT(read_attr(ITEM_A "/storeme", buf, sizeof(buf)), 0); + EXPECT_STREQ(buf, "11\n"); + ASSERT_GT(read_attr(ITEM_B "/storeme", buf, sizeof(buf)), 0); + EXPECT_STREQ(buf, "22\n"); +} + +TEST_F(configfs, group_create_and_drop) +{ + struct stat st; + + /* 03-group-children hands out groups that take items of their own. */ + ASSERT_EQ(mkdir(GROUP, 0755), 0); + EXPECT_EQ(stat(GROUP "/description", &st), 0); + + ASSERT_EQ(mkdir(GROUP_ITEM, 0755), 0); + EXPECT_EQ(stat(GROUP_ITEM "/storeme", &st), 0); + + ASSERT_EQ(rmdir(GROUP), -1); + EXPECT_EQ(errno, ENOTEMPTY); + + ASSERT_EQ(rmdir(GROUP_ITEM), 0); + ASSERT_EQ(rmdir(GROUP), 0); +} + +TEST_F(configfs, rename_item) +{ + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + ASSERT_EQ(rename(ITEM_A, ITEM_B), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, symlink_without_allow_link) +{ + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + ASSERT_EQ(symlink(ITEM_A, SIMPLE "/kselftest-link"), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, symlink_and_unlink) +{ + char buf[PATH_MAX]; + struct stat st; + ssize_t n; + + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + ASSERT_EQ(mkdir(LINK_SRC, 0755), 0); + + ASSERT_EQ(symlink(ITEM_A, LINK), 0); + + /* configfs stores its own body, a path relative to the link. */ + n = readlink(LINK, buf, sizeof(buf) - 1); + ASSERT_GT(n, 0); + buf[n] = '\0'; + EXPECT_STREQ(buf, "../../" ITEM_A); + EXPECT_EQ(stat(LINK "/storeme", &st), 0); + + /* ->allow_link() ran on the source, not on the target. */ + ASSERT_GT(read_attr(LINK_SRC "/nlinks", buf, sizeof(buf)), 0); + EXPECT_STREQ(buf, "1\n"); + + ASSERT_EQ(unlink(LINK), 0); + ASSERT_GT(read_attr(LINK_SRC "/nlinks", buf, sizeof(buf)), 0); + EXPECT_STREQ(buf, "0\n"); +} + +TEST_F(configfs, symlink_pins_both_ends) +{ + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + ASSERT_EQ(mkdir(LINK_SRC, 0755), 0); + ASSERT_EQ(symlink(ITEM_A, LINK), 0); + + /* A linked item cannot go away under the link. */ + ASSERT_EQ(rmdir(ITEM_A), -1); + EXPECT_EQ(errno, EBUSY); + + /* The link counts as a child of its source. */ + ASSERT_EQ(rmdir(LINK_SRC), -1); + EXPECT_EQ(errno, ENOTEMPTY); + + ASSERT_EQ(unlink(LINK), 0); + EXPECT_EQ(rmdir(ITEM_A), 0); +} + +TEST_F(configfs, symlink_target_outside_configfs) +{ + ASSERT_EQ(mkdir(LINK_SRC, 0755), 0); + ASSERT_EQ(symlink("/", LINK), -1); + EXPECT_EQ(errno, EPERM); +} + +TEST_F(configfs, symlink_target_missing) +{ + ASSERT_EQ(mkdir(LINK_SRC, 0755), 0); + ASSERT_EQ(symlink(SIMPLE "/kselftest-gone", LINK), -1); + EXPECT_EQ(errno, ENOENT); +} + +TEST_F(configfs, symlink_target_is_an_attribute) +{ + ASSERT_EQ(mkdir(LINK_SRC, 0755), 0); + + /* The target is resolved with LOOKUP_DIRECTORY. */ + ASSERT_EQ(symlink(CHILDLESS "/storeme", LINK), -1); + EXPECT_EQ(errno, ENOTDIR); +} + +static volatile int race_stop; + +static void *rmdir_target(void *arg) +{ + while (!race_stop) { + if (mkdir(ITEM_A, 0755) == 0 || errno == EEXIST) + rmdir(ITEM_A); + } + + return NULL; +} + +/* -1 if the kernel does not export a warning counter. */ +static long warn_count(void) +{ + char buf[32]; + + if (read_attr("/sys/kernel/warn_count", buf, sizeof(buf)) < 0) + return -1; + + return strtol(buf, NULL, 10); +} + +TEST_F(configfs, symlink_races_with_target_rmdir) +{ + pthread_t thread; + long warns; + int i; + + warns = warn_count(); + if (warns < 0) + SKIP(return, "no /sys/kernel/warn_count to watch"); + + ASSERT_EQ(mkdir(LINK_SRC, 0755), 0); + ASSERT_EQ(pthread_create(&thread, NULL, rmdir_target, NULL), 0); + + /* + * configfs_rmdir() drops the last reference to the item while its + * dentry is still hashed, and get_target() takes a hashed dentry as + * proof that the item behind it is alive. The symlink then walks + * ->ci_dentry into a released dirent, which configfs_get() warns + * about. KASAN sees the freed item itself. + */ + for (i = 0; i < RACE_ITERATIONS; i++) { + if (symlink(ITEM_A, LINK) == 0) + unlink(LINK); + + /* Give up on the first splat rather than flood the log. */ + if (!(i % 128) && warn_count() != warns) + break; + } + + race_stop = 1; + ASSERT_EQ(pthread_join(thread, NULL), 0); + + EXPECT_EQ(warn_count(), warns) + TH_LOG("kernel warned after %d iterations", i); +} + +TEST_F(configfs, module_pinned_by_item) +{ + ASSERT_EQ(mkdir(ITEM_A, 0755), 0); + + /* mkdir() pins both the subsystem's module and the new item's. */ + ASSERT_EQ(syscall(__NR_delete_module, SAMPLE_MODULE, O_NONBLOCK), -1); + if (errno == ENOSYS) + SKIP(return, "kernel built without CONFIG_MODULE_UNLOAD"); + EXPECT_EQ(errno, EWOULDBLOCK); +} + +TEST_HARNESS_MAIN