BUG: Linux 2.6.25 ptrace leaks struct

All of lore.kernel.org
 help / color / mirror / Atom feed

* BUG: Linux 2.6.25 ptrace leaks struct_task
@ 2008-06-27 18:30 Joris van Rantwijk
  2008-06-27 20:00 ` Jeff Dike
  0 siblings, 1 reply; 5+ messages in thread
From: Joris van Rantwijk @ 2008-06-27 18:30 UTC (permalink / raw)
  To: linux-kernel; +Cc: Pekka Enberg, Thorsten Knabe

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: BUG: Linux 2.6.25 ptrace leaks struct_task
  2008-06-27 18:30 BUG: Linux 2.6.25 ptrace leaks struct_task Joris van Rantwijk
@ 2008-06-27 20:00 ` Jeff Dike
  2008-06-27 20:17   ` Roland McGrath
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jeff Dike @ 2008-06-27 20:00 UTC (permalink / raw)
  To: Joris van Rantwijk
  Cc: linux-kernel, Pekka Enberg, Thorsten Knabe, Roland McGrath

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: BUG: Linux 2.6.25 ptrace leaks struct_task
  2008-06-27 20:00 ` Jeff Dike
@ 2008-06-27 20:17   ` Roland McGrath
  2008-06-27 20:18   ` [PATCH 2.6.25-stable] x86_64 ptrace: fix sys32_ptrace task_struct leak Roland McGrath
  2008-06-27 20:48   ` Roland McGrath
  2 siblings, 0 replies; 5+ messages in thread
From: Roland McGrath @ 2008-06-27 20:17 UTC (permalink / raw)
  To: Jeff Dike; +Cc: Joris van Rantwijk, linux-kernel, Pekka Enberg, Thorsten Knabe

> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2.6.25-stable] x86_64 ptrace: fix sys32_ptrace task_struct leak
  2008-06-27 20:00 ` Jeff Dike
  2008-06-27 20:17   ` Roland McGrath
@ 2008-06-27 20:18   ` Roland McGrath
  2008-06-27 20:48   ` Roland McGrath
  2 siblings, 0 replies; 5+ messages in thread
From: Roland McGrath @ 2008-06-27 20:18 UTC (permalink / raw)
  To: stable
  Cc: Jeff Dike, Joris van Rantwijk, linux-kernel, Pekka Enberg,
	Thorsten Knabe

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 <roland@redhat.com>
---
 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:

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2.6.25-stable] x86_64 ptrace: fix sys32_ptrace task_struct leak
  2008-06-27 20:00 ` Jeff Dike
  2008-06-27 20:17   ` Roland McGrath
  2008-06-27 20:18   ` [PATCH 2.6.25-stable] x86_64 ptrace: fix sys32_ptrace task_struct leak Roland McGrath
@ 2008-06-27 20:48   ` Roland McGrath
  2 siblings, 0 replies; 5+ messages in thread
From: Roland McGrath @ 2008-06-27 20:48 UTC (permalink / raw)
  To: stable
  Cc: Jeff Dike, Joris van Rantwijk, linux-kernel, Pekka Enberg,
	Thorsten Knabe

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 <roland@redhat.com>
---
 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:

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-06-27 20:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-27 18:30 BUG: Linux 2.6.25 ptrace leaks struct_task Joris van Rantwijk
2008-06-27 20:00 ` Jeff Dike
2008-06-27 20:17   ` Roland McGrath
2008-06-27 20:18   ` [PATCH 2.6.25-stable] x86_64 ptrace: fix sys32_ptrace task_struct leak Roland McGrath
2008-06-27 20:48   ` Roland McGrath

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.