netdev - Re: [PATCH net-next] mctp: Add MCTP-over-serial transport binding

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: <YZuXGBdRpAXTfONP@casper.infradead.org>
Date:   Mon, 22 Nov 2021 13:11:52 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     Jeremy Kerr <jk@...econstruct.com.au>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        netdev@...r.kernel.org, Matt Johnston <matt@...econstruct.com.au>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jiri Slaby <jirislaby@...nel.org>
Subject: Re: [PATCH net-next] mctp: Add MCTP-over-serial transport binding

On Mon, Nov 22, 2021 at 04:23:10PM +0800, Jeremy Kerr wrote:
> Hi Greg,
> 
> > ida_destroy() will not be a no-op if you have allocated some things
> > in the past.  It should always be called when your module is removed.
> > 
> > Or at least that is how it used to be, if this has changed in the
> > past year, then I am mistaken here.

I think Greg is remembering how the IDA behaved before it was converted
to use the radix tree back in 2016 (0a835c4f090a).  About two-thirds
of the users of the IDA and IDR forgot to call ida_destroy/idr_destroy,
so rather than fix those places, I decided to make those data structures
no longer require a destructor.

> I was going by this bit of the comment on ida_destroy:
> 
>    * Calling this function frees all IDs and releases all resources used
>    * by an IDA.  When this call returns, the IDA is empty and can be reused
>    * or freed.  If the IDA is already empty, there is no need to call this
>    * function.
> 
> [From a documentation improvement in 50d97d50715]
> 
> Looking at ida_destroy, it's iterating the xarray and freeing all !value
> entries. ida_free will free a (allocated) value entry once all bits are
> clear, so the comment looks correct to me - there's nothing left to free
> if the ida is empty.
> 
> However, I'm definitely no ida/idr/xarray expert! Happy to be corrected
> here - and I'll send a patch to clarify that comment too, if so.
> 
> Cheers,
> 
> 
> Jeremy
>