* [PATCH 1/1] kmemleak/module: only scan the existed data section
@ 2011-12-28 8:11 Tiejun Chen
2012-01-09 0:59 ` tiejun.chen
2012-01-09 12:05 ` Catalin Marinas
0 siblings, 2 replies; 5+ messages in thread
From: Tiejun Chen @ 2011-12-28 8:11 UTC (permalink / raw)
To: linux-kernel
We should only scan the sections containing data and it's size is not
zero as well.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
kernel/module.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 12cfa2b..0b93c30 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2045,6 +2045,8 @@ static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr,
if (strncmp(secstrings + sechdrs[i].sh_name, ".data", 5) != 0
&& strncmp(secstrings + sechdrs[i].sh_name, ".bss", 4) != 0)
continue;
+ if (sechdrs[i].sh_size == 0)
+ continue;
kmemleak_scan_area((void *)sechdrs[i].sh_addr,
sechdrs[i].sh_size, GFP_KERNEL);
--
1.6.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] kmemleak/module: only scan the existed data section
2011-12-28 8:11 [PATCH 1/1] kmemleak/module: only scan the existed data section Tiejun Chen
@ 2012-01-09 0:59 ` tiejun.chen
2012-01-09 12:05 ` Catalin Marinas
1 sibling, 0 replies; 5+ messages in thread
From: tiejun.chen @ 2012-01-09 0:59 UTC (permalink / raw)
To: catalin.marinas; +Cc: linux-kernel
Tiejun Chen wrote:
> We should only scan the sections containing data and it's size is not
> zero as well.
Catalin,
Could you take a look at this?
Thanks
Tiejun
>
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
> kernel/module.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 12cfa2b..0b93c30 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2045,6 +2045,8 @@ static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr,
> if (strncmp(secstrings + sechdrs[i].sh_name, ".data", 5) != 0
> && strncmp(secstrings + sechdrs[i].sh_name, ".bss", 4) != 0)
> continue;
> + if (sechdrs[i].sh_size == 0)
> + continue;
>
> kmemleak_scan_area((void *)sechdrs[i].sh_addr,
> sechdrs[i].sh_size, GFP_KERNEL);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] kmemleak/module: only scan the existed data section
2011-12-28 8:11 [PATCH 1/1] kmemleak/module: only scan the existed data section Tiejun Chen
2012-01-09 0:59 ` tiejun.chen
@ 2012-01-09 12:05 ` Catalin Marinas
2012-01-10 2:59 ` tiejun.chen
1 sibling, 1 reply; 5+ messages in thread
From: Catalin Marinas @ 2012-01-09 12:05 UTC (permalink / raw)
To: Tiejun Chen; +Cc: linux-kernel
On 28 December 2011 08:11, Tiejun Chen <tiejun.chen@windriver.com> wrote:
> We should only scan the sections containing data and it's size is not
> zero as well.
>
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
> kernel/module.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 12cfa2b..0b93c30 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2045,6 +2045,8 @@ static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr,
> if (strncmp(secstrings + sechdrs[i].sh_name, ".data", 5) != 0
> && strncmp(secstrings + sechdrs[i].sh_name, ".bss", 4) != 0)
> continue;
> + if (sechdrs[i].sh_size == 0)
> + continue;
>
> kmemleak_scan_area((void *)sechdrs[i].sh_addr,
> sechdrs[i].sh_size, GFP_KERNEL);
I would rather move this check to kmemleak.c. But why would it be
needed? Performance? A zero-size area shouldn't be scanned anyway.
--
Catalin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] kmemleak/module: only scan the existed data section
2012-01-09 12:05 ` Catalin Marinas
@ 2012-01-10 2:59 ` tiejun.chen
2012-01-10 10:59 ` Catalin Marinas
0 siblings, 1 reply; 5+ messages in thread
From: tiejun.chen @ 2012-01-10 2:59 UTC (permalink / raw)
To: Catalin Marinas; +Cc: linux-kernel
Catalin Marinas wrote:
> On 28 December 2011 08:11, Tiejun Chen <tiejun.chen@windriver.com> wrote:
>> We should only scan the sections containing data and it's size is not
>> zero as well.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---
>> �kernel/module.c | � �2 ++
>> �1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 12cfa2b..0b93c30 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -2045,6 +2045,8 @@ static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr,
>> � � � � � � � �if (strncmp(secstrings + sechdrs[i].sh_name, ".data", 5) != 0
>> � � � � � � � � � �&& strncmp(secstrings + sechdrs[i].sh_name, ".bss", 4) != 0)
>> � � � � � � � � � � � �continue;
>> + � � � � � � � if (sechdrs[i].sh_size == 0)
>> + � � � � � � � � � � � continue;
>>
>> � � � � � � � �kmemleak_scan_area((void *)sechdrs[i].sh_addr,
>> � � � � � � � � � � � � � � � � � sechdrs[i].sh_size, GFP_KERNEL);
>
> I would rather move this check to kmemleak.c. But why would it be
> needed? Performance? A zero-size area shouldn't be scanned anyway.
When we call layout_sections() to calculate sh_entsize, often a zero-sized
.data/.bss section would be ordered as a middle of all valid sections. For example,
------
Symbol Addr size
.init. 0xf96d3000
......
.data(or .bss) 0xf96d3180 0
...... 0xf96d4000
If so kmemleak_scan_area(0xf96d3180,0,GFP_KERNEL) is fine as we expect since
0xf96d3180 is always within a valid address scopes summarized all section,
0xf96d3000 ~ 0xf96d4000. But sometimes if that is arranged as a last section:
-----
Symbol Addr size
.init. 0xf96d3000
......
.data(or .bss) 0xf96d3180 0
An then the following call trace is triggered
......
kmemleak: Adding scan area to unknown object at 0xf96d3180
Call Trace:
[e9095de0] [c0008588] show_stack+0x68/0x1d8 (unreliable)
[e9095e30] [c0690094] dump_stack+0x2c/0x44
[e9095e40] [c015a190] kmemleak_scan_area+0x128/0x184
[e9095e70] [c00a145c] load_module+0xa98/0x1c04
[e9095f10] [c00a2650] sys_init_module+0x88/0x24c
[e9095f40] [c0012f7c] ret_from_syscall+0x0/0x4
--- Exception: c01 at 0xff63564
LR = 0x10003414
Tiejun
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] kmemleak/module: only scan the existed data section
2012-01-10 2:59 ` tiejun.chen
@ 2012-01-10 10:59 ` Catalin Marinas
0 siblings, 0 replies; 5+ messages in thread
From: Catalin Marinas @ 2012-01-10 10:59 UTC (permalink / raw)
To: tiejun.chen; +Cc: linux-kernel
On Tue, Jan 10, 2012 at 02:59:42AM +0000, tiejun.chen wrote:
> Catalin Marinas wrote:
> > I would rather move this check to kmemleak.c. But why would it be
> > needed? Performance? A zero-size area shouldn't be scanned anyway.
>
> When we call layout_sections() to calculate sh_entsize, often a zero-sized
> .data/.bss section would be ordered as a middle of all valid sections. For example,
> ------
> Symbol Addr size
>
> .init. 0xf96d3000
> ......
> .data(or .bss) 0xf96d3180 0
> ...... 0xf96d4000
>
> If so kmemleak_scan_area(0xf96d3180,0,GFP_KERNEL) is fine as we expect since
> 0xf96d3180 is always within a valid address scopes summarized all section,
> 0xf96d3000 ~ 0xf96d4000. But sometimes if that is arranged as a last section:
> -----
> Symbol Addr size
>
> .init. 0xf96d3000
> ......
> .data(or .bss) 0xf96d3180 0
>
>
> An then the following call trace is triggered
> ......
> kmemleak: Adding scan area to unknown object at 0xf96d3180
> Call Trace:
> [e9095de0] [c0008588] show_stack+0x68/0x1d8 (unreliable)
> [e9095e30] [c0690094] dump_stack+0x2c/0x44
> [e9095e40] [c015a190] kmemleak_scan_area+0x128/0x184
> [e9095e70] [c00a145c] load_module+0xa98/0x1c04
> [e9095f10] [c00a2650] sys_init_module+0x88/0x24c
> [e9095f40] [c0012f7c] ret_from_syscall+0x0/0x4
> --- Exception: c01 at 0xff63564
> LR = 0x10003414
Ah, good find. As I said, I would check the size in the
kmemleak_scan_area() function and ignore if 0 (same as the ptr check).
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-01-10 11:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-28 8:11 [PATCH 1/1] kmemleak/module: only scan the existed data section Tiejun Chen
2012-01-09 0:59 ` tiejun.chen
2012-01-09 12:05 ` Catalin Marinas
2012-01-10 2:59 ` tiejun.chen
2012-01-10 10:59 ` Catalin Marinas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).