linux-kernel - Re: [PATCH 4/7] sysfs: Implement sysfs_rename

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]

Message-ID: <20100112173030.GA13506@us.ibm.com>
Date:	Tue, 12 Jan 2010 11:30:30 -0600
From:	"Serge E. Hallyn" <serue@...ibm.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>,
	Kay Sievers <kay.sievers@...y.org>,
	linux-kernel@...r.kernel.org, Tejun Heo <tj@...nel.org>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	linux-fsdevel@...r.kernel.org,
	Eric Dumazet <eric.dumazet@...il.com>,
	Benjamin LaHaise <bcrl@...et.ca>,
	Benjamin Thery <benjamin.thery@...l.net>,
	Daniel Lezcano <dlezcano@...ibm.com>,
	"Eric W. Biederman" <ebiederm@...stanetworks.com>
Subject: Re: [PATCH 4/7] sysfs: Implement sysfs_rename_link

Quoting Eric W. Biederman (ebiederm@...ssion.com):
> From: Eric W. Biederman <ebiederm@...ssion.com>
> 
> Because of rename ordering problems we occassionally give false
> warnings about invalid sysfs operations.  So using sysfs_rename
> create a sysfs_rename_link function that doesn't need strange
> workarounds.

It looks nice and clean esp compared to the code you're replacing.
OTOH I do wonder whether your sysfs_type(sd) != SYSFS_KOBJ_LINK check
will catch unexpected ways that device_rename() was being called.
Shouldn't, I realize, since sysfs_create_link() always sets SYSFS_KOBJ_LINK,
but the check wasn't there before, so stranger things have happened.

> Cc: Benjamin Thery <benjamin.thery@...l.net>
> Cc: Daniel Lezcano <dlezcano@...ibm.com>
> Cc: Tejun Heo <tj@...nel.org>
> Signed-off-by: Eric W. Biederman <ebiederm@...stanetworks.com>

Acked-by: Serge Hallyn <serue@...ibm.com>

> ---
>  fs/sysfs/symlink.c    |   38 ++++++++++++++++++++++++++++++++++++++
>  include/linux/sysfs.h |    9 +++++++++
>  2 files changed, 47 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
> index c5eff49..1b9a3a1 100644
> --- a/fs/sysfs/symlink.c
> +++ b/fs/sysfs/symlink.c
> @@ -123,6 +123,44 @@ void sysfs_remove_link(struct kobject * kobj, const char * name)
>  	sysfs_hash_and_remove(parent_sd, name);
>  }
> 
> +/**
> + *	sysfs_rename_link - rename symlink in object's directory.
> + *	@kobj:	object we're acting for.
> + *	@targ:	object we're pointing to.
> + *	@old:	previous name of the symlink.
> + *	@new:	new name of the symlink.
> + *
> + *	A helper function for the common rename symlink idiom.
> + */
> +int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
> +			const char *old, const char *new)
> +{
> +	struct sysfs_dirent *parent_sd, *sd = NULL;
> +	int result;
> +
> +	if (!kobj)
> +		parent_sd = &sysfs_root;
> +	else
> +		parent_sd = kobj->sd;
> +
> +	result = -ENOENT;
> +	sd = sysfs_get_dirent(parent_sd, old);
> +	if (!sd)
> +		goto out;
> +
> +	result = -EINVAL;
> +	if (sysfs_type(sd) != SYSFS_KOBJ_LINK)
> +		goto out;
> +	if (sd->s_symlink.target_sd->s_dir.kobj != targ)
> +		goto out;
> +
> +	result = sysfs_rename(sd, parent_sd, new);
> +
> +out:
> +	sysfs_put(sd);
> +	return result;
> +}
> +
>  static int sysfs_get_target_path(struct sysfs_dirent *parent_sd,
>  				 struct sysfs_dirent *target_sd, char *path)
>  {
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index cfa8308..e322573 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -110,6 +110,9 @@ int __must_check sysfs_create_link_nowarn(struct kobject *kobj,
>  					  const char *name);
>  void sysfs_remove_link(struct kobject *kobj, const char *name);
> 
> +int sysfs_rename_link(struct kobject *kobj, struct kobject *target,
> +			const char *old_name, const char *new_name);
> +
>  int __must_check sysfs_create_group(struct kobject *kobj,
>  				    const struct attribute_group *grp);
>  int sysfs_update_group(struct kobject *kobj,
> @@ -203,6 +206,12 @@ static inline void sysfs_remove_link(struct kobject *kobj, const char *name)
>  {
>  }
> 
> +static inline int sysfs_rename_link(struct kobject *k, struct kobject *t,
> +				    const char *old_name, const char *new_name)
> +{
> +	return 0;
> +}
> +
>  static inline int sysfs_create_group(struct kobject *kobj,
>  				     const struct attribute_group *grp)
>  {
> -- 
> 1.6.5.2.143.g8cc62
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/