linux-kernel - Re: dm-region-hash: Fix a missing-check bug in __rh

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: <20190605145820.GA3465@zhanggen-UX430UQ>
Date:   Wed, 5 Jun 2019 22:58:20 +0800
From:   Gen Zhang <blackgod016574@...il.com>
To:     Mike Snitzer <snitzer@...hat.com>
Cc:     Jiri Slaby <jslaby@...e.cz>, agk@...hat.com, dm-devel@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: dm-region-hash: Fix a missing-check bug in __rh_alloc()

On Wed, Jun 05, 2019 at 08:21:59AM -0400, Mike Snitzer wrote:
> On Wed, Jun 05 2019 at  2:05am -0400,
> Jiri Slaby <jslaby@...e.cz> wrote:
> 
> > On 24. 05. 19, 5:12, Gen Zhang wrote:
> > > In function __rh_alloc(), the pointer nreg is allocated a memory space
> > > via kmalloc(). And it is used in the following codes. However, when 
> > > there is a memory allocation error, kmalloc() fails. Thus null pointer
> > > dereference may happen. And it will cause the kernel to crash. Therefore,
> > > we should check the return value and handle the error.
> > > Further, in __rh_find(), we should also check the return value and
> > > handle the error.
> > > 
> > > Signed-off-by: Gen Zhang <blackgod016574@...il.com>
> > > 
> > > ---
> > > diff --git a/drivers/md/dm-region-hash.c b/drivers/md/dm-region-hash.c
> > > index 1f76045..2fa1641 100644
> > > --- a/drivers/md/dm-region-hash.c
> > > +++ b/drivers/md/dm-region-hash.c
> > > @@ -290,8 +290,11 @@ static struct dm_region *__rh_alloc(struct dm_region_hash *rh, region_t region)
> > >  	struct dm_region *reg, *nreg;
> > >  
> > >  	nreg = mempool_alloc(&rh->region_pool, GFP_ATOMIC);
> > > -	if (unlikely(!nreg))
> > > +	if (unlikely(!nreg)) {
> > >  		nreg = kmalloc(sizeof(*nreg), GFP_NOIO | __GFP_NOFAIL);
> > > +		if (!nreg)
> > > +			return NULL;
> > 
> > What's the purpose of checking NO_FAIL allocations?
> 
> There isn't, that was already pointed out in a different thread for this
> same patch (think patch was posted twice):
> https://www.redhat.com/archives/dm-devel/2019-May/msg00124.html
> 
> Mike
Thanks for your reply. The first thread is not replied for a period, so
the second one is posted. But I don't know why Jiri replied to the first
thread.

Thanks
Gen