Re: [PATCH 2.6.17-rc6 7/9] Remove some of the kmemleak false positives

[Catalin Marinas]

On 24/06/06, Ingo Molnar <mingo@xxxxxxx> wrote:
> * Catalin Marinas <catalin.marinas@xxxxxxxxx> wrote:
> > To the other extreme is Ingo's suggestion of using exact type
> > identification but I don't think this would be acceptable for the
> > kernel as it would to modify all the memory alloc calls in the kernel
> > to either pass an additional parameter (the type id) or another
> > post-allocation call to kmemleak to update the id.
>
> passing in the type ID wouldnt be that bad and it would have other
> advantages as well: for example we could do strict type-checking of
> allocation size versus type-we-use-it-for.

There are valid places in the kernel where the allocated size is
different from the type's one. That's why I added the
memleak_padding().

> As long as the conversion is gradual i think we could try this. I.e.
> we'd default to 'no ID passed', and in that case we would fall back to
> the size-based method and generate an ID out of the structure size.

OK, I'll try to add the infrastructure for type ids but default to
sizeof initially.

> > Anyway, the current implementation (I'll update it for 2.6.17) detects
> > real memory leaks. I suspect that a wide range of leaks would be
> > covered if it is used on different platforms and different conditions.
>
> btw., what leaks were found so far? I know about the ACPI one - any
> other ones?

There are two leaks reported in ACPI but I only posted a patch for one
as I didn't have time to track the other (a reference count doesn't
get to zero and the structure not freed).

There is another leak in legacy_init_iomem_resources() in
arch/i386/setup.c (request_resource() fails but the memory is not
freed - see the attached patch). I initially marked this as a false
positive but it wasn't (I have to revisit the false positives).

--
Catalin
Fix a memory leak in the i386 setup code

From: Catalin Marinas <catalin.marinas@xxxxxxxxx>

Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxxxx>
---

 arch/i386/kernel/setup.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/i386/kernel/setup.c b/arch/i386/kernel/setup.c
index dd6b0e3..60f5d99 100644
--- a/arch/i386/kernel/setup.c
+++ b/arch/i386/kernel/setup.c
@@ -1332,7 +1332,10 @@ legacy_init_iomem_resources(struct resou
 		res->start = e820.map[i].addr;
 		res->end = res->start + e820.map[i].size - 1;
 		res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
-		request_resource(&iomem_resource, res);
+		if (request_resource(&iomem_resource, res)) {
+			kfree(res);
+			continue;
+		}
 		if (e820.map[i].type == E820_RAM) {
 			/*
 			 *  We don't know which RAM region contains kernel data,