netdev - Re: [PATCH net-next v3 3/9] sysfs: add sysfs_group{s}_change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Feb 2020 12:15:50 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Pavel Machek <pavel@....cz>, Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH net-next v3 3/9] sysfs: add sysfs_group{s}_change_owner()

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@...ntu.com>
> ---
> /* v2 */
> -  Greg Kroah-Hartman <gregkh@...uxfoundation.org>:
>    - Add comment how ownership of sysfs object is changed.
> 
> /* v3 */
> -  Greg Kroah-Hartman <gregkh@...uxfoundation.org>:
>    - Add explicit uid/gid parameters.
> - Christian Brauner <christian.brauner@...ntu.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@...uxfoundation.org>