Re: [PATCH] bitmap: fix bitmap_find_free_region()

[Johannes Weiner <hannes@cmpxchg.org>]

On Fri, Dec 19, 2008 at 02:18:22PM +0100, Guennadi Liakhovetski wrote:
> On Fri, 19 Dec 2008, Johannes Weiner wrote:
> 
> > On Fri, Dec 19, 2008 at 12:26:41PM +0100, Guennadi Liakhovetski wrote:
> > > Currently bitmap_find_free_region() assumes, that the requested region 
> > > size is smaller than the entire bitmap. If this is not the case it fails 
> > > to detect it and returns success, while pointing at a position outside of 
> > > the region.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <lg@denx.de>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > ---
> > > It is hard to believe, that this is a bug, last time this code was touched 
> > > in 2006... Or should the caller guarantee, that the requested region is 
> > > not larger than the bitmap? Then dma_alloc_from_coherent() is buggy, which 
> > > is where I hit this bug. But it seems to me bitmap_find_free_region() 
> > > should be fixed.
> > > 
> > > diff --git a/lib/bitmap.c b/lib/bitmap.c
> > > index 1338469..079c5e3 100644
> > > --- a/lib/bitmap.c
> > > +++ b/lib/bitmap.c
> > > @@ -950,6 +950,9 @@ int bitmap_find_free_region(unsigned long *bitmap, int bits, int order)
> > >  {
> > >  	int pos;		/* scans bitmap by regions of size order */
> > >  
> > > +	if (bits < 1 << order)
> > > +		return -ENOMEM;
> > 
> > I think this is an error on the callsite, so a WARN_ON() might be good
> > to spot these.  I would be interested in the callpath that triggered
> > this bug for you.
> 
> As I said above it's called from dma_alloc_from_coherent() from 
> dma_alloc_coherent().

Sure, but someone must have done dma_declare_coherent_memory() before
with a size smaller than it later passed to dma_alloc_coherent(), no?

I think we should add the check, WARN if it's true and return an
appropriate error number.  It will be handled gracefully then and we
still know which callsite screwed up.

	Hannes