linux-kernel - Re: [PATCH] mwifiex: don't do unbalanced free()'ing in cleanup

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]

Date:   Wed, 26 Oct 2016 16:52:03 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Brian Norris <briannorris@...omium.org>
Cc:     Amitkumar Karwar <akarwar@...vell.com>,
        Nishant Sarmukadam <nishants@...vell.com>,
        linux-kernel@...r.kernel.org, Kalle Valo <kvalo@...eaurora.org>,
        linux-wireless@...r.kernel.org, Cathy Luo <cluo@...vell.com>,
        Rajat Jain <rajatja@...gle.com>
Subject: Re: [PATCH] mwifiex: don't do unbalanced free()'ing in cleanup_if()

On Wed, Oct 26, 2016 at 04:43:54PM -0700, Brian Norris wrote:
> On Wed, Oct 26, 2016 at 04:35:54PM -0700, Dmitry Torokhov wrote:
> > On Wed, Oct 26, 2016 at 04:29:20PM -0700, Brian Norris wrote:
> 
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > > index 8718950004f3..f04cf5a551b3 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> 
> > > @@ -2291,6 +2287,14 @@ static void mwifiex_recreate_adapter(struct sdio_mmc_card *card)
> > >  
> > >  	mwifiex_sdio_remove(func);
> > >  
> > > +	/*
> > > +	 * Normally, we would let the driver core take care of releasing these.
> > > +	 * But we're not letting the driver core handle this one. See above
> > > +	 * TODO.
> > > +	 */
> > > +	sdio_set_drvdata(func, NULL);
> > > +	devm_kfree(&func->dev, card);
> > 
> > Ugh, this really messes the unwind order... I guess it is OK since it is
> > the only resource allocated with devm, but I'd be happier if we could
> > reuse existing "card" structure.
> 
> I'm really not interested in cleaning up the hacky reset function here
> (see the other TODOs here). I'm sure it's broken in other ways too. In
> its current "design" (if you can call it that) where we remove and
> re-probe the device, I'm not sure there's a way to get it to reuse the
> 'card'.

Ah, I see now... Nevermind then.

Thanks.

-- 
Dmitry