public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Alexey Dobriyan <adobriyan@sw.ru>
To: roland@redhat.com, akpm@osdl.org
Cc: magnus_vesterlund@hotmail.com, linux-kernel@vger.kernel.org,
	devel@openvz.org
Subject: [PATCH -mm] utrace: fix double free re __rcu_process_callbacks()
Date: Tue, 24 Apr 2007 13:10:23 +0400	[thread overview]
Message-ID: <20070424091023.GA10539@localhost.sw.ru> (raw)

The following patch fixes double free manifesting itself as crash in
__rcu_process_callbasks():
http://marc.info/?l=linux-kernel&m=117518764517017&w=2
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229112

The problem is with check_dead_utrace() conditionally scheduling
"struct utrace" for freeing but not cleaning struct task_struct::utrace
pointer leaving it reachable:

	tsk->utrace_flags = flags;
	if (flags)
		spin_unlock(&utrace->lock);
	else
		rcu_utrace_free(utrace);

OTOH, utrace_release_task() first clears ->utrace pointer, then frees
struct utrace itself:

Roland inserted some debugging into 2.6.21-rc6-mm1 so that aforementined
double free couldn't be reproduced without seeing
BUG at kernel/utrace.c:176 first. It triggers if one struct utrace were
passed to rcu_utrace_free() second time.

With patch applied I no longer see¹ BUG message and double frees on
2-way P3, 8-way ia64, Core 2 Duo boxes. Testcase is at the first link.

I _think_ it adds leak if utrace_reap() takes branch without freeing
but, well, I hope Roland will give me some clue on how to fix it too.

Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
---

 kernel/utrace.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

¹ But I see whole can of other bugs! I think they were already lurking
  but weren't easily reproducable without hitting double-free first.
  FWIW, it's
  	BUG_ON(!list_empty(&tsk->ptracees));
	oops at the beginning of remove_engine()
	NULL ->report_quiesce call which is absent in ptrace utrace ops
	BUG_ON(tracehook_check_released(p));

--- a/kernel/utrace.c
+++ b/kernel/utrace.c
@@ -205,7 +205,6 @@ utrace_clear_tsk(struct task_struct *tsk
 	if (utrace->u.live.signal == NULL) {
 		task_lock(tsk);
 		if (likely(tsk->utrace != NULL)) {
-			rcu_assign_pointer(tsk->utrace, NULL);
 			tsk->utrace_flags &= UTRACE_ACTION_NOREAP;
 		}
 		task_unlock(tsk);
@@ -305,10 +304,7 @@ check_dead_utrace(struct task_struct *ts
 	}
 
 	tsk->utrace_flags = flags;
-	if (flags)
-		spin_unlock(&utrace->lock);
-	else
-		rcu_utrace_free(utrace);
+	spin_unlock(&utrace->lock);
 
 	/*
 	 * Now we're finished updating the utrace state.


             reply	other threads:[~2007-04-24  9:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-24  9:10 Alexey Dobriyan [this message]
2007-04-24  9:54 ` [Devel] [PATCH -mm] utrace: fix double free re __rcu_process_callbacks() Kirill Korotaev
2007-04-24 10:40 ` Alexey Dobriyan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070424091023.GA10539@localhost.sw.ru \
    --to=adobriyan@sw.ru \
    --cc=akpm@osdl.org \
    --cc=devel@openvz.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=magnus_vesterlund@hotmail.com \
    --cc=roland@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox