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 for Android: free password hash cracker in your pocket

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

Message-ID: <82872f70-f31e-4956-86e9-42c80771fbc5@sirena.org.uk>
Date: Wed, 5 Mar 2025 14:03:12 +0000
From: Mark Brown <broonie@...nel.org>
To: Zxyan Zhu <zxyan20@....com>
Cc: gregkh@...uxfoundation.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:27:18PM +0800, Zxyan Zhu wrote:
> The `dummy_index` global variable caused debugfs file name conflicts
> during re-entry, leading to creation failures. Use atomic operations
> to ensure safe and unique debugfs `dummy%d` naming.
> 
> Signed-off-by: Zxyan Zhu <zxyan20@....com>
> 
> Changes since v2:
> - Remove atomic_dec operation if kasprintf() fails

This should go after the --- as covered in submitting-patches.rst.

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.

> At 2025-03-05 00:23:02, "Greg KH" <gregkh@...uxfoundation.org> wrote:
> >On Tue, Mar 04, 2025 at 10:24:52PM +0800, Zxyan Zhu wrote:
> >>  	if (!strcmp(name, "dummy")) {

Please don't mix stuff like this into patch postings, the tooling can't
cope with stripping it out automatically.

> 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.
> 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.
> 3. As Mark mentioned, id continuity isn't a hard requirement, so we
>  can even skip the atomic_dec on kasprintf failure.

I don't think we care about cost for allocating new regmaps, if that's a
hot path we've probably got bigger problems.

> If you still think idr has clear advantages here, I'm happy to adjust
> the code, but it'd be great to understand the specific benefits in
> this context.

It's the standard idiom for this application.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)