public inbox for linux-ia64@vger.kernel.org
 help / color / mirror / Atom feed
From: Petr Tesarik <ptesarik@suse.cz>
To: linux-ia64@vger.kernel.org
Subject: [PATCH] Simplify ptrace handling
Date: Fri, 19 Oct 2007 19:55:29 +0000	[thread overview]
Message-ID: <1192823729.2727.56.camel@elijah.suse.cz> (raw)

Hi,

with Shaohua Li's patch we can get rid of much of the ugly code in
arch/ia64/kernel/ptrace.c:

 - find_thread_for_addr() is no longer needed
 - most calls to ia64_poke() and ia64_peek() can be converted to
   simple accesses to the VM

This patch is incremental with Shaohua Li's fix, so it can't be applied
until we resolve the only remaining problem (race condition in
ptrace_stop). I'm only sending it to demonstrate the benefits of that
patch.

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>

diff -u linux.orig/arch/ia64/kernel/ptrace.c linux/arch/ia64/kernel/ptrace.c
--- linux.orig/arch/ia64/kernel/ptrace.c	2007-10-19 21:24:21.451863000 +0200
+++ linux/arch/ia64/kernel/ptrace.c	2007-10-19 21:37:48.835854400 +0200
@@ -631,52 +631,6 @@
 }
 
 /*
- * GDB apparently wants to be able to read the register-backing store
- * of any thread when attached to a given process.  If we are peeking
- * or poking an address that happens to reside in the kernel-backing
- * store of another thread, we need to attach to that thread, because
- * otherwise we end up accessing stale data.
- *
- * task_list_lock must be read-locked before calling this routine!
- */
-static struct task_struct *
-find_thread_for_addr (struct task_struct *child, unsigned long addr)
-{
-	struct task_struct *p;
-	struct mm_struct *mm;
-	struct list_head *this, *next;
-	int mm_users;
-
-	if (!(mm = get_task_mm(child)))
-		return child;
-
-	/* -1 because of our get_task_mm(): */
-	mm_users = atomic_read(&mm->mm_users) - 1;
-	if (mm_users <= 1)
-		goto out;		/* not multi-threaded */
-
-	/*
-	 * Traverse the current process' children list.  Every task that
-	 * one attaches to becomes a child.  And it is only attached children
-	 * of the debugger that are of interest (ptrace_check_attach checks
-	 * for this).
-	 */
- 	list_for_each_safe(this, next, &current->children) {
-		p = list_entry(this, struct task_struct, sibling);
-		if (p->tgid != child->tgid)
-			continue;
-		if (thread_matches(p, addr)) {
-			child = p;
-			goto out;
-		}
-	}
-
-  out:
-	mmput(mm);
-	return child;
-}
-
-/*
  * Write f32-f127 back to task->thread.fph if it has been modified.
  */
 inline void
@@ -841,7 +795,7 @@
 access_uarea (struct task_struct *child, unsigned long addr,
 	      unsigned long *data, int write_access)
 {
-	unsigned long *ptr, regnum, urbs_end, rnat_addr, cfm;
+	unsigned long *ptr, regnum, urbs_end, cfm;
 	struct switch_stack *sw;
 	struct pt_regs *pt;
 #	define pt_reg_addr(pt, reg)	((void *)			    \
@@ -1026,15 +980,11 @@
 			return 0;
 
 		      case PT_AR_RNAT:
-			urbs_end = ia64_get_user_rbs_end(child, pt, NULL);
-			rnat_addr = (long) ia64_rse_rnat_addr((long *)
-							      urbs_end);
 			if (write_access)
-				return ia64_poke(child, sw, urbs_end,
-						 rnat_addr, *data);
+				pt->ar_rnat = *data;
 			else
-				return ia64_peek(child, sw, urbs_end,
-						 rnat_addr, data);
+				*data = pt->ar_rnat;
+			return 0;
 
 		      case PT_R1:
 			ptr = pt_reg_addr(pt, r1);
@@ -1474,11 +1424,9 @@
 sys_ptrace (long request, pid_t pid, unsigned long addr, unsigned long data)
 {
 	struct pt_regs *pt;
-	unsigned long urbs_end, peek_or_poke;
 	struct task_struct *child;
 	struct switch_stack *sw;
 	long ret;
-	struct unw_frame_info info;
 
 	lock_kernel();
 	ret = -EPERM;
@@ -1487,19 +1435,12 @@
 		goto out;
 	}
 
-	peek_or_poke = (request = PTRACE_PEEKTEXT
-			|| request = PTRACE_PEEKDATA
-			|| request = PTRACE_POKETEXT
-			|| request = PTRACE_POKEDATA);
 	ret = -ESRCH;
 	read_lock(&tasklist_lock);
 	{
 		child = find_task_by_pid(pid);
-		if (child) {
-			if (peek_or_poke)
-				child = find_thread_for_addr(child, addr);
+		if (child)
 			get_task_struct(child);
-		}
 	}
 	read_unlock(&tasklist_lock);
 	if (!child)
@@ -1524,25 +1465,23 @@
 	      case PTRACE_PEEKTEXT:
 	      case PTRACE_PEEKDATA:
 		/* read word at location addr */
-		urbs_end = ia64_get_user_rbs_end(child, pt, NULL);
-		ret = ia64_peek(child, sw, urbs_end, addr, &data);
-		if (ret = 0) {
+		if (access_process_vm(child, addr, &data, sizeof(data), 0)
+		    = sizeof(data)) {
 			ret = data;
 			/* ensure "ret" is not mistaken as an error code: */
 			force_successful_syscall_return();
-		}
+		} else
+			ret = -EIO;
 		goto out_tsk;
 
 	      case PTRACE_POKETEXT:
 	      case PTRACE_POKEDATA:
 		/* write the word at location addr */
-		urbs_end = ia64_get_user_rbs_end(child, pt, NULL);
-		ret = ia64_poke(child, sw, urbs_end, addr, data);
-
-		/* Make sure user RBS has the latest data */
-		unw_init_from_blocked_task(&info, child);
-		do_sync_rbs(&info, RBS_SYNC_TO_USER);
-
+		if (access_process_vm(child, addr, &data, sizeof(data), 1)
+		    = sizeof(data))
+			ret = 0;
+		else
+			ret = -EIO;
 		goto out_tsk;
 
 	      case PTRACE_PEEKUSR:


             reply	other threads:[~2007-10-19 19:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-19 19:55 Petr Tesarik [this message]
2007-10-19 20:20 ` [PATCH] Simplify ptrace handling Christoph Hellwig

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=1192823729.2727.56.camel@elijah.suse.cz \
    --to=ptesarik@suse.cz \
    --cc=linux-ia64@vger.kernel.org \
    /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