linux-kernel - Re: [PATCH v3] mm/damon/dbgfs: avoid duplicate context directory creation

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: <YwR6dgz1LEWGQagq@kroah.com>
Date:   Tue, 23 Aug 2022 08:57:58 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     SeongJae Park <sj@...nel.org>
Cc:     akpm@...ux-foundation.org, badari.pulavarty@...el.com,
        damon@...ts.linux.dev, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v3] mm/damon/dbgfs: avoid duplicate context directory
 creation

On Mon, Aug 22, 2022 at 04:52:36PM +0000, SeongJae Park wrote:
> > >  	new_dir = debugfs_create_dir(name, root);
> > > +	/* Below check is required for a potential duplicated name case */
> > > +	if (IS_ERR(new_dir))
> > > +		return PTR_ERR(new_dir);
> > 
> > Did you just leak the memory allocated above this check?  It's hard to
> > determine as you are setting global variables.
> 
> We re-alloc the metadata arrays for context above for this new context, and we
> do not re-alloc those in this failure case.  So yes, the arrays will have one
> more item that not really needed and also not really will be used.
> 
> However, the variable for the array, 'dbgfs_nr_ctxs' is not increased here.
> Therefore, the arrays will be re-allocated to the proper size when this
> function or other function that re-alloc the arrays based on 'dbgfs_nr_ctxs'
> (For example, 'dbgfs_rm_context()') are called.
> 
> So, though the arrays could have not-really-needed one entry that is only waste
> of memory, as it's only a small and fixed amount of memory (just one more
> pointer), and as the unneeded memory will eventually be returned (by a next
> 'dbgfs_{mk,rm}_context()' call), I think that's no problem.  This is what I
> intended for keeping the logic simple.
> 
> If I'm missing something, please let me know, though.

Ah, that makes more sense, thanks.  The code was not obvious in that
error paths normally clean up allocations that were done earlier.

All is good.

thanks,

greg k-h