linux-kernel - Re: [PATCH] bitmap: fix bitmap_find_free

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]

Message-ID: <20081219224532.GA2581@cmpxchg.org>
Date:	Fri, 19 Dec 2008 23:45:33 +0100
From:	Johannes Weiner <hannes@...xchg.org>
To:	Guennadi Liakhovetski <lg@...x.de>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] bitmap: fix bitmap_find_free_region()

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@...x.de>
Signed-off-by: Johannes Weiner <hannes@...xchg.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);
--
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/