[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2024071756-uproot-relieve-6e27@gregkh>
Date: Wed, 17 Jul 2024 17:03:37 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Benjamin Thery <benjamin.thery@...l.net>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Greg Kroah-Hartman <gregkh@...e.de>, linux-kernel@...r.kernel.org,
Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH] driver core: Fix error handling in driver API
device_rename()
On Wed, Jul 17, 2024 at 10:50:05PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@...cinc.com>
>
> Call failure of device_rename(@dev, @new_name) maybe unexpectedly change
> link name within @dev's class directory to @new_name, fixed by correcting
> error handling for the API.
I'm sorry, but I don't understand the text here, what exactly are you
doing?
> Fixes: f349cf34731c ("driver core: Implement ns directory support for device classes.")
> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> ---
> drivers/base/core.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 2b4c0624b704..a05b7186cf33 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4512,9 +4512,11 @@ EXPORT_SYMBOL_GPL(device_destroy);
> */
> int device_rename(struct device *dev, const char *new_name)
> {
> + struct subsys_private *sp = NULL;
> struct kobject *kobj = &dev->kobj;
> char *old_device_name = NULL;
> int error;
> + bool is_link_renamed = false;
>
> dev = get_device(dev);
> if (!dev)
> @@ -4529,7 +4531,7 @@ int device_rename(struct device *dev, const char *new_name)
> }
>
> if (dev->class) {
> - struct subsys_private *sp = class_to_subsys(dev->class);
> + sp = class_to_subsys(dev->class);
>
> if (!sp) {
> error = -EINVAL;
> @@ -4537,17 +4539,20 @@ int device_rename(struct device *dev, const char *new_name)
> }
>
> error = sysfs_rename_link_ns(&sp->subsys.kobj, kobj, old_device_name,
> - new_name, kobject_namespace(kobj));
> - subsys_put(sp);
> + new_name, kobject_namespace(kobj));
Why did you change the indentation here?
> if (error)
> goto out;
> +
> + is_link_renamed = true;
> }
>
> error = kobject_rename(kobj, new_name);
> - if (error)
> - goto out;
> -
> out:
> + if (error && is_link_renamed)
> + sysfs_rename_link_ns(&sp->subsys.kobj, kobj, new_name,
> + old_device_name, kobject_namespace(kobj));
> + subsys_put(sp);
How was this found? What in-kernel code causes this problem? And how
was this tested?
thanks,
greg k-h
Powered by blists - more mailing lists