Re: [PATCH] bitmap: fix bitmap_find_free_region()

[Johannes Weiner <hannes@cmpxchg.org>]

On Fri, Dec 19, 2008 at 04:09:42PM +0100, Guennadi Liakhovetski wrote:
> Yes, this is the i.MX31 video capture driver (patch has once been 
> submitted, new version in work). The thing is, we reserve and declare a 
> fixed size coherent region in the board code at start-up, but then what 
> the driver allocates depends on what a user-space application requests - 
> what size video frame and how many buffers. And the driver does not check 
> how much coherent memory it has available, it relies on 
> dma_alloc_coherent() to tell. In fact, the actual i.MX31 camera driver 
> knows nothing about this allocation, this all happens automatically in 
> soc_camera.c::soc_camera_mmap() -> 
> videobuf-dma-contig.c::__videobuf_mmap_mapper().
> 
> > 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.
> 
> Well, given above you'd get warning each time the user requests too big a 
> frame or too many buffers, which is not nice.

Indeed, that's BS then.

The region size information is local to the dma-coherent code.  So
either the callsite keeps track of the size itself or, if it can not
as in your case, we should check for the correct size on exactly that
layer of code, as well.  Does that sound reasonable?

Why do other driver not need this?  I will look further into it.  If
it's any use, here is a patch that does the check on the allocation
level.

---
dma-coherent: catch oversized requests to dma_alloc_from_coherent()

Prevent passing an order to bitmap_find_free_region() that is larger
than the actual bitmap can represent.

These requests can come from device drivers that have no idea how big
the dma region is and need to rely on dma_alloc_from_coherent() to
sort it out for them.

Reported-by: Guennadi Liakhovetski <lg@denx.de>
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---

diff --git a/kernel/dma-coherent.c b/kernel/dma-coherent.c
index f013a0c..56ea73e 100644
--- a/kernel/dma-coherent.c
+++ b/kernel/dma-coherent.c
@@ -112,6 +112,9 @@ int dma_alloc_from_coherent(struct device *dev, ssize_t size,
 	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
 	int order = get_order(size);
 
+	if (unlikely(size > mem->size))
+		return 0;
+
 	if (mem) {
 		int page = bitmap_find_free_region(mem->bitmap, mem->size,
 						     order);