| Message ID | 20200218162943.2488012-4-christian.brauner@ubuntu.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | net: fix sysfs permssions when device changes network | expand |
On Tue, Feb 18, 2020 at 05:29:37PM +0100, Christian Brauner wrote: > Add helpers to change the owner of sysfs groups. > This function will be used to correctly account for kobject ownership > changes, e.g. when moving network devices between network namespaces. > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > --- > /* v2 */ > - Greg Kroah-Hartman <gregkh@linuxfoundation.org>: > - Add comment how ownership of sysfs object is changed. > > /* v3 */ > - Greg Kroah-Hartman <gregkh@linuxfoundation.org>: > - Add explicit uid/gid parameters. > - Christian Brauner <christian.brauner@ubuntu.com>: > - Collapse groups ownership helper patches into a single patch. > --- > fs/sysfs/group.c | 117 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/sysfs.h | 20 ++++++++ > 2 files changed, 137 insertions(+) > > diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c > index c4ab045926b7..bae562d3cba1 100644 > --- a/fs/sysfs/group.c > +++ b/fs/sysfs/group.c > @@ -13,6 +13,7 @@ > #include <linux/dcache.h> > #include <linux/namei.h> > #include <linux/err.h> > +#include <linux/fs.h> > #include "sysfs.h" > > > @@ -457,3 +458,119 @@ int __compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj, > return PTR_ERR_OR_ZERO(link); > } > EXPORT_SYMBOL_GPL(__compat_only_sysfs_link_entry_to_kobj); > + > +static int sysfs_group_attrs_change_owner(struct kernfs_node *grp_kn, > + const struct attribute_group *grp, > + struct iattr *newattrs) > +{ > + struct kernfs_node *kn; > + int error; > + > + if (grp->attrs) { > + struct attribute *const *attr; > + > + for (attr = grp->attrs; *attr; attr++) { > + kn = kernfs_find_and_get(grp_kn, (*attr)->name); > + if (!kn) > + return -ENOENT; > + > + error = kernfs_setattr(kn, newattrs); > + kernfs_put(kn); > + if (error) > + return error; > + } > + } > + > + if (grp->bin_attrs) { > + struct bin_attribute *const *bin_attr; > + > + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) { > + kn = kernfs_find_and_get(grp_kn, (*bin_attr)->attr.name); > + if (!kn) > + return -ENOENT; > + > + error = kernfs_setattr(kn, newattrs); > + kernfs_put(kn); > + if (error) > + return error; > + } > + } > + > + return 0; > +} > + > +/** > + * sysfs_group_change_owner - change owner of an attribute group. > + * @kobj: The kobject containing the group. > + * @grp: The attribute group. > + * @kuid: new owner's kuid > + * @kgid: new owner's kgid > + * > + * Returns 0 on success or error code on failure. This is fine to document, just funny it's the only one documented about the return value so far in this series. Anyway, looks good to me: Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Thu, Feb 20, 2020 at 12:15:50PM +0100, Greg Kroah-Hartman wrote: > On Tue, Feb 18, 2020 at 05:29:37PM +0100, Christian Brauner wrote: > > Add helpers to change the owner of sysfs groups. > > This function will be used to correctly account for kobject ownership > > changes, e.g. when moving network devices between network namespaces. > > > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > > --- > > /* v2 */ > > - Greg Kroah-Hartman <gregkh@linuxfoundation.org>: > > - Add comment how ownership of sysfs object is changed. > > > > /* v3 */ > > - Greg Kroah-Hartman <gregkh@linuxfoundation.org>: > > - Add explicit uid/gid parameters. > > - Christian Brauner <christian.brauner@ubuntu.com>: > > - Collapse groups ownership helper patches into a single patch. > > --- > > fs/sysfs/group.c | 117 ++++++++++++++++++++++++++++++++++++++++++ > > include/linux/sysfs.h | 20 ++++++++ > > 2 files changed, 137 insertions(+) > > > > diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c > > index c4ab045926b7..bae562d3cba1 100644 > > --- a/fs/sysfs/group.c > > +++ b/fs/sysfs/group.c > > @@ -13,6 +13,7 @@ > > #include <linux/dcache.h> > > #include <linux/namei.h> > > #include <linux/err.h> > > +#include <linux/fs.h> > > #include "sysfs.h" > > > > > > @@ -457,3 +458,119 @@ int __compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj, > > return PTR_ERR_OR_ZERO(link); > > } > > EXPORT_SYMBOL_GPL(__compat_only_sysfs_link_entry_to_kobj); > > + > > +static int sysfs_group_attrs_change_owner(struct kernfs_node *grp_kn, > > + const struct attribute_group *grp, > > + struct iattr *newattrs) > > +{ > > + struct kernfs_node *kn; > > + int error; > > + > > + if (grp->attrs) { > > + struct attribute *const *attr; > > + > > + for (attr = grp->attrs; *attr; attr++) { > > + kn = kernfs_find_and_get(grp_kn, (*attr)->name); > > + if (!kn) > > + return -ENOENT; > > + > > + error = kernfs_setattr(kn, newattrs); > > + kernfs_put(kn); > > + if (error) > > + return error; > > + } > > + } > > + > > + if (grp->bin_attrs) { > > + struct bin_attribute *const *bin_attr; > > + > > + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) { > > + kn = kernfs_find_and_get(grp_kn, (*bin_attr)->attr.name); > > + if (!kn) > > + return -ENOENT; > > + > > + error = kernfs_setattr(kn, newattrs); > > + kernfs_put(kn); > > + if (error) > > + return error; > > + } > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * sysfs_group_change_owner - change owner of an attribute group. > > + * @kobj: The kobject containing the group. > > + * @grp: The attribute group. > > + * @kuid: new owner's kuid > > + * @kgid: new owner's kgid > > + * > > + * Returns 0 on success or error code on failure. > > This is fine to document, just funny it's the only one documented about > the return value so far in this series. I stuck to the documentation style common to the file. Most of the functions in fs/syfs/file.c did not mention return codes sysfs_remove_bin_file(), sysfs_create_bin_file(), sysfs_remove_file_from_group() etc. But I'll document all in this series with return codes now. Thanks! Christian
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c index c4ab045926b7..bae562d3cba1 100644 --- a/fs/sysfs/group.c +++ b/fs/sysfs/group.c @@ -13,6 +13,7 @@ #include <linux/dcache.h> #include <linux/namei.h> #include <linux/err.h> +#include <linux/fs.h> #include "sysfs.h" @@ -457,3 +458,119 @@ int __compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj, return PTR_ERR_OR_ZERO(link); } EXPORT_SYMBOL_GPL(__compat_only_sysfs_link_entry_to_kobj); + +static int sysfs_group_attrs_change_owner(struct kernfs_node *grp_kn, + const struct attribute_group *grp, + struct iattr *newattrs) +{ + struct kernfs_node *kn; + int error; + + if (grp->attrs) { + struct attribute *const *attr; + + for (attr = grp->attrs; *attr; attr++) { + kn = kernfs_find_and_get(grp_kn, (*attr)->name); + if (!kn) + return -ENOENT; + + error = kernfs_setattr(kn, newattrs); + kernfs_put(kn); + if (error) + return error; + } + } + + if (grp->bin_attrs) { + struct bin_attribute *const *bin_attr; + + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) { + kn = kernfs_find_and_get(grp_kn, (*bin_attr)->attr.name); + if (!kn) + return -ENOENT; + + error = kernfs_setattr(kn, newattrs); + kernfs_put(kn); + if (error) + return error; + } + } + + return 0; +} + +/** + * sysfs_group_change_owner - change owner of an attribute group. + * @kobj: The kobject containing the group. + * @grp: The attribute group. + * @kuid: new owner's kuid + * @kgid: new owner's kgid + * + * Returns 0 on success or error code on failure. + */ +int sysfs_group_change_owner(struct kobject *kobj, + const struct attribute_group *grp, kuid_t kuid, + kgid_t kgid) +{ + struct kernfs_node *grp_kn; + kuid_t uid; + kgid_t gid; + int error; + struct iattr newattrs = { + .ia_valid = ATTR_UID | ATTR_GID, + }; + + if (!kobj->state_in_sysfs) + return -EINVAL; + + if (grp->name) { + grp_kn = kernfs_find_and_get(kobj->sd, grp->name); + } else { + kernfs_get(kobj->sd); + grp_kn = kobj->sd; + } + if (!grp_kn) + return -ENOENT; + + newattrs.ia_uid = kuid; + newattrs.ia_gid = kgid; + error = kernfs_setattr(grp_kn, &newattrs); + if (!error) + error = sysfs_group_attrs_change_owner(grp_kn, grp, &newattrs); + + kernfs_put(grp_kn); + + return error; +} +EXPORT_SYMBOL_GPL(sysfs_group_change_owner); + +/** + * sysfs_groups_change_owner - change owner of a set of attribute groups. + * @kobj: The kobject containing the groups. + * @groups: The attribute groups. + * @kuid: new owner's kuid + * @kgid: new owner's kgid + * + * Returns 0 on success or error code on failure. + */ +int sysfs_groups_change_owner(struct kobject *kobj, + const struct attribute_group **groups, + kuid_t kuid, kgid_t kgid) +{ + int error = 0, i; + + if (!kobj->state_in_sysfs) + return -EINVAL; + + if (!groups) + return 0; + + for (i = 0; groups[i]; i++) { + error = sysfs_group_change_owner(kobj, groups[i], kuid, kgid); + if (error) + break; + } + + return error; +} +EXPORT_SYMBOL_GPL(sysfs_groups_change_owner); diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 899500950410..564a2e57b90a 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -315,6 +315,12 @@ int sysfs_file_change_owner_by_name(struct kobject *kobj, const char *name, kuid_t kuid, kgid_t kgid); int sysfs_link_change_owner(struct kobject *kobj, struct kobject *targ, const char *name, kuid_t kuid, kgid_t kgid); +int sysfs_groups_change_owner(struct kobject *kobj, + const struct attribute_group **groups, + kuid_t kuid, kgid_t kgid); +int sysfs_group_change_owner(struct kobject *kobj, + const struct attribute_group *groups, kuid_t kuid, + kgid_t kgid); #else /* CONFIG_SYSFS */ @@ -549,6 +555,20 @@ static inline int sysfs_link_change_owner(struct kobject *kobj, return 0; } +static inline int sysfs_groups_change_owner(struct kobject *kobj, + const struct attribute_group **groups, + kuid_t kuid, kgid_t kgid) +{ + return 0; +} + +static inline int sysfs_group_change_owner(struct kobject *kobj, + const struct attribute_group **groups, + kuid_t kuid, kgid_t kgid) +{ + return 0; +} + #endif /* CONFIG_SYSFS */ static inline int __must_check sysfs_create_file(struct kobject *kobj,
Add helpers to change the owner of sysfs groups. This function will be used to correctly account for kobject ownership changes, e.g. when moving network devices between network namespaces. Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- /* v2 */ - Greg Kroah-Hartman <gregkh@linuxfoundation.org>: - Add comment how ownership of sysfs object is changed. /* v3 */ - Greg Kroah-Hartman <gregkh@linuxfoundation.org>: - Add explicit uid/gid parameters. - Christian Brauner <christian.brauner@ubuntu.com>: - Collapse groups ownership helper patches into a single patch. --- fs/sysfs/group.c | 117 ++++++++++++++++++++++++++++++++++++++++++ include/linux/sysfs.h | 20 ++++++++ 2 files changed, 137 insertions(+)