linux-kernel - Re: [PATCH 2/2] drivers/base: fix iterator cleanup in attribute_container_find_class

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] [day] [month] [year] [list]

Message-ID: <2025022819-pushing-virtual-9b85@gregkh>
Date: Fri, 28 Feb 2025 23:34:16 -0800
From: Greg KH <gregkh@...uxfoundation.org>
To: Ayaan Mirza Baig <ayaanmirzabaig85@...il.com>
Cc: ayaanmirza85@...il.com, dakr@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] drivers/base: fix iterator cleanup in
 attribute_container_find_class_device (v2)

On Thu, Feb 27, 2025 at 06:11:29PM +0530, Ayaan Mirza Baig wrote:
> This patch fixes a long-standing FIXME by refactoring
> the function to ensure klist_iter_exit() is always
> invoked once via a unified cleanup path
> 
> The patch has been tested thoroughly with a minimal
> Debain System and scsi devices and GDB.
> 
> Tested-by: Ayaan Mirza Baig <ayaanmirzabaig85@...il.com>
> Signed-off-by: Ayaan Mirza Baig <ayaanmirzabaig85@...il.com>

No need to duplicate this here, if you sign-off it's implicit that you
tested it :)

> ---
>  drivers/base/attribute_container.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/attribute_container.c b/drivers/base/attribute_container.c
> index 69637b2ec3bc..fbf7fd45651e 100644
> --- a/drivers/base/attribute_container.c
> +++ b/drivers/base/attribute_container.c
> @@ -492,12 +492,11 @@ attribute_container_find_class_device(struct attribute_container *cont,
>  	klist_for_each_entry(ic, &cont->containers, node, &iter) {
>  		if (ic->classdev.parent == dev) {
>  			cdev = &ic->classdev;
> -			/* FIXME: must exit iterator then break */
> -			klist_iter_exit(&iter);
> -			break;
> +			goto out;
>  		}
>  	}
> -
> +out:
> +	klist_iter_exit(&iter);

Are you sure this actually does something different?  Why is the FIXME
there and why does this "simple" change solve the issue?  I don't
understand how this change resolves the problem.

In other words, you're going to have to document this a lot better in
the changelog message please.

thanks,

greg k-h