Messages in this thread | | | Date | Thu, 20 Feb 2020 12:25:13 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH net-next v3 5/9] device: add device_change_owner() |
| |
On Tue, Feb 18, 2020 at 05:29:39PM +0100, Christian Brauner wrote: > Add a helper to change the owner of a device's sysfs entries. This > needs to happen when the ownership of a device is changed, e.g. when > moving network devices between network namespaces. > This function will be used to correctly account for ownership changes, > e.g. when moving network devices between network namespaces. > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > --- > /* v2 */ > unchanged > > /* v3 */ > - Greg Kroah-Hartman <gregkh@linuxfoundation.org>: > - Add explicit uid/gid parameters. > --- > drivers/base/core.c | 80 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/device.h | 1 + > 2 files changed, 81 insertions(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 42a672456432..ec0d5e8cfd0f 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -3458,6 +3458,86 @@ int device_move(struct device *dev, struct device *new_parent, > } > EXPORT_SYMBOL_GPL(device_move); > > +static int device_attrs_change_owner(struct device *dev, kuid_t kuid, > + kgid_t kgid) > +{ > + struct kobject *kobj = &dev->kobj; > + struct class *class = dev->class; > + const struct device_type *type = dev->type; > + int error; > + > + if (class) { > + error = sysfs_groups_change_owner(kobj, class->dev_groups, kuid, > + kgid); > + if (error) > + return error; > + } > + > + if (type) { > + error = sysfs_groups_change_owner(kobj, type->groups, kuid, > + kgid); > + if (error) > + return error; > + } > + > + error = sysfs_groups_change_owner(kobj, dev->groups, kuid, kgid); > + if (error) > + return error; > + > + if (device_supports_offline(dev) && !dev->offline_disabled) { > + error = sysfs_file_change_owner_by_name( > + kobj, dev_attr_online.attr.name, kuid, kgid); > + if (error) > + return error; > + } > + > + return 0; > +} > + > +/** > + * device_change_owner - change the owner of an existing device.
The "owner" and what else gets changed here? Please document this better.
> + * @dev: device. > + * @kuid: new owner's kuid > + * @kgid: new owner's kgid > + */ > +int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid) > +{ > + int error; > + struct kobject *kobj = &dev->kobj; > + > + dev = get_device(dev); > + if (!dev) > + return -EINVAL; > + > + error = sysfs_change_owner(kobj, kuid, kgid);
the kobject of the device is changed, good.
> + if (error) > + goto out; > + > + error = sysfs_file_change_owner_by_name(kobj, dev_attr_uevent.attr.name, > + kuid, kgid);
Why call out the uevent file explicitly here?
> + if (error) > + goto out; > + > + error = device_attrs_change_owner(dev, kuid, kgid); > + if (error) > + goto out;
Doesn't this also change the uevent file?
> + > +#ifdef CONFIG_BLOCK > + if (sysfs_deprecated && dev->class == &block_class) > + goto out; > +#endif
Ugh, we still need this?
> + > + error = sysfs_link_change_owner(&dev->class->p->subsys.kobj, &dev->kobj, > + dev_name(dev), kuid, kgid);
Now what is this changing?
Again, more documentation please as to exactly what is being changed in this function is needed.
thanks,
greg k-h
| |