linux-kernel - Re: [PATCH] mcb: fix error handling in mcb_alloc

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210806133700.GP22532@kadam>
Date:   Fri, 6 Aug 2021 16:37:00 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Johannes Thumshirn <jth@...nel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andreas Werner <andreas.werner@....de>,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] mcb: fix error handling in mcb_alloc_bus()

On Fri, Aug 06, 2021 at 04:32:40PM +0300, Dan Carpenter wrote:
> On Tue, Jun 01, 2021 at 03:23:55PM +0200, Johannes Thumshirn wrote:
> > 
> > 
> > Am 01.06.21 um 11:18 schrieb Dan Carpenter:
> > > There are two bugs:
> > > 1) If ida_simple_get() fails then this code calls put_device(carrier)
> > >    but we haven't yet called get_device(carrier) and probably that
> > >    leads to a use after free.
> > > 2) After device_initialize() then we need to use put_device() to
> > >    release the bus.  This will free the internal resources tied to the
> > >    device and call mcb_free_bus() which will free the rest.
> > >
> > > Fixes: 5d9e2ab9fea4 ("mcb: Implement bus->dev.release callback")
> > > Fixes: 18d288198099 ("mcb: Correctly initialize the bus's device")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
> > >
> > 
> > Thanks applied
> 
> I don't think this was actually applied and that's a good thing because
> my patch wasn't correct.  It needs to call device_put() on both
> carrier and &bus->dev.
> 
> I'm going to send a v2 fix for this.

Actually, no...  My first patch was correct.  Calling put_device(&bus->dev);
will call the release function which will put the carrier.  My first
patch was correct.

:P

regards,
dan carpenter