linux-kernel - Re: [PATCH v2] zram: fix possible use after free in zcomp

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:	Tue, 8 Sep 2015 10:34:29 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:	Luis Henriques <luis.henriques@...onical.com>,
	Nitin Gupta <ngupta@...are.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] zram: fix possible use after free in zcomp_create()

On Tue, Sep 08, 2015 at 10:20:38AM +0900, Sergey Senozhatsky wrote:
> On (09/08/15 10:07), Minchan Kim wrote:
> [..]
> > > +	int ret;
> > 
> > For the clarification, I want to call it as 'error' instead of ret.
> > 
> > >  
> > >  	backend = find_backend(compress);
> > >  	if (!backend)
> > > @@ -347,10 +348,10 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> > >  
> > >  	comp->backend = backend;
> > >  	if (max_strm > 1)
> > > -		zcomp_strm_multi_create(comp, max_strm);
> > > +		ret = zcomp_strm_multi_create(comp, max_strm);
> > >  	else
> > > -		zcomp_strm_single_create(comp);
> > > -	if (!comp->stream) {
> > > +		ret = zcomp_strm_single_create(comp);
> > > +	if (ret) {
> > >  		kfree(comp);
> > >  		return ERR_PTR(-ENOMEM);
> > >  	}
> > 
> > And we could return ERR_PTR(error) rather than fixed -ENOMEM to propagate
> 
> yep, I thought about that; looks to be minor so I didn't insist
> on changing that. don't mind to change it to ERR_PTR(error).

Thanks.

> 
> > other errors potentially could be happen in future(ex, crypto support).
> > Of course, we should change description of the function about error return.
> 
> on the other hand, this is zcomp_strm_FOO_create(), which is mostly
> about memory allocation. but yeah, who knows. We can change this as
> a part of crypto api rework; ERR_PTR(error) does not fix anything
> per se, so may be we can avoid pushing this change into stable.

Strictly speaking, you're right. I asked two things bug fixing and
refactoring. But I thought it is really trivial enough to push
-stable.

With bonus, it's more readable(ie, error naming) and makes sense(
ie, not assume under layer function always fails with no memory).

So, hope to fix it altogether if anyone isn't strong against that.

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/