linux-kernel - Re: [PATCH v3] regmap: debugfs: Fix name collision without atomic operations

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: <2025030531-spilt-duchess-0fa5@gregkh>
Date: Wed, 5 Mar 2025 15:22:20 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Zxyan Zhu <zxyan20@....com>
Cc: broonie@...nel.org, rafael@...nel.org, dakr@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] regmap: debugfs: Fix name collision without atomic
 operations

On Wed, Mar 05, 2025 at 09:32:52PM +0800, Zxyan Zhu wrote:
> Thanks for the feedback! I think using atomic_t is a better fit here
> than idr because:
> 1. It's lighter and simpler for our basic indexing needs.

No, idr, and ida (don't remember which is which for this use case, but
one of them is the right one) was explicitly written for this exact type
of functionality.

> 2. No extra resource management is needed. If we use idr, we'd have to
>  handle resource cleanup and add locks for debugfs_init/debugfs_exit
>  concurrency.

The idr has a lock in it, right?  Use that.

> 3. As Mark mentioned, id continuity isn't a hard requirement, so we
>  can even skip the atomic_dec on kasprintf failure.

Again, use the data structure designed for this very thing.  To ignore
that feels very odd.

Drivers should almost NEVER use atomic values, as there are other apis
to use instead that are correct.  If you find yourself using one, always
verify that you really should be doing that.

thanks,

greg k-h