Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 5 Jun 2019 08:21:59 -0400
From:   Mike Snitzer <snitzer@redhat.com>
To:     Jiri Slaby <jslaby@suse.cz>
Cc:     Gen Zhang <blackgod016574@gmail.com>, agk@redhat.com,
        dm-devel@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: dm-region-hash: Fix a missing-check bug in __rh_alloc()
Message-ID: <20190605122159.GA32538@redhat.com>
References: <20190524031248.GA6295@zhanggen-UX430UQ>
 <79ec221d-6970-3b30-0660-4a288a4c465e@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <79ec221d-6970-3b30-0660-4a288a4c465e@suse.cz>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jun 05 2019 at  2:05am -0400,
Jiri Slaby <jslaby@suse.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@gmail.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