I think sys32_ptrace() is leaking struct_task.
In arch/x86/kernel/ptrace.c, function sys32_ptrace(), there
is a call to ptrace_get_task_struct(). In some cases (such as
PTRACE_GETREGS), there is no matching call to put_task_struct().
Test case: fork many childs, calling PTRACE_GETREGS on each child.
Indeed this causes unbounded growth of the task_struct allocation
in /proc/slabinfo, and it also causes physical memory to disappear
from /proc/meminfo.
I have tested this on Linux 2.6.25.4 for x86_64, but the
relevant code has not been changed in 2.6.25.9.
This bug appears to be fixed in 2.6.26-rc8: the code looks
different and I can't reproduce the issue there.
Oops, just discovered this problem has already been reported
in http://lkml.org/lkml/2008/5/29/266 although not much
seems to have been done about it.
Should this be fixed for 2.6.25.10 ?
Is it likely that this bug is related to the mysterious
disappearance of memory from /proc/meminfo as reported in
http://lkml.org/lkml/2008/6/24/15 ?
Even so, how is it possible that memory just disappears
from /proc/meminfo? It can't be the task_struct cache itself,
because that is all covered under Slab, right ?
Greetings, Joris.
On Fri, Jun 27, 2008 at 08:30:45PM +0200, Joris van Rantwijk wrote:
> I think sys32_ptrace() is leaking struct_task.
>
> In arch/x86/kernel/ptrace.c, function sys32_ptrace(), there
> is a call to ptrace_get_task_struct(). In some cases (such as
> PTRACE_GETREGS), there is no matching call to put_task_struct().
Yup, good diagnosis. The culprit is
5a4646a4efed8c835f76c3b88f3155f6ab5b8d9b. Doing an s/return /ret = /
inside that switch should fix the bug.
It looks like it's since been fixed in mainline by the restructuring
done in 562b80bafffaf42a6d916b0a2ee3d684220a1c10.
Jeff
--
Work email - jdike at linux dot intel dot com
> Yup, good diagnosis. The culprit is
> 5a4646a4efed8c835f76c3b88f3155f6ab5b8d9b. Doing an s/return /ret = /
> inside that switch should fix the bug.
I could swear I posted this fix two weeks ago, but I can't find the posting
now. I do still have it lurking in a GIT tree though.
Thanks,
Roland
Commit 5a4646a4efed8c835f76c3b88f3155f6ab5b8d9b introduced a leak of
task_struct refs into sys32_ptrace. This bug has already gone away in
for 2.6.26 in commit 562b80bafffaf42a6d916b0a2ee3d684220a1c10.
Signed-off-by: Roland McGrath <[email protected]>
---
arch/x86/kernel/ptrace.c | 45 ++++++++++++++++++++++++++-------------------
1 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 9003e0b..a10ba65 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1309,42 +1309,49 @@ asmlinkage long sys32_ptrace(long request, u32 pid, u32 addr, u32 data)
break;
case PTRACE_GETREGS: /* Get all gp regs from the child. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_GENERAL,
- 0, sizeof(struct user_regs_struct32),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_GENERAL,
+ 0, sizeof(struct user_regs_struct32),
+ datap);
+ break;
case PTRACE_SETREGS: /* Set all gp regs in the child. */
- return copy_regset_from_user(child, &user_x86_32_view,
- REGSET_GENERAL, 0,
- sizeof(struct user_regs_struct32),
- datap);
+ ret = copy_regset_from_user(child, &user_x86_32_view,
+ REGSET_GENERAL, 0,
+ sizeof(struct user_regs_struct32),
+ datap);
+ break;
case PTRACE_GETFPREGS: /* Get the child FPU state. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_FP, 0,
- sizeof(struct user_i387_ia32_struct),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_FP, 0,
+ sizeof(struct user_i387_ia32_struct),
+ datap);
+ break;
case PTRACE_SETFPREGS: /* Set the child FPU state. */
- return copy_regset_from_user(
+ ret = copy_regset_from_user(
child, &user_x86_32_view, REGSET_FP,
0, sizeof(struct user_i387_ia32_struct), datap);
+ break;
case PTRACE_GETFPXREGS: /* Get the child extended FPU state. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_XFP, 0,
- sizeof(struct user32_fxsr_struct),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_XFP, 0,
+ sizeof(struct user32_fxsr_struct),
+ datap);
+ break;
case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
- return copy_regset_from_user(child, &user_x86_32_view,
+ ret = copy_regset_from_user(child, &user_x86_32_view,
REGSET_XFP, 0,
sizeof(struct user32_fxsr_struct),
datap);
+ break;
default:
- return compat_ptrace_request(child, request, addr, data);
+ ret = compat_ptrace_request(child, request, addr, data);
+ break;
}
out:
Commit 5a4646a4efed8c835f76c3b88f3155f6ab5b8d9b introduced a leak of
task_struct refs into sys32_ptrace. This bug has already gone away in
for 2.6.26 in commit 562b80bafffaf42a6d916b0a2ee3d684220a1c10.
Signed-off-by: Roland McGrath <[email protected]>
---
arch/x86/kernel/ptrace.c | 45 ++++++++++++++++++++++++++-------------------
1 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 9003e0b..a10ba65 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1309,42 +1309,49 @@ asmlinkage long sys32_ptrace(long request, u32 pid, u32 addr, u32 data)
break;
case PTRACE_GETREGS: /* Get all gp regs from the child. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_GENERAL,
- 0, sizeof(struct user_regs_struct32),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_GENERAL,
+ 0, sizeof(struct user_regs_struct32),
+ datap);
+ break;
case PTRACE_SETREGS: /* Set all gp regs in the child. */
- return copy_regset_from_user(child, &user_x86_32_view,
- REGSET_GENERAL, 0,
- sizeof(struct user_regs_struct32),
- datap);
+ ret = copy_regset_from_user(child, &user_x86_32_view,
+ REGSET_GENERAL, 0,
+ sizeof(struct user_regs_struct32),
+ datap);
+ break;
case PTRACE_GETFPREGS: /* Get the child FPU state. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_FP, 0,
- sizeof(struct user_i387_ia32_struct),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_FP, 0,
+ sizeof(struct user_i387_ia32_struct),
+ datap);
+ break;
case PTRACE_SETFPREGS: /* Set the child FPU state. */
- return copy_regset_from_user(
+ ret = copy_regset_from_user(
child, &user_x86_32_view, REGSET_FP,
0, sizeof(struct user_i387_ia32_struct), datap);
+ break;
case PTRACE_GETFPXREGS: /* Get the child extended FPU state. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_XFP, 0,
- sizeof(struct user32_fxsr_struct),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_XFP, 0,
+ sizeof(struct user32_fxsr_struct),
+ datap);
+ break;
case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
- return copy_regset_from_user(child, &user_x86_32_view,
+ ret = copy_regset_from_user(child, &user_x86_32_view,
REGSET_XFP, 0,
sizeof(struct user32_fxsr_struct),
datap);
+ break;
default:
- return compat_ptrace_request(child, request, addr, data);
+ ret = compat_ptrace_request(child, request, addr, data);
+ break;
}
out: