| Message ID | 20200218162943.2488012-3-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:36PM +0100, Christian Brauner wrote: > Add a helper to change the owner of a sysfs link. > 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. > --- > fs/sysfs/file.c | 40 ++++++++++++++++++++++++++++++++++++++++ > include/linux/sysfs.h | 10 ++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > index 32bb04b4d9d9..df5107d7b3fd 100644 > --- a/fs/sysfs/file.c > +++ b/fs/sysfs/file.c > @@ -570,6 +570,46 @@ static int internal_change_owner(struct kernfs_node *kn, struct kobject *kobj, > return kernfs_setattr(kn, &newattrs); > } > > +/** > + * sysfs_link_change_owner - change owner of a link. > + * @kobj: object of the kernfs_node the symlink is located in. > + * @targ: object of the kernfs_node the symlink points to. > + * @name: name of the link. > + * @kuid: new owner's kuid > + * @kgid: new owner's kgid > + */ > +int sysfs_link_change_owner(struct kobject *kobj, struct kobject *targ, > + const char *name, kuid_t kuid, kgid_t kgid) > +{ > + struct kernfs_node *parent, *kn = NULL; > + int error; > + > + if (!kobj) > + parent = sysfs_root_kn; > + else > + parent = kobj->sd; I don't understand this, why would (!kobj) ever be a valid situation? > + if (!targ->state_in_sysfs) > + return -EINVAL; Should you also check kobj->state_in_sysfs as well? thanks, greg k-h
On Thu, Feb 20, 2020 at 12:14:43PM +0100, Greg Kroah-Hartman wrote: > On Tue, Feb 18, 2020 at 05:29:36PM +0100, Christian Brauner wrote: > > Add a helper to change the owner of a sysfs link. > > 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. > > --- > > fs/sysfs/file.c | 40 ++++++++++++++++++++++++++++++++++++++++ > > include/linux/sysfs.h | 10 ++++++++++ > > 2 files changed, 50 insertions(+) > > > > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > > index 32bb04b4d9d9..df5107d7b3fd 100644 > > --- a/fs/sysfs/file.c > > +++ b/fs/sysfs/file.c > > @@ -570,6 +570,46 @@ static int internal_change_owner(struct kernfs_node *kn, struct kobject *kobj, > > return kernfs_setattr(kn, &newattrs); > > } > > > > +/** > > + * sysfs_link_change_owner - change owner of a link. > > + * @kobj: object of the kernfs_node the symlink is located in. > > + * @targ: object of the kernfs_node the symlink points to. > > + * @name: name of the link. > > + * @kuid: new owner's kuid > > + * @kgid: new owner's kgid > > + */ > > +int sysfs_link_change_owner(struct kobject *kobj, struct kobject *targ, > > + const char *name, kuid_t kuid, kgid_t kgid) > > +{ > > + struct kernfs_node *parent, *kn = NULL; > > + int error; > > + > > + if (!kobj) > > + parent = sysfs_root_kn; > > + else > > + parent = kobj->sd; > > I don't understand this, why would (!kobj) ever be a valid situation? Yeah, a caller could just pass in "sysfs_root_kn" itself if they for some reason needed to. > > > + if (!targ->state_in_sysfs) > > + return -EINVAL; > > Should you also check kobj->state_in_sysfs as well? Probably. I'll take a closer look now. Thanks! Christian
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 32bb04b4d9d9..df5107d7b3fd 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -570,6 +570,46 @@ static int internal_change_owner(struct kernfs_node *kn, struct kobject *kobj, return kernfs_setattr(kn, &newattrs); } +/** + * sysfs_link_change_owner - change owner of a link. + * @kobj: object of the kernfs_node the symlink is located in. + * @targ: object of the kernfs_node the symlink points to. + * @name: name of the link. + * @kuid: new owner's kuid + * @kgid: new owner's kgid + */ +int sysfs_link_change_owner(struct kobject *kobj, struct kobject *targ, + const char *name, kuid_t kuid, kgid_t kgid) +{ + struct kernfs_node *parent, *kn = NULL; + int error; + + if (!kobj) + parent = sysfs_root_kn; + else + parent = kobj->sd; + + if (!targ->state_in_sysfs) + return -EINVAL; + + error = -ENOENT; + kn = kernfs_find_and_get_ns(parent, name, targ->sd->ns); + if (!kn) + goto out; + + error = -EINVAL; + if (kernfs_type(kn) != KERNFS_LINK) + goto out; + if (kn->symlink.target_kn->priv != targ) + goto out; + + error = internal_change_owner(kn, targ, kuid, kgid); + +out: + kernfs_put(kn); + return error; +} + /** * sysfs_file_change_owner_by_name - change owner of a file. * @kobj: object. diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index c11d11c78713..899500950410 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -313,6 +313,8 @@ static inline void sysfs_enable_ns(struct kernfs_node *kn) int sysfs_file_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid); 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); #else /* CONFIG_SYSFS */ @@ -539,6 +541,14 @@ static inline int sysfs_file_change_owner_by_name(struct kobject *kobj, return 0; } +static inline int sysfs_link_change_owner(struct kobject *kobj, + struct kobject *targ, + const char *name, kuid_t kuid, + kgid_t kgid) +{ + return 0; +} + #endif /* CONFIG_SYSFS */ static inline int __must_check sysfs_create_file(struct kobject *kobj,
Add a helper to change the owner of a sysfs link. 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. --- fs/sysfs/file.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/linux/sysfs.h | 10 ++++++++++ 2 files changed, 50 insertions(+)