linux-kernel - Re: [PATCH] regmap: do not call regmap_debugfs_init() from regmap_attach

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: <20210726184805.GK4670@sirena.org.uk>
Date:   Mon, 26 Jul 2021 19:48:05 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Matthias Schiffer <matthias.schiffer@...tq-group.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        David Lechner <david@...hnology.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] regmap: do not call regmap_debugfs_init() from
 regmap_attach_dev()

On Mon, Jul 26, 2021 at 02:18:42PM +0200, Matthias Schiffer wrote:
> On Mon, 2021-07-26 at 13:11 +0100, Mark Brown wrote:

> > That's not what your patch says it's fixing, your patch says it's
> > fixing an attempt to recreate the same directory as we had originally
> > (we should probably clean up the one with no device but that's not what
> > your commit does).  I think what you need to look at here is that we
> > store map->debugfs_name and don't overwrite it when the device is
> > supplied.

> That would be fine if regmap_debugfs_init() didn't do a lot more than
> just create the debugfs directory. I'm more concerned about the mutex

The whole point here is to move the debugfs directory so if any fix
stops that happening it's not really viable.  If we knew that devices
were definitely going to have a device bound we could just defer till
the device is bound but it's not clear to me that that will always
happen.

> and list head initialization that is happening on an already
> initialized structure. I haven't looked in detail what the mutex and
> list head are used for, but I assume bad things™ are going to happen
> when someone is already holding the mutex or using the list.

They're used to cache information on where registers are located in the
debugfs files so seeks work much faster on large register maps, they
won't be doing anything if userspace isn't up yet which should really be
the case for anything that's initializing early enough that it needed to
have a regmap prior to the driver model being up.  You're right that
there is a potential issue there though, but that can be handled
separately.

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