Date:   Thu, 6 Jan 2022 18:33:52 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Fabio Estevam <festevam@gmail.com>
Cc:     matthias.schiffer@ew.tq-group.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] regmap: debugfs: Free debugfs_name buffer after usage
Message-ID: <Ydc2EHf5f12w4YcW@sirena.org.uk>
References: <20220106175019.3116389-1-festevam@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="oNEgbt41o7xMIY3M"
Content-Disposition: inline
In-Reply-To: <20220106175019.3116389-1-festevam@gmail.com>
Precedence: bulk


--oNEgbt41o7xMIY3M
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Jan 06, 2022 at 02:50:19PM -0300, Fabio Estevam wrote:

> The reason for the duplicate name is that map->debugfs_name is never freed,
> which can cause a directory to be created with the same name used in the
> previous debugfs entry allocation.

> Fix this problem by freeing map->debugfs_name and setting it to NULL
> after its usage.

OK, but what's the logic here?  The name is getting thrown away here but
clearly there is a file still so I'm not seeing how anything is going to
clean that file up.  That means that if the device gets freed we'll end
up with the old debugfs file hanging around pointing at nothing.  Like I
said (originally in response to Matthias' patch but pasted in this
thread as well):

| (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

The use after free extends beyond just the filename, we're also loosing
track of the already created file, which does seem to be an existing
bug.  To be more explicit this means we need a call to regmap_debugfs_exit()
which will clean up all the existing debugfs stuff before we loose
references to it.

--oNEgbt41o7xMIY3M
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmHXNg8ACgkQJNaLcl1U
h9AczQf+MWrIXa2mSNXhkoiFt3mCkFFC/wxFGchHjw6pGzL66+e5JdeHDqa2EjWH
WjDlxHsv+5zWF6tiJT4c5oNZNE3MYpHp63SMkIKnlCow4KLYcuTQtk3Lxz+nPzGF
PoMiIPG387pXVxP1DvNfzOHqXfLXPllVHs2AlEKPTpd7v2gAgvexZjMnVD9SMBb2
7hMSdaroeULHWTTlvnuI+b3mEV2OmP1A8ApiPwByaSANgLBtnIg5wyrzWMJ6CRXM
yxSDc0krbxQnaBAdGBAZN//R3KLU5h4c55gDu5NlK4GnxKEgcY0l4adfyL9in9ZL
V9xV/7AMSR/pVKhu8lZfB5TNiLe+4Q==
=qp/v
-----END PGP SIGNATURE-----

--oNEgbt41o7xMIY3M--