[PATCH v2 00/24] ptrace cleanups

[PATCH v2 00/24] ptrace cleanups

[Namhyung Kim]

Hello,

This patchset tries to cleanup architecture independent/dependent part of
ptrace syscall. Specifically it changes prototype of sys_ptrace() to have
its arguments @addr and @data to be unsigned long. Since user space API of
ptrace() declares them as void pointers, and most of archs consider them
unsigned already in their arch_ptrace(), it would be no harmful to change
them from (signed) long into unsigned long, IMHO.

ptrace() code accesses user area using above arguments frequently, changing
them enables to drop unnecessary __force markup when casting them to __user
pointers because sparse allows it only in case of unsigned long. And some
duplicated/misused castings can be grouped into one place in favor of new
variables to cleanup code further. This was suggested by Arnd Bergmann and
comment from Christoph Hellwig was also helpful.

Although I couldn't test all archs, I hope it will not break anything.

Any comments would be appreciated.
Thanks.


* Changes from v1:
  combine all arch signature changes into a single patch
  retain uses of get/put_user()
  rebased on top of 2.6.36-rc3

---
Namhyung Kim (24):
  ptrace: change signature of sys_ptrace() and friends
  ptrace: cleanup ptrace_request()
  ptrace: change signature of arch_ptrace()
  ptrace: cleanup arch_ptrace() on x86
  ptrace: cleanup arch_ptrace() on ARM
  ptrace: cleanup arch_ptrace() on avr32
  ptrace: cleanup arch_ptrace() and friends on Blackfin
  ptrace: cleanup arch_ptrace() on cris
  ptrace: cleanup arch_ptrace() on frv
  ptrace: cleanup arch_ptrace() on h8300
  ptrace: cleanup arch_ptrace() on m32r
  ptrace: cleanup arch_ptrace() on m68k
  ptrace: cleanup arch_ptrace() on m68knommu
  ptrace: cleanup arch_ptrace() on microblaze
  ptrace: cleanup arch_ptrace() on MIPS
  ptrace: cleanup arch_ptrace() on mn10300
  ptrace: cleanup arch_ptrace() on parisc
  ptrace: cleanup arch_ptrace() on powerpc
  ptrace: cleanup arch_ptrace() on score
  ptrace: cleanup arch_ptrace() on sh
  ptrace: cleanup arch_ptrace() on sparc
  ptrace: cleanup arch_ptrace() on tile
  ptrace: cleanup arch_ptrace() on um
  ptrace: cleanup arch_ptrace() on xtensa

 arch/alpha/kernel/ptrace.c         |    7 ++--
 arch/arm/kernel/ptrace.c           |   28 ++++++++-------
 arch/avr32/kernel/ptrace.c         |   11 +++---
 arch/blackfin/kernel/ptrace.c      |   16 +++++----
 arch/cris/arch-v10/kernel/ptrace.c |   20 +++++-----
 arch/cris/arch-v32/kernel/ptrace.c |   16 ++++----
 arch/frv/kernel/ptrace.c           |   32 ++++++++---------
 arch/h8300/kernel/ptrace.c         |   33 +++++++++---------
 arch/ia64/kernel/ptrace.c          |    3 +-
 arch/m32r/kernel/ptrace.c          |   11 +++---
 arch/m68k/kernel/ptrace.c          |   51 ++++++++++++++-------------
 arch/m68knommu/kernel/ptrace.c     |   63 ++++++++++++++++------------------
 arch/microblaze/kernel/ptrace.c    |    5 ++-
 arch/mips/kernel/ptrace.c          |   25 +++++++------
 arch/mn10300/kernel/ptrace.c       |   20 +++++-----
 arch/parisc/kernel/ptrace.c        |   13 ++++---
 arch/powerpc/kernel/ptrace.c       |   66 ++++++++++++++++++-----------------
 arch/s390/kernel/ptrace.c          |    3 +-
 arch/score/kernel/ptrace.c         |    7 ++--
 arch/sh/kernel/ptrace_32.c         |   45 +++++++++++++-----------
 arch/sh/kernel/ptrace_64.c         |   25 +++++++++-----
 arch/sparc/kernel/ptrace_32.c      |   57 ++++++++++++-------------------
 arch/sparc/kernel/ptrace_64.c      |   15 ++++----
 arch/tile/kernel/ptrace.c          |   11 +++---
 arch/um/kernel/ptrace.c            |   23 ++++++------
 arch/um/sys-i386/ptrace.c          |    4 +-
 arch/um/sys-x86_64/ptrace.c        |   11 +++---
 arch/x86/kernel/ptrace.c           |   20 +++++------
 arch/xtensa/kernel/ptrace.c        |   14 ++++---
 include/linux/ptrace.h             |   12 ++++--
 include/linux/syscalls.h           |    3 +-
 kernel/ptrace.c                    |   29 +++++++++------
 32 files changed, 360 insertions(+), 339 deletions(-)

--
1.7.2.2

[PATCH v2 01/24] ptrace: change signature of sys_ptrace() and friends

[Namhyung Kim]

Since user space API of ptrace syscall defines @addr and @data as void
pointers, it would be more appropriate to define them as unsigned long
in kernel. Therefore related functions are changed also.

'unsigned long' is typically used in other places in kernel as an
opaque data type and that using this helps cleaning up a lot of warnings
from sparse.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/ptrace.h   |    9 ++++++---
 include/linux/syscalls.h |    3 ++-
 kernel/ptrace.c          |   16 ++++++++++------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 4272521..67a4cd7 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -108,7 +108,8 @@ extern int ptrace_attach(struct task_struct *tsk);
 extern int ptrace_detach(struct task_struct *, unsigned int);
 extern void ptrace_disable(struct task_struct *);
 extern int ptrace_check_attach(struct task_struct *task, int kill);
-extern int ptrace_request(struct task_struct *child, long request, long addr, long data);
+extern int ptrace_request(struct task_struct *child, long request,
+			  unsigned long addr, unsigned long data);
 extern void ptrace_notify(int exit_code);
 extern void __ptrace_link(struct task_struct *child,
 			  struct task_struct *new_parent);
@@ -132,8 +133,10 @@ static inline void ptrace_unlink(struct task_struct *child)
 		__ptrace_unlink(child);
 }
 
-int generic_ptrace_peekdata(struct task_struct *tsk, long addr, long data);
-int generic_ptrace_pokedata(struct task_struct *tsk, long addr, long data);
+int generic_ptrace_peekdata(struct task_struct *tsk, unsigned long addr,
+			    unsigned long data);
+int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr,
+			    unsigned long data);
 
 /**
  * task_ptrace - return %PT_* flags that apply to a task
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e6319d1..cacc27a 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -701,7 +701,8 @@ asmlinkage long sys_nfsservctl(int cmd,
 asmlinkage long sys_syslog(int type, char __user *buf, int len);
 asmlinkage long sys_uselib(const char __user *library);
 asmlinkage long sys_ni_syscall(void);
-asmlinkage long sys_ptrace(long request, long pid, long addr, long data);
+asmlinkage long sys_ptrace(long request, long pid, unsigned long addr,
+			   unsigned long data);
 
 asmlinkage long sys_add_key(const char __user *_type,
 			    const char __user *_description,
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index f34d798..f838afe 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -402,7 +402,7 @@ int ptrace_writedata(struct task_struct *tsk, char __user *src, unsigned long ds
 	return copied;
 }
 
-static int ptrace_setoptions(struct task_struct *child, long data)
+static int ptrace_setoptions(struct task_struct *child, unsigned long data)
 {
 	child->ptrace &= ~PT_TRACE_MASK;
 
@@ -481,7 +481,8 @@ static int ptrace_setsiginfo(struct task_struct *child, const siginfo_t *info)
 #define is_sysemu_singlestep(request)	0
 #endif
 
-static int ptrace_resume(struct task_struct *child, long request, long data)
+static int ptrace_resume(struct task_struct *child, long request,
+			 unsigned long data)
 {
 	if (!valid_signal(data))
 		return -EIO;
@@ -558,7 +559,7 @@ static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
 #endif
 
 int ptrace_request(struct task_struct *child, long request,
-		   long addr, long data)
+		   unsigned long addr, unsigned long data)
 {
 	int ret = -EIO;
 	siginfo_t siginfo;
@@ -691,7 +692,8 @@ static struct task_struct *ptrace_get_task_struct(pid_t pid)
 #define arch_ptrace_attach(child)	do { } while (0)
 #endif
 
-SYSCALL_DEFINE4(ptrace, long, request, long, pid, long, addr, long, data)
+SYSCALL_DEFINE4(ptrace, long, request, long, pid, unsigned long, addr,
+		unsigned long, data)
 {
 	struct task_struct *child;
 	long ret;
@@ -732,7 +734,8 @@ SYSCALL_DEFINE4(ptrace, long, request, long, pid, long, addr, long, data)
 	return ret;
 }
 
-int generic_ptrace_peekdata(struct task_struct *tsk, long addr, long data)
+int generic_ptrace_peekdata(struct task_struct *tsk, unsigned long addr,
+			    unsigned long data)
 {
 	unsigned long tmp;
 	int copied;
@@ -743,7 +746,8 @@ int generic_ptrace_peekdata(struct task_struct *tsk, long addr, long data)
 	return put_user(tmp, (unsigned long __user *)data);
 }
 
-int generic_ptrace_pokedata(struct task_struct *tsk, long addr, long data)
+int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr,
+			    unsigned long data)
 {
 	int copied;
 
-- 
1.7.2.2

[PATCH v2 02/24] ptrace: cleanup ptrace_request()

[Namhyung Kim]

use new local variable to remove unnecesary casting, but it requires
put_user() to be changed to copy_to_user().

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 kernel/ptrace.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index f838afe..1084bef 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -563,6 +563,7 @@ int ptrace_request(struct task_struct *child, long request,
 {
 	int ret = -EIO;
 	siginfo_t siginfo;
+	void __user *datap = (void __user *) data;
 
 	switch (request) {
 	case PTRACE_PEEKTEXT:
@@ -579,19 +580,19 @@ int ptrace_request(struct task_struct *child, long request,
 		ret = ptrace_setoptions(child, data);
 		break;
 	case PTRACE_GETEVENTMSG:
-		ret = put_user(child->ptrace_message, (unsigned long __user *) data);
+		ret = copy_to_user(datap, &child->ptrace_message,
+				   sizeof (child->ptrace_message)) ?
+			-EFAULT : 0;
 		break;
 
 	case PTRACE_GETSIGINFO:
 		ret = ptrace_getsiginfo(child, &siginfo);
 		if (!ret)
-			ret = copy_siginfo_to_user((siginfo_t __user *) data,
-						   &siginfo);
+			ret = copy_siginfo_to_user(datap, &siginfo);
 		break;
 
 	case PTRACE_SETSIGINFO:
-		if (copy_from_user(&siginfo, (siginfo_t __user *) data,
-				   sizeof siginfo))
+		if (copy_from_user(&siginfo, datap, sizeof siginfo))
 			ret = -EFAULT;
 		else
 			ret = ptrace_setsiginfo(child, &siginfo);
@@ -622,7 +623,7 @@ int ptrace_request(struct task_struct *child, long request,
 		}
 		mmput(mm);
 
-		ret = put_user(tmp, (unsigned long __user *) data);
+		ret = copy_to_user(datap, &tmp, sizeof tmp) ? -EFAULT : 0;
 		break;
 	}
 #endif
-- 
1.7.2.2

Re: [PATCH v2 02/24] ptrace: cleanup ptrace_request()

[Roland McGrath]

You've still changed put_user to copy_to_user here.
I thought you'd agreed not to change that.

Thanks,
Roland

Re: [PATCH v2 02/24] ptrace: cleanup ptrace_request()

[Namhyung Kim]

Roland McGrath <roland@redhat.com> writes:
> You've still changed put_user to copy_to_user here.
> I thought you'd agreed not to change that.
>

My bad. I just sent v1, sorry.
Here comes updated version.


>From 2e6543cdfb05c66dcb18ce5e8d941d835a4767af Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@gmail.com>
Date: Thu, 26 Aug 2010 01:46:52 +0900
Subject: [PATCH v2 02/24 UPDATED] ptrace: cleanup ptrace_request()

Use new 'datavp' and 'datalp' variables to remove unnecesary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 kernel/ptrace.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index f838afe..dded1d6 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -563,6 +563,8 @@ int ptrace_request(struct task_struct *child, long request,
 {
 	int ret = -EIO;
 	siginfo_t siginfo;
+	void __user *datavp = (void __user *) data;
+	unsigned long __user *datalp = datavp;
 
 	switch (request) {
 	case PTRACE_PEEKTEXT:
@@ -579,19 +581,17 @@ int ptrace_request(struct task_struct *child, long request,
 		ret = ptrace_setoptions(child, data);
 		break;
 	case PTRACE_GETEVENTMSG:
-		ret = put_user(child->ptrace_message, (unsigned long __user *) data);
+		ret = put_user(child->ptrace_message, datalp);
 		break;
 
 	case PTRACE_GETSIGINFO:
 		ret = ptrace_getsiginfo(child, &siginfo);
 		if (!ret)
-			ret = copy_siginfo_to_user((siginfo_t __user *) data,
-						   &siginfo);
+			ret = copy_siginfo_to_user(datavp, &siginfo);
 		break;
 
 	case PTRACE_SETSIGINFO:
-		if (copy_from_user(&siginfo, (siginfo_t __user *) data,
-				   sizeof siginfo))
+		if (copy_from_user(&siginfo, datavp, sizeof siginfo))
 			ret = -EFAULT;
 		else
 			ret = ptrace_setsiginfo(child, &siginfo);
@@ -622,7 +622,7 @@ int ptrace_request(struct task_struct *child, long request,
 		}
 		mmput(mm);
 
-		ret = put_user(tmp, (unsigned long __user *) data);
+		ret = put_user(tmp, datalp);
 		break;
 	}
 #endif
@@ -651,7 +651,7 @@ int ptrace_request(struct task_struct *child, long request,
 	case PTRACE_SETREGSET:
 	{
 		struct iovec kiov;
-		struct iovec __user *uiov = (struct iovec __user *) data;
+		struct iovec __user *uiov = datavp;
 
 		if (!access_ok(VERIFY_WRITE, uiov, sizeof(*uiov)))
 			return -EFAULT;
-- 
1.7.2.2

[PATCH v2 03/24] ptrace: change signature of arch_ptrace()

[Namhyung Kim]

Fix up the arguments to arch_ptrace() to take account of the fact that
@addr and @data are now unsigned long rather than long as of a preceding
patch in this series.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: linux-arch@vger.kernel.org
---
 arch/alpha/kernel/ptrace.c         |    7 ++++---
 arch/arm/kernel/ptrace.c           |    3 ++-
 arch/avr32/kernel/ptrace.c         |    3 ++-
 arch/blackfin/kernel/ptrace.c      |    3 ++-
 arch/cris/arch-v10/kernel/ptrace.c |    7 ++++---
 arch/cris/arch-v32/kernel/ptrace.c |    3 ++-
 arch/frv/kernel/ptrace.c           |    3 ++-
 arch/h8300/kernel/ptrace.c         |    7 ++++---
 arch/ia64/kernel/ptrace.c          |    3 ++-
 arch/m32r/kernel/ptrace.c          |    3 ++-
 arch/m68k/kernel/ptrace.c          |    9 +++++----
 arch/m68knommu/kernel/ptrace.c     |    9 +++++----
 arch/microblaze/kernel/ptrace.c    |    3 ++-
 arch/mips/kernel/ptrace.c          |    3 ++-
 arch/mn10300/kernel/ptrace.c       |    3 ++-
 arch/parisc/kernel/ptrace.c        |   11 ++++++-----
 arch/powerpc/kernel/ptrace.c       |   15 ++++++++-------
 arch/s390/kernel/ptrace.c          |    3 ++-
 arch/score/kernel/ptrace.c         |    3 ++-
 arch/sh/kernel/ptrace_32.c         |   25 +++++++++++++------------
 arch/sh/kernel/ptrace_64.c         |    6 ++++--
 arch/sparc/kernel/ptrace_32.c      |    3 ++-
 arch/sparc/kernel/ptrace_64.c      |    7 ++++---
 arch/tile/kernel/ptrace.c          |    7 ++++---
 arch/um/kernel/ptrace.c            |    5 +++--
 arch/um/sys-i386/ptrace.c          |    4 ++--
 arch/um/sys-x86_64/ptrace.c        |    4 ++--
 arch/x86/kernel/ptrace.c           |    7 ++++---
 arch/xtensa/kernel/ptrace.c        |    3 ++-
 include/linux/ptrace.h             |    3 ++-
 30 files changed, 102 insertions(+), 73 deletions(-)

diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
index baa9036..e2af5eb 100644
--- a/arch/alpha/kernel/ptrace.c
+++ b/arch/alpha/kernel/ptrace.c
@@ -269,7 +269,8 @@ void ptrace_disable(struct task_struct *child)
 	user_disable_single_step(child);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	unsigned long tmp;
 	size_t copied;
@@ -292,7 +293,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	case PTRACE_PEEKUSR:
 		force_successful_syscall_return();
 		ret = get_reg(child, addr);
-		DBG(DBG_MEM, ("peek $%ld->%#lx\n", addr, ret));
+		DBG(DBG_MEM, ("peek $%lu->%#lx\n", addr, ret));
 		break;
 
 	/* When I and D space are separate, this will have to be fixed.  */
@@ -302,7 +303,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 		break;
 
 	case PTRACE_POKEUSR: /* write the specified register */
-		DBG(DBG_MEM, ("poke $%ld<-%#lx\n", addr, data));
+		DBG(DBG_MEM, ("poke $%lu<-%#lx\n", addr, data));
 		ret = put_reg(child, addr, data);
 		break;
 	default:
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index f99d489..87ca2c7 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -847,7 +847,8 @@ static int ptrace_setvfpregs(struct task_struct *tsk, void __user *data)
 }
 #endif
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 
diff --git a/arch/avr32/kernel/ptrace.c b/arch/avr32/kernel/ptrace.c
index 5e73c25..ecea9b6 100644
--- a/arch/avr32/kernel/ptrace.c
+++ b/arch/avr32/kernel/ptrace.c
@@ -146,7 +146,8 @@ static int ptrace_setregs(struct task_struct *tsk, const void __user *uregs)
 	return ret;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 
diff --git a/arch/blackfin/kernel/ptrace.c b/arch/blackfin/kernel/ptrace.c
index 6ec7768..fb81620 100644
--- a/arch/blackfin/kernel/ptrace.c
+++ b/arch/blackfin/kernel/ptrace.c
@@ -232,7 +232,8 @@ void user_disable_single_step(struct task_struct *child)
 	clear_tsk_thread_flag(child, TIF_SINGLESTEP);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 	unsigned long __user *datap = (unsigned long __user *)data;
diff --git a/arch/cris/arch-v10/kernel/ptrace.c b/arch/cris/arch-v10/kernel/ptrace.c
index e70c804..d411e02 100644
--- a/arch/cris/arch-v10/kernel/ptrace.c
+++ b/arch/cris/arch-v10/kernel/ptrace.c
@@ -76,7 +76,8 @@ ptrace_disable(struct task_struct *child)
  * (in user space) where the result of the ptrace call is written (instead of
  * being returned).
  */
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 	unsigned long __user *datap = (unsigned long __user *)data;
@@ -141,7 +142,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 					break;
 				}
 				
-				data += sizeof(long);
+				data += sizeof(unsigned long);
 			}
 
 			break;
@@ -165,7 +166,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 				}
 				
 				put_reg(child, i, tmp);
-				data += sizeof(long);
+				data += sizeof(unsigned long);
 			}
 			
 			break;
diff --git a/arch/cris/arch-v32/kernel/ptrace.c b/arch/cris/arch-v32/kernel/ptrace.c
index f4ebd1e..3e058a1 100644
--- a/arch/cris/arch-v32/kernel/ptrace.c
+++ b/arch/cris/arch-v32/kernel/ptrace.c
@@ -126,7 +126,8 @@ ptrace_disable(struct task_struct *child)
 }
 
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 	unsigned long __user *datap = (unsigned long __user *)data;
diff --git a/arch/frv/kernel/ptrace.c b/arch/frv/kernel/ptrace.c
index fac0289..e9dbfad 100644
--- a/arch/frv/kernel/ptrace.c
+++ b/arch/frv/kernel/ptrace.c
@@ -254,7 +254,8 @@ void ptrace_disable(struct task_struct *child)
 	user_disable_single_step(child);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	unsigned long tmp;
 	int ret;
diff --git a/arch/h8300/kernel/ptrace.c b/arch/h8300/kernel/ptrace.c
index df11412..ef1aa0b 100644
--- a/arch/h8300/kernel/ptrace.c
+++ b/arch/h8300/kernel/ptrace.c
@@ -50,7 +50,8 @@ void ptrace_disable(struct task_struct *child)
 	user_disable_single_step(child);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 
@@ -120,7 +121,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 				ret = -EFAULT;
 				break;
 			    }
-			    data += sizeof(long);
+			    data += sizeof(unsigned long);
 			}
 			ret = 0;
 			break;
@@ -135,7 +136,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 				break;
 			    }
 			    h8300_put_reg(child, i, tmp);
-			    data += sizeof(long);
+			    data += sizeof(unsigned long);
 			}
 			ret = 0;
 			break;
diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index 7c7909f..8848f43 100644
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1177,7 +1177,8 @@ ptrace_disable (struct task_struct *child)
 }
 
 long
-arch_ptrace (struct task_struct *child, long request, long addr, long data)
+arch_ptrace (struct task_struct *child, long request,
+	     unsigned long addr, unsigned long data)
 {
 	switch (request) {
 	case PTRACE_PEEKTEXT:
diff --git a/arch/m32r/kernel/ptrace.c b/arch/m32r/kernel/ptrace.c
index e555091..69bbf7a 100644
--- a/arch/m32r/kernel/ptrace.c
+++ b/arch/m32r/kernel/ptrace.c
@@ -621,7 +621,8 @@ void ptrace_disable(struct task_struct *child)
 }
 
 long
-arch_ptrace(struct task_struct *child, long request, long addr, long data)
+arch_ptrace(struct task_struct *child, long request,
+	    unsigned long addr, unsigned long data)
 {
 	int ret;
 
diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
index 616e597..583f59f 100644
--- a/arch/m68k/kernel/ptrace.c
+++ b/arch/m68k/kernel/ptrace.c
@@ -156,7 +156,8 @@ void user_disable_single_step(struct task_struct *child)
 	singlestep_disable(child);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	unsigned long tmp;
 	int i, ret = 0;
@@ -200,7 +201,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 			 * into internal fpu reg representation
 			 */
 			if (FPU_IS_EMU && (addr < 45) && !(addr % 3)) {
-				data = (unsigned long)data << 15;
+				data <<= 15;
 				data = (data & 0xffff0000) |
 				       ((data & 0x0000ffff) >> 1);
 			}
@@ -215,7 +216,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 			ret = put_user(tmp, (unsigned long *)data);
 			if (ret)
 				break;
-			data += sizeof(long);
+			data += sizeof(unsigned long);
 		}
 		break;
 
@@ -229,7 +230,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 				tmp |= get_reg(child, PT_SR) & ~SR_MASK;
 			}
 			put_reg(child, i, tmp);
-			data += sizeof(long);
+			data += sizeof(unsigned long);
 		}
 		break;
 
diff --git a/arch/m68knommu/kernel/ptrace.c b/arch/m68knommu/kernel/ptrace.c
index f6be124..4ab9448 100644
--- a/arch/m68knommu/kernel/ptrace.c
+++ b/arch/m68knommu/kernel/ptrace.c
@@ -111,7 +111,8 @@ void ptrace_disable(struct task_struct *child)
 	user_disable_single_step(child);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 
@@ -180,7 +181,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 				 * into internal fpu reg representation
 				 */
 				if (FPU_IS_EMU && (addr < 45) && !(addr % 3)) {
-					data = (unsigned long)data << 15;
+					data <<= 15;
 					data = (data & 0xffff0000) |
 					       ((data & 0x0000ffff) >> 1);
 				}
@@ -201,7 +202,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 				ret = -EFAULT;
 				break;
 			    }
-			    data += sizeof(long);
+			    data += sizeof(unsigned long);
 			}
 			ret = 0;
 			break;
@@ -221,7 +222,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 				tmp |= get_reg(child, PT_SR) & ~(SR_MASK << 16);
 			    }
 			    put_reg(child, i, tmp);
-			    data += sizeof(long);
+			    data += sizeof(unsigned long);
 			}
 			ret = 0;
 			break;
diff --git a/arch/microblaze/kernel/ptrace.c b/arch/microblaze/kernel/ptrace.c
index dc03ffc..3544bc1 100644
--- a/arch/microblaze/kernel/ptrace.c
+++ b/arch/microblaze/kernel/ptrace.c
@@ -73,7 +73,8 @@ static microblaze_reg_t *reg_save_addr(unsigned reg_offs,
 	return (microblaze_reg_t *)((char *)regs + reg_offs);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int rval;
 	unsigned long val = 0;
diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index c51b95f..26c29dd 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -255,7 +255,8 @@ int ptrace_set_watch_regs(struct task_struct *child,
 	return 0;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 
diff --git a/arch/mn10300/kernel/ptrace.c b/arch/mn10300/kernel/ptrace.c
index cf847da..ec4b414 100644
--- a/arch/mn10300/kernel/ptrace.c
+++ b/arch/mn10300/kernel/ptrace.c
@@ -295,7 +295,8 @@ void ptrace_disable(struct task_struct *child)
 /*
  * handle the arch-specific side of process tracing
  */
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	unsigned long tmp;
 	int ret;
diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c
index c4f49e4..03920db 100644
--- a/arch/parisc/kernel/ptrace.c
+++ b/arch/parisc/kernel/ptrace.c
@@ -110,7 +110,8 @@ void user_enable_block_step(struct task_struct *task)
 	pa_psw(task)->l = 0;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	unsigned long tmp;
 	long ret = -EIO;
@@ -120,8 +121,8 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	/* Read the word at location addr in the USER area.  For ptraced
 	   processes, the kernel saves all regs on a syscall. */
 	case PTRACE_PEEKUSR:
-		if ((addr & (sizeof(long)-1)) ||
-		    (unsigned long) addr >= sizeof(struct pt_regs))
+		if ((addr & (sizeof(unsigned long)-1)) ||
+		     addr >= sizeof(struct pt_regs))
 			break;
 		tmp = *(unsigned long *) ((char *) task_regs(child) + addr);
 		ret = put_user(tmp, (unsigned long *) data);
@@ -151,8 +152,8 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 			break;
 		}
 
-		if ((addr & (sizeof(long)-1)) ||
-		    (unsigned long) addr >= sizeof(struct pt_regs))
+		if ((addr & (sizeof(unsigned long)-1)) ||
+		     addr >= sizeof(struct pt_regs))
 			break;
 		if ((addr >= PT_GR1 && addr <= PT_GR31) ||
 				addr == PT_IAOQ0 || addr == PT_IAOQ1 ||
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 11f3cd9..e4b6d75 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1406,8 +1406,8 @@ static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
  * Here are the old "legacy" powerpc specific getregs/setregs ptrace calls,
  * we mark them as obsolete now, they will be removed in a future version
  */
-static long arch_ptrace_old(struct task_struct *child, long request, long addr,
-			    long data)
+static long arch_ptrace_old(struct task_struct *child, long request,
+			    unsigned long addr, unsigned long data)
 {
 	switch (request) {
 	case PPC_PTRACE_GETREGS:	/* Get GPRs 0 - 31. */
@@ -1434,7 +1434,8 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
 	return -EPERM;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret = -EPERM;
 
@@ -1446,11 +1447,11 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 		ret = -EIO;
 		/* convert to index and check */
 #ifdef CONFIG_PPC32
-		index = (unsigned long) addr >> 2;
+		index = addr >> 2;
 		if ((addr & 3) || (index > PT_FPSCR)
 		    || (child->thread.regs == NULL))
 #else
-		index = (unsigned long) addr >> 3;
+		index = addr >> 3;
 		if ((addr & 7) || (index > PT_FPSCR))
 #endif
 			break;
@@ -1474,11 +1475,11 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 		ret = -EIO;
 		/* convert to index and check */
 #ifdef CONFIG_PPC32
-		index = (unsigned long) addr >> 2;
+		index = addr >> 2;
 		if ((addr & 3) || (index > PT_FPSCR)
 		    || (child->thread.regs == NULL))
 #else
-		index = (unsigned long) addr >> 3;
+		index = addr >> 3;
 		if ((addr & 7) || (index > PT_FPSCR))
 #endif
 			break;
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 83339d3..019bb71 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -343,7 +343,8 @@ poke_user(struct task_struct *child, addr_t addr, addr_t data)
 	return __poke_user(child, addr, data);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	ptrace_area parea; 
 	int copied, ret;
diff --git a/arch/score/kernel/ptrace.c b/arch/score/kernel/ptrace.c
index 174c642..894dcbf 100644
--- a/arch/score/kernel/ptrace.c
+++ b/arch/score/kernel/ptrace.c
@@ -325,7 +325,8 @@ void ptrace_disable(struct task_struct *child)
 }
 
 long
-arch_ptrace(struct task_struct *child, long request, long addr, long data)
+arch_ptrace(struct task_struct *child, long request,
+	    unsigned long addr, unsigned long data)
 {
 	int ret;
 	unsigned long __user *datap = (void __user *)data;
diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c
index 6c4bbba..7464607 100644
--- a/arch/sh/kernel/ptrace_32.c
+++ b/arch/sh/kernel/ptrace_32.c
@@ -338,7 +338,8 @@ const struct user_regset_view *task_user_regset_view(struct task_struct *task)
 	return &user_sh_native_view;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	struct user * dummy = NULL;
 	unsigned long __user *datap = (unsigned long __user *)data;
@@ -356,17 +357,17 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 
 		if (addr < sizeof(struct pt_regs))
 			tmp = get_stack_long(child, addr);
-		else if (addr >= (long) &dummy->fpu &&
-			 addr < (long) &dummy->u_fpvalid) {
+		else if (addr >= (unsigned long) &dummy->fpu &&
+			 addr < (unsigned long) &dummy->u_fpvalid) {
 			if (!tsk_used_math(child)) {
-				if (addr == (long)&dummy->fpu.fpscr)
+				if (addr == (unsigned long)&dummy->fpu.fpscr)
 					tmp = FPSCR_INIT;
 				else
 					tmp = 0;
 			} else
-				tmp = ((long *)child->thread.xstate)
-					[(addr - (long)&dummy->fpu) >> 2];
-		} else if (addr == (long) &dummy->u_fpvalid)
+				tmp = ((unsigned long *)child->thread.xstate)
+					[(addr - (unsigned long)&dummy->fpu) >> 2];
+		} else if (addr == (unsigned long) &dummy->u_fpvalid)
 			tmp = !!tsk_used_math(child);
 		else if (addr == PT_TEXT_ADDR)
 			tmp = child->mm->start_code;
@@ -390,13 +391,13 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 
 		if (addr < sizeof(struct pt_regs))
 			ret = put_stack_long(child, addr, data);
-		else if (addr >= (long) &dummy->fpu &&
-			 addr < (long) &dummy->u_fpvalid) {
+		else if (addr >= (unsigned long) &dummy->fpu &&
+			 addr < (unsigned long) &dummy->u_fpvalid) {
 			set_stopped_child_used_math(child);
-			((long *)child->thread.xstate)
-				[(addr - (long)&dummy->fpu) >> 2] = data;
+			((unsigned long *)child->thread.xstate)
+				[(addr - (unsigned long)&dummy->fpu) >> 2] = data;
 			ret = 0;
-		} else if (addr == (long) &dummy->u_fpvalid) {
+		} else if (addr == (unsigned long) &dummy->u_fpvalid) {
 			conditional_stopped_child_used_math(data, child);
 			ret = 0;
 		}
diff --git a/arch/sh/kernel/ptrace_64.c b/arch/sh/kernel/ptrace_64.c
index 5fd644d..1367134 100644
--- a/arch/sh/kernel/ptrace_64.c
+++ b/arch/sh/kernel/ptrace_64.c
@@ -304,7 +304,8 @@ const struct user_regset_view *task_user_regset_view(struct task_struct *task)
 	return &user_sh64_native_view;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 
@@ -392,7 +393,8 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	return ret;
 }
 
-asmlinkage int sh64_ptrace(long request, long pid, long addr, long data)
+asmlinkage int sh64_ptrace(long request, long pid,
+			   unsigned long addr, unsigned long data)
 {
 #define WPC_DBRMODE 0x0d104008
 	static int first_call = 1;
diff --git a/arch/sparc/kernel/ptrace_32.c b/arch/sparc/kernel/ptrace_32.c
index e608f39..e08ba4a 100644
--- a/arch/sparc/kernel/ptrace_32.c
+++ b/arch/sparc/kernel/ptrace_32.c
@@ -323,7 +323,8 @@ const struct user_regset_view *task_user_regset_view(struct task_struct *task)
 	return &user_sparc32_view;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	unsigned long addr2 = current->thread.kregs->u_regs[UREG_I4];
 	const struct user_regset_view *view;
diff --git a/arch/sparc/kernel/ptrace_64.c b/arch/sparc/kernel/ptrace_64.c
index aa90da0..d9db5a4 100644
--- a/arch/sparc/kernel/ptrace_64.c
+++ b/arch/sparc/kernel/ptrace_64.c
@@ -969,7 +969,8 @@ struct fps {
 	unsigned long fsr;
 };
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	const struct user_regset_view *view = task_user_regset_view(current);
 	unsigned long addr2 = task_pt_regs(current)->u_regs[UREG_I4];
@@ -977,8 +978,8 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	struct fps __user *fps;
 	int ret;
 
-	pregs = (struct pt_regs __user *) (unsigned long) addr;
-	fps = (struct fps __user *) (unsigned long) addr;
+	pregs = (struct pt_regs __user *) addr;
+	fps = (struct fps __user *) addr;
 
 	switch (request) {
 	case PTRACE_PEEKUSR:
diff --git a/arch/tile/kernel/ptrace.c b/arch/tile/kernel/ptrace.c
index 7161bd0..aed9256 100644
--- a/arch/tile/kernel/ptrace.c
+++ b/arch/tile/kernel/ptrace.c
@@ -64,7 +64,8 @@ void ptrace_disable(struct task_struct *child)
 	clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	unsigned long __user *datap;
 	unsigned long tmp;
@@ -102,7 +103,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	case PTRACE_GETREGS:  /* Get all registers from the child. */
 		if (!access_ok(VERIFY_WRITE, datap, PTREGS_SIZE))
 			break;
-		for (i = 0; i < PTREGS_SIZE; i += sizeof(long)) {
+		for (i = 0; i < PTREGS_SIZE; i += sizeof(unsigned long)) {
 			ret = __put_user(getreg(child, i), datap);
 			if (ret != 0)
 				break;
@@ -113,7 +114,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	case PTRACE_SETREGS:  /* Set all registers in the child. */
 		if (!access_ok(VERIFY_READ, datap, PTREGS_SIZE))
 			break;
-		for (i = 0; i < PTREGS_SIZE; i += sizeof(long)) {
+		for (i = 0; i < PTREGS_SIZE; i += sizeof(unsigned long)) {
 			ret = __get_user(tmp, datap);
 			if (ret != 0)
 				break;
diff --git a/arch/um/kernel/ptrace.c b/arch/um/kernel/ptrace.c
index e051049..963d82b 100644
--- a/arch/um/kernel/ptrace.c
+++ b/arch/um/kernel/ptrace.c
@@ -42,10 +42,11 @@ void ptrace_disable(struct task_struct *child)
 extern int peek_user(struct task_struct * child, long addr, long data);
 extern int poke_user(struct task_struct * child, long addr, long data);
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int i, ret;
-	unsigned long __user *p = (void __user *)(unsigned long)data;
+	unsigned long __user *p = (void __user *)data;
 
 	switch (request) {
 	/* read word at location addr. */
diff --git a/arch/um/sys-i386/ptrace.c b/arch/um/sys-i386/ptrace.c
index c9b1765..d23b2d3 100644
--- a/arch/um/sys-i386/ptrace.c
+++ b/arch/um/sys-i386/ptrace.c
@@ -203,8 +203,8 @@ int set_fpxregs(struct user_fxsr_struct __user *buf, struct task_struct *child)
 				     (unsigned long *) &fpregs);
 }
 
-long subarch_ptrace(struct task_struct *child, long request, long addr,
-		    long data)
+long subarch_ptrace(struct task_struct *child, long request,
+		    unsigned long addr, unsigned long data)
 {
 	return -EIO;
 }
diff --git a/arch/um/sys-x86_64/ptrace.c b/arch/um/sys-x86_64/ptrace.c
index f3458d7..67e6368 100644
--- a/arch/um/sys-x86_64/ptrace.c
+++ b/arch/um/sys-x86_64/ptrace.c
@@ -175,8 +175,8 @@ int set_fpregs(struct user_i387_struct __user *buf, struct task_struct *child)
 	return restore_fp_registers(userspace_pid[cpu], fpregs);
 }
 
-long subarch_ptrace(struct task_struct *child, long request, long addr,
-		    long data)
+long subarch_ptrace(struct task_struct *child, long request,
+		    unsigned long addr, unsigned long data)
 {
 	int ret = -EIO;
 
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 70c4872..1a7ca04 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -801,7 +801,8 @@ void ptrace_disable(struct task_struct *child)
 static const struct user_regset_view user_x86_32_view; /* Initialized below. */
 #endif
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret;
 	unsigned long __user *datap = (unsigned long __user *)data;
@@ -888,14 +889,14 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 
 #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
 	case PTRACE_GET_THREAD_AREA:
-		if (addr < 0)
+		if ((int) addr < 0)
 			return -EIO;
 		ret = do_get_thread_area(child, addr,
 					 (struct user_desc __user *) data);
 		break;
 
 	case PTRACE_SET_THREAD_AREA:
-		if (addr < 0)
+		if ((int) addr < 0)
 			return -EIO;
 		ret = do_set_thread_area(child, addr,
 					 (struct user_desc __user *) data, 0);
diff --git a/arch/xtensa/kernel/ptrace.c b/arch/xtensa/kernel/ptrace.c
index 9d4e1ce..af9ba80 100644
--- a/arch/xtensa/kernel/ptrace.c
+++ b/arch/xtensa/kernel/ptrace.c
@@ -256,7 +256,8 @@ int ptrace_pokeusr(struct task_struct *child, long regno, long val)
 	return 0;
 }
 
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+long arch_ptrace(struct task_struct *child, long request,
+		 unsigned long addr, unsigned long data)
 {
 	int ret = -EPERM;
 
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 67a4cd7..092a04f 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -100,7 +100,8 @@
 #include <linux/sched.h>		/* For struct task_struct.  */
 
 
-extern long arch_ptrace(struct task_struct *child, long request, long addr, long data);
+extern long arch_ptrace(struct task_struct *child, long request,
+			unsigned long addr, unsigned long data);
 extern int ptrace_traceme(void);
 extern int ptrace_readdata(struct task_struct *tsk, unsigned long src, char __user *dst, int len);
 extern int ptrace_writedata(struct task_struct *tsk, char __user *src, unsigned long dst, int len);
-- 
1.7.2.2

[PATCH v2 04/24] ptrace: cleanup arch_ptrace() on x86

[Namhyung Kim]

Remove checking @addr less than 0 because @addr is now unsigned and
use new udescp variable in order to remove unnecessary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/kernel/ptrace.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 1a7ca04..6167a0c 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -806,6 +806,7 @@ long arch_ptrace(struct task_struct *child, long request,
 {
 	int ret;
 	unsigned long __user *datap = (unsigned long __user *)data;
+	struct user_desc __user *udescp = (struct user_desc __user *)data;
 
 	switch (request) {
 	/* read the word at location addr in the USER area. */
@@ -813,8 +814,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		unsigned long tmp;
 
 		ret = -EIO;
-		if ((addr & (sizeof(data) - 1)) || addr < 0 ||
-		    addr >= sizeof(struct user))
+		if ((addr & (sizeof(data) - 1)) || addr >= sizeof(struct user))
 			break;
 
 		tmp = 0;  /* Default return condition */
@@ -831,8 +831,7 @@ long arch_ptrace(struct task_struct *child, long request,
 
 	case PTRACE_POKEUSR: /* write the word at location addr in the USER area */
 		ret = -EIO;
-		if ((addr & (sizeof(data) - 1)) || addr < 0 ||
-		    addr >= sizeof(struct user))
+		if ((addr & (sizeof(data) - 1)) || addr >= sizeof(struct user))
 			break;
 
 		if (addr < sizeof(struct user_regs_struct))
@@ -891,15 +890,13 @@ long arch_ptrace(struct task_struct *child, long request,
 	case PTRACE_GET_THREAD_AREA:
 		if ((int) addr < 0)
 			return -EIO;
-		ret = do_get_thread_area(child, addr,
-					 (struct user_desc __user *) data);
+		ret = do_get_thread_area(child, addr, udescp);
 		break;
 
 	case PTRACE_SET_THREAD_AREA:
 		if ((int) addr < 0)
 			return -EIO;
-		ret = do_set_thread_area(child, addr,
-					 (struct user_desc __user *) data, 0);
+		ret = do_set_thread_area(child, addr, udescp, 0);
 		break;
 #endif
 
-- 
1.7.2.2

[PATCH v2 05/24] ptrace: cleanup arch_ptrace() on ARM

[Namhyung Kim]

use new 'datap' variable in order to remove unnecessary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>
---
 arch/arm/kernel/ptrace.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 87ca2c7..9dfb40a 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -851,10 +851,11 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 		case PTRACE_PEEKUSR:
-			ret = ptrace_read_user(child, addr, (unsigned long __user *)data);
+			ret = ptrace_read_user(child, addr, datap);
 			break;
 
 		case PTRACE_POKEUSR:
@@ -862,34 +863,34 @@ long arch_ptrace(struct task_struct *child, long request,
 			break;
 
 		case PTRACE_GETREGS:
-			ret = ptrace_getregs(child, (void __user *)data);
+			ret = ptrace_getregs(child, datap);
 			break;
 
 		case PTRACE_SETREGS:
-			ret = ptrace_setregs(child, (void __user *)data);
+			ret = ptrace_setregs(child, datap);
 			break;
 
 		case PTRACE_GETFPREGS:
-			ret = ptrace_getfpregs(child, (void __user *)data);
+			ret = ptrace_getfpregs(child, datap);
 			break;
 		
 		case PTRACE_SETFPREGS:
-			ret = ptrace_setfpregs(child, (void __user *)data);
+			ret = ptrace_setfpregs(child, datap);
 			break;
 
 #ifdef CONFIG_IWMMXT
 		case PTRACE_GETWMMXREGS:
-			ret = ptrace_getwmmxregs(child, (void __user *)data);
+			ret = ptrace_getwmmxregs(child, datap);
 			break;
 
 		case PTRACE_SETWMMXREGS:
-			ret = ptrace_setwmmxregs(child, (void __user *)data);
+			ret = ptrace_setwmmxregs(child, datap);
 			break;
 #endif
 
 		case PTRACE_GET_THREAD_AREA:
 			ret = put_user(task_thread_info(child)->tp_value,
-				       (unsigned long __user *) data);
+				       datap);
 			break;
 
 		case PTRACE_SET_SYSCALL:
@@ -899,21 +900,21 @@ long arch_ptrace(struct task_struct *child, long request,
 
 #ifdef CONFIG_CRUNCH
 		case PTRACE_GETCRUNCHREGS:
-			ret = ptrace_getcrunchregs(child, (void __user *)data);
+			ret = ptrace_getcrunchregs(child, datap);
 			break;
 
 		case PTRACE_SETCRUNCHREGS:
-			ret = ptrace_setcrunchregs(child, (void __user *)data);
+			ret = ptrace_setcrunchregs(child, datap);
 			break;
 #endif
 
 #ifdef CONFIG_VFP
 		case PTRACE_GETVFPREGS:
-			ret = ptrace_getvfpregs(child, (void __user *)data);
+			ret = ptrace_getvfpregs(child, datap);
 			break;
 
 		case PTRACE_SETVFPREGS:
-			ret = ptrace_setvfpregs(child, (void __user *)data);
+			ret = ptrace_setvfpregs(child, datap);
 			break;
 #endif
 
-- 
1.7.2.2

[PATCH v2 06/24] ptrace: cleanup arch_ptrace() on avr32

[Namhyung Kim]

use new 'datap' variable type of void pointer in order to remove unnecessary
castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Acked-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 arch/avr32/kernel/ptrace.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/avr32/kernel/ptrace.c b/arch/avr32/kernel/ptrace.c
index ecea9b6..4aedcab 100644
--- a/arch/avr32/kernel/ptrace.c
+++ b/arch/avr32/kernel/ptrace.c
@@ -150,6 +150,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	void __user *datap = (void __user *) data;
 
 	switch (request) {
 	/* Read the word at location addr in the child process */
@@ -159,8 +160,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		break;
 
 	case PTRACE_PEEKUSR:
-		ret = ptrace_read_user(child, addr,
-				       (unsigned long __user *)data);
+		ret = ptrace_read_user(child, addr, datap);
 		break;
 
 	/* Write the word in data at location addr */
@@ -174,11 +174,11 @@ long arch_ptrace(struct task_struct *child, long request,
 		break;
 
 	case PTRACE_GETREGS:
-		ret = ptrace_getregs(child, (void __user *)data);
+		ret = ptrace_getregs(child, datap);
 		break;
 
 	case PTRACE_SETREGS:
-		ret = ptrace_setregs(child, (const void __user *)data);
+		ret = ptrace_setregs(child, datap);
 		break;
 
 	default:
-- 
1.7.2.2

[PATCH v2 07/24] ptrace: cleanup arch_ptrace() and friends on Blackfin

[Namhyung Kim]

Change signature of get/put_reg() according to the change of arch_ptrace()
and remove unnecessary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/kernel/ptrace.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/blackfin/kernel/ptrace.c b/arch/blackfin/kernel/ptrace.c
index fb81620..545fc14 100644
--- a/arch/blackfin/kernel/ptrace.c
+++ b/arch/blackfin/kernel/ptrace.c
@@ -37,12 +37,13 @@
  * Get contents of register REGNO in task TASK.
  */
 static inline long
-get_reg(struct task_struct *task, long regno, unsigned long __user *datap)
+get_reg(struct task_struct *task, unsigned long regno,
+	unsigned long __user *datap)
 {
 	long tmp;
 	struct pt_regs *regs = task_pt_regs(task);
 
-	if (regno & 3 || regno > PT_LAST_PSEUDO || regno < 0)
+	if (regno & 3 || regno > PT_LAST_PSEUDO)
 		return -EIO;
 
 	switch (regno) {
@@ -73,11 +74,11 @@ get_reg(struct task_struct *task, long regno, unsigned long __user *datap)
  * Write contents of register REGNO in task TASK.
  */
 static inline int
-put_reg(struct task_struct *task, long regno, unsigned long data)
+put_reg(struct task_struct *task, unsigned long regno, unsigned long data)
 {
 	struct pt_regs *regs = task_pt_regs(task);
 
-	if (regno & 3 || regno > PT_LAST_PSEUDO || regno < 0)
+	if (regno & 3 || regno > PT_LAST_PSEUDO)
 		return -EIO;
 
 	switch (regno) {
@@ -361,14 +362,14 @@ long arch_ptrace(struct task_struct *child, long request,
 		return copy_regset_to_user(child, &user_bfin_native_view,
 					   REGSET_GENERAL,
 					   0, sizeof(struct pt_regs),
-					   (void __user *)data);
+					   datap);
 
 	case PTRACE_SETREGS:
 		pr_debug("ptrace: PTRACE_SETREGS\n");
 		return copy_regset_from_user(child, &user_bfin_native_view,
 					     REGSET_GENERAL,
 					     0, sizeof(struct pt_regs),
-					     (const void __user *)data);
+					     datap);
 
 	case_default:
 	default:
-- 
1.7.2.2

Re: [PATCH v2 07/24] ptrace: cleanup arch_ptrace() and friends on Blackfin

[Mike Frysinger]

On Thu, Sep 2, 2010 at 11:46, Namhyung Kim wrote:
> Change signature of get/put_reg() according to the change of arch_ptrace()
> and remove unnecessary castings.

thanks; Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

[PATCH v2 08/24] ptrace: cleanup arch_ptrace() on cris

[Namhyung Kim]

Use new 'regno' variable in order to remove redandunt expression and
remove checking @addr less than 0 because @addr is now unsigned.
Also update 'datap' on PTRACE_GET/SETREGS to fix a bug on arch-v10.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Mikael Starvik <starvik@axis.com>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
---
 arch/cris/arch-v10/kernel/ptrace.c |   17 ++++++++---------
 arch/cris/arch-v32/kernel/ptrace.c |   13 ++++++-------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/arch/cris/arch-v10/kernel/ptrace.c b/arch/cris/arch-v10/kernel/ptrace.c
index d411e02..320065f 100644
--- a/arch/cris/arch-v10/kernel/ptrace.c
+++ b/arch/cris/arch-v10/kernel/ptrace.c
@@ -80,6 +80,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	unsigned int regno = addr >> 2;
 	unsigned long __user *datap = (unsigned long __user *)data;
 
 	switch (request) {
@@ -94,10 +95,10 @@ long arch_ptrace(struct task_struct *child, long request,
 			unsigned long tmp;
 
 			ret = -EIO;
-			if ((addr & 3) || addr < 0 || addr > PT_MAX << 2)
+			if ((addr & 3) || regno > PT_MAX)
 				break;
 
-			tmp = get_reg(child, addr >> 2);
+			tmp = get_reg(child, regno);
 			ret = put_user(tmp, datap);
 			break;
 		}
@@ -111,19 +112,17 @@ long arch_ptrace(struct task_struct *child, long request,
  		/* Write the word at location address in the USER area. */
 		case PTRACE_POKEUSR:
 			ret = -EIO;
-			if ((addr & 3) || addr < 0 || addr > PT_MAX << 2)
+			if ((addr & 3) || regno > PT_MAX)
 				break;
 
-			addr >>= 2;
-
-			if (addr == PT_DCCR) {
+			if (regno == PT_DCCR) {
 				/* don't allow the tracing process to change stuff like
 				 * interrupt enable, kernel/user bit, dma enables etc.
 				 */
 				data &= DCCR_MASK;
 				data |= get_reg(child, PT_DCCR) & ~DCCR_MASK;
 			}
-			if (put_reg(child, addr, data))
+			if (put_reg(child, regno, data))
 				break;
 			ret = 0;
 			break;
@@ -142,7 +141,7 @@ long arch_ptrace(struct task_struct *child, long request,
 					break;
 				}
 				
-				data += sizeof(unsigned long);
+				datap++;
 			}
 
 			break;
@@ -166,7 +165,7 @@ long arch_ptrace(struct task_struct *child, long request,
 				}
 				
 				put_reg(child, i, tmp);
-				data += sizeof(unsigned long);
+				datap++;
 			}
 			
 			break;
diff --git a/arch/cris/arch-v32/kernel/ptrace.c b/arch/cris/arch-v32/kernel/ptrace.c
index 3e058a1..511ece9 100644
--- a/arch/cris/arch-v32/kernel/ptrace.c
+++ b/arch/cris/arch-v32/kernel/ptrace.c
@@ -130,6 +130,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	unsigned int regno = addr >> 2;
 	unsigned long __user *datap = (unsigned long __user *)data;
 
 	switch (request) {
@@ -164,10 +165,10 @@ long arch_ptrace(struct task_struct *child, long request,
 			unsigned long tmp;
 
 			ret = -EIO;
-			if ((addr & 3) || addr < 0 || addr > PT_MAX << 2)
+			if ((addr & 3) || regno > PT_MAX)
 				break;
 
-			tmp = get_reg(child, addr >> 2);
+			tmp = get_reg(child, regno);
 			ret = put_user(tmp, datap);
 			break;
 		}
@@ -181,19 +182,17 @@ long arch_ptrace(struct task_struct *child, long request,
 		/* Write the word at location address in the USER area. */
 		case PTRACE_POKEUSR:
 			ret = -EIO;
-			if ((addr & 3) || addr < 0 || addr > PT_MAX << 2)
+			if ((addr & 3) || regno > PT_MAX)
 				break;
 
-			addr >>= 2;
-
-			if (addr == PT_CCS) {
+			if (regno == PT_CCS) {
 				/* don't allow the tracing process to change stuff like
 				 * interrupt enable, kernel/user bit, dma enables etc.
 				 */
 				data &= CCS_MASK;
 				data |= get_reg(child, PT_CCS) & ~CCS_MASK;
 			}
-			if (put_reg(child, addr, data))
+			if (put_reg(child, regno, data))
 				break;
 			ret = 0;
 			break;
-- 
1.7.2.2

[PATCH v2 09/24] ptrace: cleanup arch_ptrace() on frv

[Namhyung Kim]

Use new 'regno', 'datap' variables in order to remove duplicated
expressions and unnecessary castings. Alse remove checking @addr
less than 0 because addr is now unsigned.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: David Howells <dhowells@redhat.com>
---
 arch/frv/kernel/ptrace.c |   29 +++++++++++++----------------
 1 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/arch/frv/kernel/ptrace.c b/arch/frv/kernel/ptrace.c
index e9dbfad..9d68f7f 100644
--- a/arch/frv/kernel/ptrace.c
+++ b/arch/frv/kernel/ptrace.c
@@ -259,19 +259,21 @@ long arch_ptrace(struct task_struct *child, long request,
 {
 	unsigned long tmp;
 	int ret;
+	int regno = addr >> 2;
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 		/* read the word at location addr in the USER area. */
 	case PTRACE_PEEKUSR: {
 		tmp = 0;
 		ret = -EIO;
-		if ((addr & 3) || addr < 0)
+		if (addr & 3)
 			break;
 
 		ret = 0;
-		switch (addr >> 2) {
+		switch (regno) {
 		case 0 ... PT__END - 1:
-			tmp = get_reg(child, addr >> 2);
+			tmp = get_reg(child, regno);
 			break;
 
 		case PT__END + 0:
@@ -300,23 +302,18 @@ long arch_ptrace(struct task_struct *child, long request,
 		}
 
 		if (ret == 0)
-			ret = put_user(tmp, (unsigned long *) data);
+			ret = put_user(tmp, datap);
 		break;
 	}
 
 	case PTRACE_POKEUSR: /* write the word at location addr in the USER area */
 		ret = -EIO;
-		if ((addr & 3) || addr < 0)
+		if (addr & 3)
 			break;
 
-		ret = 0;
-		switch (addr >> 2) {
+		switch (regno) {
 		case 0 ... PT__END - 1:
-			ret = put_reg(child, addr >> 2, data);
-			break;
-
-		default:
-			ret = -EIO;
+			ret = put_reg(child, regno, data);
 			break;
 		}
 		break;
@@ -325,25 +322,25 @@ long arch_ptrace(struct task_struct *child, long request,
 		return copy_regset_to_user(child, &user_frv_native_view,
 					   REGSET_GENERAL,
 					   0, sizeof(child->thread.user->i),
-					   (void __user *)data);
+					   datap);
 
 	case PTRACE_SETREGS:	/* Set all integer regs in the child. */
 		return copy_regset_from_user(child, &user_frv_native_view,
 					     REGSET_GENERAL,
 					     0, sizeof(child->thread.user->i),
-					     (const void __user *)data);
+					     datap);
 
 	case PTRACE_GETFPREGS:	/* Get the child FP/Media state. */
 		return copy_regset_to_user(child, &user_frv_native_view,
 					   REGSET_FPMEDIA,
 					   0, sizeof(child->thread.user->f),
-					   (void __user *)data);
+					   datap);
 
 	case PTRACE_SETFPREGS:	/* Set the child FP/Media state. */
 		return copy_regset_from_user(child, &user_frv_native_view,
 					     REGSET_FPMEDIA,
 					     0, sizeof(child->thread.user->f),
-					     (const void __user *)data);
+					     datap);
 
 	default:
 		ret = ptrace_request(child, request, addr, data);
-- 
1.7.2.2

[PATCH v2 10/24] ptrace: cleanup arch_ptrace() on h8300

[Namhyung Kim]

Use new 'regno', 'datap' variables in order to remove duplicated
expressions and unnecessary castings. Alse remove checking @addr
less than 0 because addr is now unsigned.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 arch/h8300/kernel/ptrace.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/h8300/kernel/ptrace.c b/arch/h8300/kernel/ptrace.c
index ef1aa0b..7388697 100644
--- a/arch/h8300/kernel/ptrace.c
+++ b/arch/h8300/kernel/ptrace.c
@@ -54,24 +54,25 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	int regno = addr >> 2;
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 	/* read the word at location addr in the USER area. */
 		case PTRACE_PEEKUSR: {
 			unsigned long tmp = 0;
 			
-			if ((addr & 3) || addr < 0 || addr >= sizeof(struct user)) {
+			if ((addr & 3) || addr >= sizeof(struct user)) {
 				ret = -EIO;
 				break ;
 			}
 			
 		        ret = 0;  /* Default return condition */
-			addr = addr >> 2; /* temporary hack. */
 
-			if (addr < H8300_REGS_NO)
-				tmp = h8300_get_reg(child, addr);
+			if (regno < H8300_REGS_NO)
+				tmp = h8300_get_reg(child, regno);
 			else {
-				switch(addr) {
+				switch(regno) {
 				case 49:
 					tmp = child->mm->start_code;
 					break ;
@@ -89,24 +90,23 @@ long arch_ptrace(struct task_struct *child, long request,
 				}
 			}
 			if (!ret)
-				ret = put_user(tmp,(unsigned long *) data);
+				ret = put_user(tmp, datap);
 			break ;
 		}
 
       /* when I and D space are separate, this will have to be fixed. */
 		case PTRACE_POKEUSR: /* write the word at location addr in the USER area */
-			if ((addr & 3) || addr < 0 || addr >= sizeof(struct user)) {
+			if ((addr & 3) || addr >= sizeof(struct user)) {
 				ret = -EIO;
 				break ;
 			}
-			addr = addr >> 2; /* temporary hack. */
 			    
-			if (addr == PT_ORIG_ER0) {
+			if (regno == PT_ORIG_ER0) {
 				ret = -EIO;
 				break ;
 			}
-			if (addr < H8300_REGS_NO) {
-				ret = h8300_put_reg(child, addr, data);
+			if (regno < H8300_REGS_NO) {
+				ret = h8300_put_reg(child, regno, data);
 				break ;
 			}
 			ret = -EIO;
@@ -117,11 +117,11 @@ long arch_ptrace(struct task_struct *child, long request,
 			unsigned long tmp;
 			for (i = 0; i < H8300_REGS_NO; i++) {
 			    tmp = h8300_get_reg(child, i);
-			    if (put_user(tmp, (unsigned long *) data)) {
+			    if (put_user(tmp, datap)) {
 				ret = -EFAULT;
 				break;
 			    }
-			    data += sizeof(unsigned long);
+			    datap++;
 			}
 			ret = 0;
 			break;
@@ -131,12 +131,12 @@ long arch_ptrace(struct task_struct *child, long request,
 			int i;
 			unsigned long tmp;
 			for (i = 0; i < H8300_REGS_NO; i++) {
-			    if (get_user(tmp, (unsigned long *) data)) {
+			    if (get_user(tmp, datap)) {
 				ret = -EFAULT;
 				break;
 			    }
 			    h8300_put_reg(child, i, tmp);
-			    data += sizeof(unsigned long);
+			    datap++;
 			}
 			ret = 0;
 			break;
-- 
1.7.2.2

[PATCH v2 11/24] ptrace: cleanup arch_ptrace() on m32r

[Namhyung Kim]

Use new 'datap' variable in order to remove duplicated castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Hirokazu Takata <takata@linux-m32r.org>
---
 arch/m32r/kernel/ptrace.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/m32r/kernel/ptrace.c b/arch/m32r/kernel/ptrace.c
index 69bbf7a..fc93e8d 100644
--- a/arch/m32r/kernel/ptrace.c
+++ b/arch/m32r/kernel/ptrace.c
@@ -625,6 +625,7 @@ arch_ptrace(struct task_struct *child, long request,
 	    unsigned long addr, unsigned long data)
 {
 	int ret;
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 	/*
@@ -639,8 +640,7 @@ arch_ptrace(struct task_struct *child, long request,
 	 * read the word at location addr in the USER area.
 	 */
 	case PTRACE_PEEKUSR:
-		ret = ptrace_read_user(child, addr,
-				       (unsigned long __user *)data);
+		ret = ptrace_read_user(child, addr, datap);
 		break;
 
 	/*
@@ -661,11 +661,11 @@ arch_ptrace(struct task_struct *child, long request,
 		break;
 
 	case PTRACE_GETREGS:
-		ret = ptrace_getregs(child, (void __user *)data);
+		ret = ptrace_getregs(child, datap);
 		break;
 
 	case PTRACE_SETREGS:
-		ret = ptrace_setregs(child, (void __user *)data);
+		ret = ptrace_setregs(child, datap);
 		break;
 
 	default:
-- 
1.7.2.2

[PATCH v2 12/24] ptrace: cleanup arch_ptrace() on m68k

[Namhyung Kim]

Use new 'regno', 'datap' variables in order to remove duplicated
expressions and unnecessary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Roman Zippel <zippel@linux-m68k.org>
---
 arch/m68k/kernel/ptrace.c |   46 ++++++++++++++++++++++----------------------
 1 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
index 583f59f..0b25268 100644
--- a/arch/m68k/kernel/ptrace.c
+++ b/arch/m68k/kernel/ptrace.c
@@ -161,51 +161,52 @@ long arch_ptrace(struct task_struct *child, long request,
 {
 	unsigned long tmp;
 	int i, ret = 0;
+	int regno = addr >> 2; /* temporary hack. */
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 	/* read the word at location addr in the USER area. */
 	case PTRACE_PEEKUSR:
 		if (addr & 3)
 			goto out_eio;
-		addr >>= 2;	/* temporary hack. */
 
-		if (addr >= 0 && addr < 19) {
-			tmp = get_reg(child, addr);
-		} else if (addr >= 21 && addr < 49) {
-			tmp = child->thread.fp[addr - 21];
+		if (regno >= 0 && regno < 19) {
+			tmp = get_reg(child, regno);
+		} else if (regno >= 21 && regno < 49) {
+			tmp = child->thread.fp[regno - 21];
 			/* Convert internal fpu reg representation
 			 * into long double format
 			 */
-			if (FPU_IS_EMU && (addr < 45) && !(addr % 3))
+			if (FPU_IS_EMU && (regno < 45) && !(regno % 3))
 				tmp = ((tmp & 0xffff0000) << 15) |
 				      ((tmp & 0x0000ffff) << 16);
 		} else
 			goto out_eio;
-		ret = put_user(tmp, (unsigned long *)data);
+		ret = put_user(tmp, datap);
 		break;
 
-	case PTRACE_POKEUSR:	/* write the word at location addr in the USER area */
+	case PTRACE_POKEUSR:
+	/* write the word at location addr in the USER area */
 		if (addr & 3)
 			goto out_eio;
-		addr >>= 2;	/* temporary hack. */
 
-		if (addr == PT_SR) {
+		if (regno == PT_SR) {
 			data &= SR_MASK;
 			data |= get_reg(child, PT_SR) & ~SR_MASK;
 		}
-		if (addr >= 0 && addr < 19) {
-			if (put_reg(child, addr, data))
+		if (regno >= 0 && regno < 19) {
+			if (put_reg(child, regno, data))
 				goto out_eio;
-		} else if (addr >= 21 && addr < 48) {
+		} else if (regno >= 21 && regno < 48) {
 			/* Convert long double format
 			 * into internal fpu reg representation
 			 */
-			if (FPU_IS_EMU && (addr < 45) && !(addr % 3)) {
+			if (FPU_IS_EMU && (regno < 45) && !(regno % 3)) {
 				data <<= 15;
 				data = (data & 0xffff0000) |
 				       ((data & 0x0000ffff) >> 1);
 			}
-			child->thread.fp[addr - 21] = data;
+			child->thread.fp[regno - 21] = data;
 		} else
 			goto out_eio;
 		break;
@@ -213,16 +214,16 @@ long arch_ptrace(struct task_struct *child, long request,
 	case PTRACE_GETREGS:	/* Get all gp regs from the child. */
 		for (i = 0; i < 19; i++) {
 			tmp = get_reg(child, i);
-			ret = put_user(tmp, (unsigned long *)data);
+			ret = put_user(tmp, datap);
 			if (ret)
 				break;
-			data += sizeof(unsigned long);
+			datap++;
 		}
 		break;
 
 	case PTRACE_SETREGS:	/* Set all gp regs in the child. */
 		for (i = 0; i < 19; i++) {
-			ret = get_user(tmp, (unsigned long *)data);
+			ret = get_user(tmp, datap);
 			if (ret)
 				break;
 			if (i == PT_SR) {
@@ -230,25 +231,24 @@ long arch_ptrace(struct task_struct *child, long request,
 				tmp |= get_reg(child, PT_SR) & ~SR_MASK;
 			}
 			put_reg(child, i, tmp);
-			data += sizeof(unsigned long);
+			datap++;
 		}
 		break;
 
 	case PTRACE_GETFPREGS:	/* Get the child FPU state. */
-		if (copy_to_user((void *)data, &child->thread.fp,
+		if (copy_to_user(datap, &child->thread.fp,
 				 sizeof(struct user_m68kfp_struct)))
 			ret = -EFAULT;
 		break;
 
 	case PTRACE_SETFPREGS:	/* Set the child FPU state. */
-		if (copy_from_user(&child->thread.fp, (void *)data,
+		if (copy_from_user(&child->thread.fp, datap,
 				   sizeof(struct user_m68kfp_struct)))
 			ret = -EFAULT;
 		break;
 
 	case PTRACE_GET_THREAD_AREA:
-		ret = put_user(task_thread_info(child)->tp_value,
-			       (unsigned long __user *)data);
+		ret = put_user(task_thread_info(child)->tp_value, datap);
 		break;
 
 	default:
-- 
1.7.2.2

[PATCH v2 13/24] ptrace: cleanup arch_ptrace() on m68knommu

[Namhyung Kim]

Use new 'regno', 'datap' variables in order to remove duplicated
expressions and unnecessary castings. Alse remove checking @addr
less than 0 because addr is now unsigned.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Greg Ungerer <gerg@uclinux.org>
---
 arch/m68knommu/kernel/ptrace.c |   58 ++++++++++++++++++---------------------
 1 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/arch/m68knommu/kernel/ptrace.c b/arch/m68knommu/kernel/ptrace.c
index 4ab9448..342baab 100644
--- a/arch/m68knommu/kernel/ptrace.c
+++ b/arch/m68knommu/kernel/ptrace.c
@@ -115,6 +115,8 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	int regno = addr >> 2;
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 		/* read the word at location addr in the USER area. */
@@ -122,71 +124,66 @@ long arch_ptrace(struct task_struct *child, long request,
 			unsigned long tmp;
 			
 			ret = -EIO;
-			if ((addr & 3) || addr < 0 ||
-			    addr > sizeof(struct user) - 3)
+			if ((addr & 3) || addr > sizeof(struct user) - 3)
 				break;
 			
 			tmp = 0;  /* Default return condition */
-			addr = addr >> 2; /* temporary hack. */
 			ret = -EIO;
-			if (addr < 19) {
-				tmp = get_reg(child, addr);
-				if (addr == PT_SR)
+			if (regno < 19) {
+				tmp = get_reg(child, regno);
+				if (regno == PT_SR)
 					tmp >>= 16;
-			} else if (addr >= 21 && addr < 49) {
-				tmp = child->thread.fp[addr - 21];
+			} else if (regno >= 21 && regno < 49) {
+				tmp = child->thread.fp[regno - 21];
 #ifdef CONFIG_M68KFPU_EMU
 				/* Convert internal fpu reg representation
 				 * into long double format
 				 */
-				if (FPU_IS_EMU && (addr < 45) && !(addr % 3))
+				if (FPU_IS_EMU && (regno < 45) && !(regno % 3))
 					tmp = ((tmp & 0xffff0000) << 15) |
 					      ((tmp & 0x0000ffff) << 16);
 #endif
-			} else if (addr == 49) {
+			} else if (regno == 49) {
 				tmp = child->mm->start_code;
-			} else if (addr == 50) {
+			} else if (regno == 50) {
 				tmp = child->mm->start_data;
-			} else if (addr == 51) {
+			} else if (regno == 51) {
 				tmp = child->mm->end_code;
 			} else
 				break;
-			ret = put_user(tmp,(unsigned long *) data);
+			ret = put_user(tmp, datap);
 			break;
 		}
 
 		case PTRACE_POKEUSR: /* write the word at location addr in the USER area */
 			ret = -EIO;
-			if ((addr & 3) || addr < 0 ||
-			    addr > sizeof(struct user) - 3)
+			if ((addr & 3) || addr > sizeof(struct user) - 3)
 				break;
 
-			addr = addr >> 2; /* temporary hack. */
-			    
-			if (addr == PT_SR) {
+			if (regno == PT_SR) {
 				data &= SR_MASK;
 				data <<= 16;
 				data |= get_reg(child, PT_SR) & ~(SR_MASK << 16);
 			}
-			if (addr < 19) {
-				if (put_reg(child, addr, data))
+			if (regno < 19) {
+				if (put_reg(child, regno, data))
 					break;
 				ret = 0;
 				break;
 			}
-			if (addr >= 21 && addr < 48)
+			if (regno >= 21 && regno < 48)
 			{
 #ifdef CONFIG_M68KFPU_EMU
 				/* Convert long double format
 				 * into internal fpu reg representation
 				 */
-				if (FPU_IS_EMU && (addr < 45) && !(addr % 3)) {
+				if (FPU_IS_EMU && (regno < 45) && !(regno % 3)) {
 					data <<= 15;
 					data = (data & 0xffff0000) |
 					       ((data & 0x0000ffff) >> 1);
 				}
 #endif
-				child->thread.fp[addr - 21] = data;
+				child->thread.fp[regno - 21] = data;
 				ret = 0;
 			}
 			break;
@@ -198,11 +195,11 @@ long arch_ptrace(struct task_struct *child, long request,
 			    tmp = get_reg(child, i);
 			    if (i == PT_SR)
 				tmp >>= 16;
-			    if (put_user(tmp, (unsigned long *) data)) {
+			    if (put_user(tmp, datap)) {
 				ret = -EFAULT;
 				break;
 			    }
-			    data += sizeof(unsigned long);
+			    datap++;
 			}
 			ret = 0;
 			break;
@@ -212,7 +209,7 @@ long arch_ptrace(struct task_struct *child, long request,
 			int i;
 			unsigned long tmp;
 			for (i = 0; i < 19; i++) {
-			    if (get_user(tmp, (unsigned long *) data)) {
+			    if (get_user(tmp, datap)) {
 				ret = -EFAULT;
 				break;
 			    }
@@ -222,7 +219,7 @@ long arch_ptrace(struct task_struct *child, long request,
 				tmp |= get_reg(child, PT_SR) & ~(SR_MASK << 16);
 			    }
 			    put_reg(child, i, tmp);
-			    data += sizeof(unsigned long);
+			    datap++;
 			}
 			ret = 0;
 			break;
@@ -231,7 +228,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #ifdef PTRACE_GETFPREGS
 		case PTRACE_GETFPREGS: { /* Get the child FPU state. */
 			ret = 0;
-			if (copy_to_user((void *)data, &child->thread.fp,
+			if (copy_to_user(datap, &child->thread.fp,
 					 sizeof(struct user_m68kfp_struct)))
 				ret = -EFAULT;
 			break;
@@ -241,7 +238,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #ifdef PTRACE_SETFPREGS
 		case PTRACE_SETFPREGS: { /* Set the child FPU state. */
 			ret = 0;
-			if (copy_from_user(&child->thread.fp, (void *)data,
+			if (copy_from_user(&child->thread.fp, datap,
 					   sizeof(struct user_m68kfp_struct)))
 				ret = -EFAULT;
 			break;
@@ -249,8 +246,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #endif
 
 	case PTRACE_GET_THREAD_AREA:
-		ret = put_user(task_thread_info(child)->tp_value,
-			       (unsigned long __user *)data);
+		ret = put_user(task_thread_info(child)->tp_value, datap);
 		break;
 
 		default:
-- 
1.7.2.2

Re: [PATCH v2 13/24] ptrace: cleanup arch_ptrace() on m68knommu

[Greg Ungerer]

Hi Namhyung,

Namhyung Kim wrote:
> Use new 'regno', 'datap' variables in order to remove duplicated
> expressions and unnecessary castings. Alse remove checking @addr
> less than 0 because addr is now unsigned.
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> Cc: Greg Ungerer <gerg@uclinux.org>

Looks ok to me:

Acked-by: Greg Ungerer <gerg@uclinux.org>


> ---
>  arch/m68knommu/kernel/ptrace.c |   58 ++++++++++++++++++---------------------
>  1 files changed, 27 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/m68knommu/kernel/ptrace.c b/arch/m68knommu/kernel/ptrace.c
> index 4ab9448..342baab 100644
> --- a/arch/m68knommu/kernel/ptrace.c
> +++ b/arch/m68knommu/kernel/ptrace.c
> @@ -115,6 +115,8 @@ long arch_ptrace(struct task_struct *child, long request,
>  		 unsigned long addr, unsigned long data)
>  {
>  	int ret;
> +	int regno = addr >> 2;
> +	unsigned long __user *datap = (unsigned long __user *) data;
>  
>  	switch (request) {
>  		/* read the word at location addr in the USER area. */
> @@ -122,71 +124,66 @@ long arch_ptrace(struct task_struct *child, long request,
>  			unsigned long tmp;
>  			
>  			ret = -EIO;
> -			if ((addr & 3) || addr < 0 ||
> -			    addr > sizeof(struct user) - 3)
> +			if ((addr & 3) || addr > sizeof(struct user) - 3)
>  				break;
>  			
>  			tmp = 0;  /* Default return condition */
> -			addr = addr >> 2; /* temporary hack. */
>  			ret = -EIO;
> -			if (addr < 19) {
> -				tmp = get_reg(child, addr);
> -				if (addr == PT_SR)
> +			if (regno < 19) {
> +				tmp = get_reg(child, regno);
> +				if (regno == PT_SR)
>  					tmp >>= 16;
> -			} else if (addr >= 21 && addr < 49) {
> -				tmp = child->thread.fp[addr - 21];
> +			} else if (regno >= 21 && regno < 49) {
> +				tmp = child->thread.fp[regno - 21];
>  #ifdef CONFIG_M68KFPU_EMU
>  				/* Convert internal fpu reg representation
>  				 * into long double format
>  				 */
> -				if (FPU_IS_EMU && (addr < 45) && !(addr % 3))
> +				if (FPU_IS_EMU && (regno < 45) && !(regno % 3))
>  					tmp = ((tmp & 0xffff0000) << 15) |
>  					      ((tmp & 0x0000ffff) << 16);
>  #endif
> -			} else if (addr == 49) {
> +			} else if (regno == 49) {
>  				tmp = child->mm->start_code;
> -			} else if (addr == 50) {
> +			} else if (regno == 50) {
>  				tmp = child->mm->start_data;
> -			} else if (addr == 51) {
> +			} else if (regno == 51) {
>  				tmp = child->mm->end_code;
>  			} else
>  				break;
> -			ret = put_user(tmp,(unsigned long *) data);
> +			ret = put_user(tmp, datap);
>  			break;
>  		}
>  
>  		case PTRACE_POKEUSR: /* write the word at location addr in the USER area */
>  			ret = -EIO;
> -			if ((addr & 3) || addr < 0 ||
> -			    addr > sizeof(struct user) - 3)
> +			if ((addr & 3) || addr > sizeof(struct user) - 3)
>  				break;
>  
> -			addr = addr >> 2; /* temporary hack. */
> -			    
> -			if (addr == PT_SR) {
> +			if (regno == PT_SR) {
>  				data &= SR_MASK;
>  				data <<= 16;
>  				data |= get_reg(child, PT_SR) & ~(SR_MASK << 16);
>  			}
> -			if (addr < 19) {
> -				if (put_reg(child, addr, data))
> +			if (regno < 19) {
> +				if (put_reg(child, regno, data))
>  					break;
>  				ret = 0;
>  				break;
>  			}
> -			if (addr >= 21 && addr < 48)
> +			if (regno >= 21 && regno < 48)
>  			{
>  #ifdef CONFIG_M68KFPU_EMU
>  				/* Convert long double format
>  				 * into internal fpu reg representation
>  				 */
> -				if (FPU_IS_EMU && (addr < 45) && !(addr % 3)) {
> +				if (FPU_IS_EMU && (regno < 45) && !(regno % 3)) {
>  					data <<= 15;
>  					data = (data & 0xffff0000) |
>  					       ((data & 0x0000ffff) >> 1);
>  				}
>  #endif
> -				child->thread.fp[addr - 21] = data;
> +				child->thread.fp[regno - 21] = data;
>  				ret = 0;
>  			}
>  			break;
> @@ -198,11 +195,11 @@ long arch_ptrace(struct task_struct *child, long request,
>  			    tmp = get_reg(child, i);
>  			    if (i == PT_SR)
>  				tmp >>= 16;
> -			    if (put_user(tmp, (unsigned long *) data)) {
> +			    if (put_user(tmp, datap)) {
>  				ret = -EFAULT;
>  				break;
>  			    }
> -			    data += sizeof(unsigned long);
> +			    datap++;
>  			}
>  			ret = 0;
>  			break;
> @@ -212,7 +209,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  			int i;
>  			unsigned long tmp;
>  			for (i = 0; i < 19; i++) {
> -			    if (get_user(tmp, (unsigned long *) data)) {
> +			    if (get_user(tmp, datap)) {
>  				ret = -EFAULT;
>  				break;
>  			    }
> @@ -222,7 +219,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  				tmp |= get_reg(child, PT_SR) & ~(SR_MASK << 16);
>  			    }
>  			    put_reg(child, i, tmp);
> -			    data += sizeof(unsigned long);
> +			    datap++;
>  			}
>  			ret = 0;
>  			break;
> @@ -231,7 +228,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  #ifdef PTRACE_GETFPREGS
>  		case PTRACE_GETFPREGS: { /* Get the child FPU state. */
>  			ret = 0;
> -			if (copy_to_user((void *)data, &child->thread.fp,
> +			if (copy_to_user(datap, &child->thread.fp,
>  					 sizeof(struct user_m68kfp_struct)))
>  				ret = -EFAULT;
>  			break;
> @@ -241,7 +238,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  #ifdef PTRACE_SETFPREGS
>  		case PTRACE_SETFPREGS: { /* Set the child FPU state. */
>  			ret = 0;
> -			if (copy_from_user(&child->thread.fp, (void *)data,
> +			if (copy_from_user(&child->thread.fp, datap,
>  					   sizeof(struct user_m68kfp_struct)))
>  				ret = -EFAULT;
>  			break;
> @@ -249,8 +246,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  #endif
>  
>  	case PTRACE_GET_THREAD_AREA:
> -		ret = put_user(task_thread_info(child)->tp_value,
> -			       (unsigned long __user *)data);
> +		ret = put_user(task_thread_info(child)->tp_value, datap);
>  		break;
>  
>  		default:


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

[PATCH v2 14/24] ptrace: cleanup arch_ptrace() on microblaze

[Namhyung Kim]

Remove checking @addr greater than 0 because @addr is now unsigned.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Michal Simek <monstr@monstr.eu>
---
 arch/microblaze/kernel/ptrace.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/microblaze/kernel/ptrace.c b/arch/microblaze/kernel/ptrace.c
index 3544bc1..05ac8cc 100644
--- a/arch/microblaze/kernel/ptrace.c
+++ b/arch/microblaze/kernel/ptrace.c
@@ -100,7 +100,7 @@ long arch_ptrace(struct task_struct *child, long request,
 			} else {
 				rval = -EIO;
 			}
-		} else if (addr >= 0 && addr < PT_SIZE && (addr & 0x3) == 0) {
+		} else if (addr < PT_SIZE && (addr & 0x3) == 0) {
 			microblaze_reg_t *reg_addr = reg_save_addr(addr, child);
 			if (request == PTRACE_PEEKUSR)
 				val = *reg_addr;
-- 
1.7.2.2

Re: [PATCH v2 14/24] ptrace: cleanup arch_ptrace() on microblaze

[Namhyung Kim]

On Mon, Sep 13, 2010 at 16:14, Michal Simek <monstr@monstr.eu> wrote:
> Hi,
>
> just for my info how does it look like this patch series?
> I mean who should added that patches to mainline?
>

Hello,

I am not sure but I hope Roland will take care of it.

-- 
Regards,
Namhyung Kim

Re: [PATCH v2 14/24] ptrace: cleanup arch_ptrace() on microblaze

[Roland McGrath]

> On Mon, Sep 13, 2010 at 16:14, Michal Simek <monstr@monstr.eu> wrote:
> > Hi,
> >
> > just for my info how does it look like this patch series?
> > I mean who should added that patches to mainline?
> >
> 
> Hello,
> 
> I am not sure but I hope Roland will take care of it.

Sorry, I didn't know you had any such expectation.  Oleg and I do not
autonomously maintain any ptrace tree that gets automatically merged or
anything like that.  Our Acked-by's are presumed to help your case and our
objections to hurt it, but you just have to submit patches to linus, akpm,
arch maintainers, or whatever other channels get merged regularly.  We're
not that kind of authoritative ourselves, though we are listed as the
ptrace maintainers.  We have to do the same with our own ptrace patches.


Thanks,
Roland

Re: [PATCH v2 14/24] ptrace: cleanup arch_ptrace() on microblaze

[Namhyung Kim]

On Tue, Sep 14, 2010 at 11:15, Roland McGrath <roland@redhat.com> wrote:
> Sorry, I didn't know you had any such expectation.  Oleg and I do not
> autonomously maintain any ptrace tree that gets automatically merged or
> anything like that.  Our Acked-by's are presumed to help your case and our
> objections to hurt it, but you just have to submit patches to linus, akpm,
> arch maintainers, or whatever other channels get merged regularly.  We're
> not that kind of authoritative ourselves, though we are listed as the
> ptrace maintainers.  We have to do the same with our own ptrace patches.
>

Got it. Then I'll submit this to akpm but I think I don't have your
Acked-by yet. :-)
Thanks for the information.

-- 
Regards,
Namhyung Kim

Re: [PATCH v2 14/24] ptrace: cleanup arch_ptrace() on microblaze

[Roland McGrath]

> Got it. Then I'll submit this to akpm but I think I don't have your
> Acked-by yet. :-)
> Thanks for the information.

The cosmetic-only version of the generic patches was fine with me,
i.e. where you don't replace put_user with copy_to_user et al.
I don't have a particular say on the various arch code (a few of the arch
maintainers tend to listen to me about this stuff, but it's up to them), so
I didn't respond to those.


Thanks,
Roland

Re: [PATCH v2 14/24] ptrace: cleanup arch_ptrace() on microblaze

[Namhyung Kim]

On Tue, Sep 14, 2010 at 11:31, Roland McGrath <roland@redhat.com> wrote:
> The cosmetic-only version of the generic patches was fine with me,
> i.e. where you don't replace put_user with copy_to_user et al.
> I don't have a particular say on the various arch code (a few of the arch
> maintainers tend to listen to me about this stuff, but it's up to them), so
> I didn't respond to those.
>

OK. That means I can add your Acked-by's on architecture independent
part in this series
which are 01, 03 and (resended) 02, right?

-- 
Regards,
Namhyung Kim

Re: [PATCH v2 14/24] ptrace: cleanup arch_ptrace() on microblaze

[Roland McGrath]

> OK. That means I can add your Acked-by's on architecture independent
> part in this series
> which are 01, 03 and (resended) 02, right?

Yes, those alone should not change the compiled code at all.
If that's so, then they are certainly fine with me.


Thanks,
Roland

[PATCH v2 15/24] ptrace: cleanup arch_ptrace() on MIPS

[Namhyung Kim]

Use new 'addrp', 'datavp' and 'datalp' variables in order to remove
unnecessary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 arch/mips/kernel/ptrace.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index 26c29dd..2465ed1 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -259,6 +259,9 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	void __user *addrp = (void __user *) addr;
+	void __user *datavp = (void __user *) data;
+	unsigned long __user *datalp = (void __user *) data;
 
 	switch (request) {
 	/* when I and D space are separate, these will need to be fixed. */
@@ -387,7 +390,7 @@ long arch_ptrace(struct task_struct *child, long request,
 			ret = -EIO;
 			goto out;
 		}
-		ret = put_user(tmp, (unsigned long __user *) data);
+		ret = put_user(tmp, datalp);
 		break;
 	}
 
@@ -479,34 +482,31 @@ long arch_ptrace(struct task_struct *child, long request,
 		}
 
 	case PTRACE_GETREGS:
-		ret = ptrace_getregs(child, (__s64 __user *) data);
+		ret = ptrace_getregs(child, datavp);
 		break;
 
 	case PTRACE_SETREGS:
-		ret = ptrace_setregs(child, (__s64 __user *) data);
+		ret = ptrace_setregs(child, datavp);
 		break;
 
 	case PTRACE_GETFPREGS:
-		ret = ptrace_getfpregs(child, (__u32 __user *) data);
+		ret = ptrace_getfpregs(child, datavp);
 		break;
 
 	case PTRACE_SETFPREGS:
-		ret = ptrace_setfpregs(child, (__u32 __user *) data);
+		ret = ptrace_setfpregs(child, datavp);
 		break;
 
 	case PTRACE_GET_THREAD_AREA:
-		ret = put_user(task_thread_info(child)->tp_value,
-				(unsigned long __user *) data);
+		ret = put_user(task_thread_info(child)->tp_value, datalp);
 		break;
 
 	case PTRACE_GET_WATCH_REGS:
-		ret = ptrace_get_watch_regs(child,
-					(struct pt_watch_regs __user *) addr);
+		ret = ptrace_get_watch_regs(child, addrp);
 		break;
 
 	case PTRACE_SET_WATCH_REGS:
-		ret = ptrace_set_watch_regs(child,
-					(struct pt_watch_regs __user *) addr);
+		ret = ptrace_set_watch_regs(child, addrp);
 		break;
 
 	default:
-- 
1.7.2.2

[PATCH v2 16/24] ptrace: cleanup arch_ptrace() on mn10300

[Namhyung Kim]

Use new 'datap' variable in order to remove unnecessary castings.
Also remove checking @addr less than 0 because @addr is now unsigned.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
---
 arch/mn10300/kernel/ptrace.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/mn10300/kernel/ptrace.c b/arch/mn10300/kernel/ptrace.c
index ec4b414..5c0b07e 100644
--- a/arch/mn10300/kernel/ptrace.c
+++ b/arch/mn10300/kernel/ptrace.c
@@ -300,27 +300,26 @@ long arch_ptrace(struct task_struct *child, long request,
 {
 	unsigned long tmp;
 	int ret;
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 	/* read the word at location addr in the USER area. */
 	case PTRACE_PEEKUSR:
 		ret = -EIO;
-		if ((addr & 3) || addr < 0 ||
-		    addr > sizeof(struct user) - 3)
+		if ((addr & 3) || addr > sizeof(struct user) - 3)
 			break;
 
 		tmp = 0;  /* Default return condition */
 		if (addr < NR_PTREGS << 2)
 			tmp = get_stack_long(child,
 					     ptrace_regid_to_frame[addr]);
-		ret = put_user(tmp, (unsigned long *) data);
+		ret = put_user(tmp, datap);
 		break;
 
 		/* write the word at location addr in the USER area */
 	case PTRACE_POKEUSR:
 		ret = -EIO;
-		if ((addr & 3) || addr < 0 ||
-		    addr > sizeof(struct user) - 3)
+		if ((addr & 3) || addr > sizeof(struct user) - 3)
 			break;
 
 		ret = 0;
@@ -333,25 +332,25 @@ long arch_ptrace(struct task_struct *child, long request,
 		return copy_regset_to_user(child, &user_mn10300_native_view,
 					   REGSET_GENERAL,
 					   0, NR_PTREGS * sizeof(long),
-					   (void __user *)data);
+					   datap);
 
 	case PTRACE_SETREGS:	/* Set all integer regs in the child. */
 		return copy_regset_from_user(child, &user_mn10300_native_view,
 					     REGSET_GENERAL,
 					     0, NR_PTREGS * sizeof(long),
-					     (const void __user *)data);
+					     datap);
 
 	case PTRACE_GETFPREGS:	/* Get the child FPU state. */
 		return copy_regset_to_user(child, &user_mn10300_native_view,
 					   REGSET_FPU,
 					   0, sizeof(struct fpu_state_struct),
-					   (void __user *)data);
+					   datap);
 
 	case PTRACE_SETFPREGS:	/* Set the child FPU state. */
 		return copy_regset_from_user(child, &user_mn10300_native_view,
 					     REGSET_FPU,
 					     0, sizeof(struct fpu_state_struct),
-					     (const void __user *)data);
+					     datap);
 
 	default:
 		ret = ptrace_request(child, request, addr, data);
-- 
1.7.2.2

[PATCH v2 17/24] ptrace: cleanup arch_ptrace() on parisc

[Namhyung Kim]

Add missing __user markup on the argument of put_user().

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Cc: Helge Deller <deller@gmx.de>
Cc: "James E.J. Bottomley" <jejb@parisc-linux.org>
---
 arch/parisc/kernel/ptrace.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c
index 03920db..2905b1f 100644
--- a/arch/parisc/kernel/ptrace.c
+++ b/arch/parisc/kernel/ptrace.c
@@ -125,7 +125,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		     addr >= sizeof(struct pt_regs))
 			break;
 		tmp = *(unsigned long *) ((char *) task_regs(child) + addr);
-		ret = put_user(tmp, (unsigned long *) data);
+		ret = put_user(tmp, (unsigned long __user *) data);
 		break;
 
 	/* Write the word at location addr in the USER area.  This will need
-- 
1.7.2.2

[PATCH v2 18/24] ptrace: cleanup arch_ptrace() on powerpc

[Namhyung Kim]

Use new 'datavp' and 'datalp' variables in order to remove unnecessary
castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
---
 arch/powerpc/kernel/ptrace.c |   51 +++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index e4b6d75..8d5301c 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1409,26 +1409,28 @@ static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
 static long arch_ptrace_old(struct task_struct *child, long request,
 			    unsigned long addr, unsigned long data)
 {
+	void __user *datavp = (void __user *) data;
+
 	switch (request) {
 	case PPC_PTRACE_GETREGS:	/* Get GPRs 0 - 31. */
 		return copy_regset_to_user(child, &user_ppc_native_view,
 					   REGSET_GPR, 0, 32 * sizeof(long),
-					   (void __user *) data);
+					   datavp);
 
 	case PPC_PTRACE_SETREGS:	/* Set GPRs 0 - 31. */
 		return copy_regset_from_user(child, &user_ppc_native_view,
 					     REGSET_GPR, 0, 32 * sizeof(long),
-					     (const void __user *) data);
+					     datavp);
 
 	case PPC_PTRACE_GETFPREGS:	/* Get FPRs 0 - 31. */
 		return copy_regset_to_user(child, &user_ppc_native_view,
 					   REGSET_FPR, 0, 32 * sizeof(double),
-					   (void __user *) data);
+					   datavp);
 
 	case PPC_PTRACE_SETFPREGS:	/* Set FPRs 0 - 31. */
 		return copy_regset_from_user(child, &user_ppc_native_view,
 					     REGSET_FPR, 0, 32 * sizeof(double),
-					     (const void __user *) data);
+					     datavp);
 	}
 
 	return -EPERM;
@@ -1438,6 +1440,8 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret = -EPERM;
+	void __user *datavp = (void __user *) data;
+	unsigned long __user *datalp = datavp;
 
 	switch (request) {
 	/* read the word at location addr in the USER area. */
@@ -1464,7 +1468,7 @@ long arch_ptrace(struct task_struct *child, long request,
 			tmp = ((unsigned long *)child->thread.fpr)
 				[TS_FPRWIDTH * (index - PT_FPR0)];
 		}
-		ret = put_user(tmp,(unsigned long __user *) data);
+		ret = put_user(tmp, datalp);
 		break;
 	}
 
@@ -1526,11 +1530,11 @@ long arch_ptrace(struct task_struct *child, long request,
 		dbginfo.features = 0;
 #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
 
-		if (!access_ok(VERIFY_WRITE, data,
+		if (!access_ok(VERIFY_WRITE, datavp,
 			       sizeof(struct ppc_debug_info)))
 			return -EFAULT;
-		ret = __copy_to_user((struct ppc_debug_info __user *)data,
-				     &dbginfo, sizeof(struct ppc_debug_info)) ?
+		ret = __copy_to_user(datavp, &dbginfo,
+				     sizeof(struct ppc_debug_info)) ?
 		      -EFAULT : 0;
 		break;
 	}
@@ -1538,11 +1542,10 @@ long arch_ptrace(struct task_struct *child, long request,
 	case PPC_PTRACE_SETHWDEBUG: {
 		struct ppc_hw_breakpoint bp_info;
 
-		if (!access_ok(VERIFY_READ, data,
+		if (!access_ok(VERIFY_READ, datavp,
 			       sizeof(struct ppc_hw_breakpoint)))
 			return -EFAULT;
-		ret = __copy_from_user(&bp_info,
-				       (struct ppc_hw_breakpoint __user *)data,
+		ret = __copy_from_user(&bp_info, datavp,
 				       sizeof(struct ppc_hw_breakpoint)) ?
 		      -EFAULT : 0;
 		if (!ret)
@@ -1561,11 +1564,9 @@ long arch_ptrace(struct task_struct *child, long request,
 		if (addr > 0)
 			break;
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
-		ret = put_user(child->thread.dac1,
-			       (unsigned long __user *)data);
+		ret = put_user(child->thread.dac1, datalp);
 #else
-		ret = put_user(child->thread.dabr,
-			       (unsigned long __user *)data);
+		ret = put_user(child->thread.dabr, datalp);
 #endif
 		break;
 	}
@@ -1581,7 +1582,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		return copy_regset_to_user(child, &user_ppc_native_view,
 					   REGSET_GPR,
 					   0, sizeof(struct pt_regs),
-					   (void __user *) data);
+					   datavp);
 
 #ifdef CONFIG_PPC64
 	case PTRACE_SETREGS64:
@@ -1590,19 +1591,19 @@ long arch_ptrace(struct task_struct *child, long request,
 		return copy_regset_from_user(child, &user_ppc_native_view,
 					     REGSET_GPR,
 					     0, sizeof(struct pt_regs),
-					     (const void __user *) data);
+					     datavp);
 
 	case PTRACE_GETFPREGS: /* Get the child FPU state (FPR0...31 + FPSCR) */
 		return copy_regset_to_user(child, &user_ppc_native_view,
 					   REGSET_FPR,
 					   0, sizeof(elf_fpregset_t),
-					   (void __user *) data);
+					   datavp);
 
 	case PTRACE_SETFPREGS: /* Set the child FPU state (FPR0...31 + FPSCR) */
 		return copy_regset_from_user(child, &user_ppc_native_view,
 					     REGSET_FPR,
 					     0, sizeof(elf_fpregset_t),
-					     (const void __user *) data);
+					     datavp);
 
 #ifdef CONFIG_ALTIVEC
 	case PTRACE_GETVRREGS:
@@ -1610,40 +1611,40 @@ long arch_ptrace(struct task_struct *child, long request,
 					   REGSET_VMX,
 					   0, (33 * sizeof(vector128) +
 					       sizeof(u32)),
-					   (void __user *) data);
+					   datavp);
 
 	case PTRACE_SETVRREGS:
 		return copy_regset_from_user(child, &user_ppc_native_view,
 					     REGSET_VMX,
 					     0, (33 * sizeof(vector128) +
 						 sizeof(u32)),
-					     (const void __user *) data);
+					     datavp);
 #endif
 #ifdef CONFIG_VSX
 	case PTRACE_GETVSRREGS:
 		return copy_regset_to_user(child, &user_ppc_native_view,
 					   REGSET_VSX,
 					   0, 32 * sizeof(double),
-					   (void __user *) data);
+					   datavp);
 
 	case PTRACE_SETVSRREGS:
 		return copy_regset_from_user(child, &user_ppc_native_view,
 					     REGSET_VSX,
 					     0, 32 * sizeof(double),
-					     (const void __user *) data);
+					     datavp);
 #endif
 #ifdef CONFIG_SPE
 	case PTRACE_GETEVRREGS:
 		/* Get the child spe register state. */
 		return copy_regset_to_user(child, &user_ppc_native_view,
 					   REGSET_SPE, 0, 35 * sizeof(u32),
-					   (void __user *) data);
+					   datavp);
 
 	case PTRACE_SETEVRREGS:
 		/* Set the child spe register state. */
 		return copy_regset_from_user(child, &user_ppc_native_view,
 					     REGSET_SPE, 0, 35 * sizeof(u32),
-					     (const void __user *) data);
+					     datavp);
 #endif
 
 	/* Old reverse args ptrace callss */
-- 
1.7.2.2

[PATCH v2 19/24] ptrace: cleanup arch_ptrace() on score

[Namhyung Kim]

Remove unnecessary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Chen Liqin <liqin.chen@sunplusct.com>
Cc: Lennox Wu <lennox.wu@gmail.com>
---
 arch/score/kernel/ptrace.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/score/kernel/ptrace.c b/arch/score/kernel/ptrace.c
index 894dcbf..5583618 100644
--- a/arch/score/kernel/ptrace.c
+++ b/arch/score/kernel/ptrace.c
@@ -336,14 +336,14 @@ arch_ptrace(struct task_struct *child, long request,
 		ret = copy_regset_to_user(child, &user_score_native_view,
 						REGSET_GENERAL,
 						0, sizeof(struct pt_regs),
-						(void __user *)datap);
+						datap);
 		break;
 
 	case PTRACE_SETREGS:
 		ret = copy_regset_from_user(child, &user_score_native_view,
 						REGSET_GENERAL,
 						0, sizeof(struct pt_regs),
-						(const void __user *)datap);
+						datap);
 		break;
 
 	default:
-- 
1.7.2.2

[PATCH v2 20/24] ptrace: cleanup arch_ptrace() on sh

[Namhyung Kim]

Remove unnecessary castings and get rid of dummy pointer in favor of
offsetof() macro in ptrace_32.c. Also use temporary variables and
break long lines in order to improve readability.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mundt <lethal@linux-sh.org>
---
 arch/sh/kernel/ptrace_32.c |   38 +++++++++++++++++++++-----------------
 arch/sh/kernel/ptrace_64.c |   19 ++++++++++++-------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c
index 7464607..b791460 100644
--- a/arch/sh/kernel/ptrace_32.c
+++ b/arch/sh/kernel/ptrace_32.c
@@ -341,7 +341,6 @@ const struct user_regset_view *task_user_regset_view(struct task_struct *task)
 long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
-	struct user * dummy = NULL;
 	unsigned long __user *datap = (unsigned long __user *)data;
 	int ret;
 
@@ -357,17 +356,20 @@ long arch_ptrace(struct task_struct *child, long request,
 
 		if (addr < sizeof(struct pt_regs))
 			tmp = get_stack_long(child, addr);
-		else if (addr >= (unsigned long) &dummy->fpu &&
-			 addr < (unsigned long) &dummy->u_fpvalid) {
+		else if (addr >= offsetof(struct user, fpu) &&
+			 addr < offsetof(struct user, u_fpvalid)) {
 			if (!tsk_used_math(child)) {
-				if (addr == (unsigned long)&dummy->fpu.fpscr)
+				if (addr == offsetof(struct user, fpu.fpscr))
 					tmp = FPSCR_INIT;
 				else
 					tmp = 0;
-			} else
+			} else {
+				unsigned long index;
+				index = addr - offsetof(struct user, fpu);
 				tmp = ((unsigned long *)child->thread.xstate)
-					[(addr - (unsigned long)&dummy->fpu) >> 2];
-		} else if (addr == (unsigned long) &dummy->u_fpvalid)
+					[index >> 2];
+			}
+		} else if (addr == offsetof(struct user, u_fpvalid))
 			tmp = !!tsk_used_math(child);
 		else if (addr == PT_TEXT_ADDR)
 			tmp = child->mm->start_code;
@@ -391,13 +393,15 @@ long arch_ptrace(struct task_struct *child, long request,
 
 		if (addr < sizeof(struct pt_regs))
 			ret = put_stack_long(child, addr, data);
-		else if (addr >= (unsigned long) &dummy->fpu &&
-			 addr < (unsigned long) &dummy->u_fpvalid) {
+		else if (addr >= offsetof(struct user, fpu) &&
+			 addr < offsetof(struct user, u_fpvalid)) {
+			unsigned long index;
+			index = addr - offsetof(struct user, fpu);
 			set_stopped_child_used_math(child);
 			((unsigned long *)child->thread.xstate)
-				[(addr - (unsigned long)&dummy->fpu) >> 2] = data;
+				[index >> 2] = data;
 			ret = 0;
-		} else if (addr == (unsigned long) &dummy->u_fpvalid) {
+		} else if (addr == offsetof(struct user, u_fpvalid)) {
 			conditional_stopped_child_used_math(data, child);
 			ret = 0;
 		}
@@ -407,35 +411,35 @@ long arch_ptrace(struct task_struct *child, long request,
 		return copy_regset_to_user(child, &user_sh_native_view,
 					   REGSET_GENERAL,
 					   0, sizeof(struct pt_regs),
-					   (void __user *)data);
+					   datap);
 	case PTRACE_SETREGS:
 		return copy_regset_from_user(child, &user_sh_native_view,
 					     REGSET_GENERAL,
 					     0, sizeof(struct pt_regs),
-					     (const void __user *)data);
+					     datap);
 #ifdef CONFIG_SH_FPU
 	case PTRACE_GETFPREGS:
 		return copy_regset_to_user(child, &user_sh_native_view,
 					   REGSET_FPU,
 					   0, sizeof(struct user_fpu_struct),
-					   (void __user *)data);
+					   datap);
 	case PTRACE_SETFPREGS:
 		return copy_regset_from_user(child, &user_sh_native_view,
 					     REGSET_FPU,
 					     0, sizeof(struct user_fpu_struct),
-					     (const void __user *)data);
+					     datap);
 #endif
 #ifdef CONFIG_SH_DSP
 	case PTRACE_GETDSPREGS:
 		return copy_regset_to_user(child, &user_sh_native_view,
 					   REGSET_DSP,
 					   0, sizeof(struct pt_dspregs),
-					   (void __user *)data);
+					   datap);
 	case PTRACE_SETDSPREGS:
 		return copy_regset_from_user(child, &user_sh_native_view,
 					     REGSET_DSP,
 					     0, sizeof(struct pt_dspregs),
-					     (const void __user *)data);
+					     datap);
 #endif
 	default:
 		ret = ptrace_request(child, request, addr, data);
diff --git a/arch/sh/kernel/ptrace_64.c b/arch/sh/kernel/ptrace_64.c
index 1367134..1ee5ce6 100644
--- a/arch/sh/kernel/ptrace_64.c
+++ b/arch/sh/kernel/ptrace_64.c
@@ -308,6 +308,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret;
+	unsigned long __user *datap = (unsigned long __user *) data;
 
 	switch (request) {
 	/* read the word at location addr in the USER area. */
@@ -322,13 +323,15 @@ long arch_ptrace(struct task_struct *child, long request,
 			tmp = get_stack_long(child, addr);
 		else if ((addr >= offsetof(struct user, fpu)) &&
 			 (addr <  offsetof(struct user, u_fpvalid))) {
-			tmp = get_fpu_long(child, addr - offsetof(struct user, fpu));
+			unsigned long index;
+			index = addr - offsetof(struct user, fpu);
+			tmp = get_fpu_long(child, index);
 		} else if (addr == offsetof(struct user, u_fpvalid)) {
 			tmp = !!tsk_used_math(child);
 		} else {
 			break;
 		}
-		ret = put_user(tmp, (unsigned long *)data);
+		ret = put_user(tmp, datap);
 		break;
 	}
 
@@ -359,7 +362,9 @@ long arch_ptrace(struct task_struct *child, long request,
 		}
 		else if ((addr >= offsetof(struct user, fpu)) &&
 			 (addr <  offsetof(struct user, u_fpvalid))) {
-			ret = put_fpu_long(child, addr - offsetof(struct user, fpu), data);
+			unsigned long index;
+			index = addr - offsetof(struct user, fpu);
+			ret = put_fpu_long(child, index, data);
 		}
 		break;
 
@@ -367,23 +372,23 @@ long arch_ptrace(struct task_struct *child, long request,
 		return copy_regset_to_user(child, &user_sh64_native_view,
 					   REGSET_GENERAL,
 					   0, sizeof(struct pt_regs),
-					   (void __user *)data);
+					   datap);
 	case PTRACE_SETREGS:
 		return copy_regset_from_user(child, &user_sh64_native_view,
 					     REGSET_GENERAL,
 					     0, sizeof(struct pt_regs),
-					     (const void __user *)data);
+					     datap);
 #ifdef CONFIG_SH_FPU
 	case PTRACE_GETFPREGS:
 		return copy_regset_to_user(child, &user_sh64_native_view,
 					   REGSET_FPU,
 					   0, sizeof(struct user_fpu_struct),
-					   (void __user *)data);
+					   datap);
 	case PTRACE_SETFPREGS:
 		return copy_regset_from_user(child, &user_sh64_native_view,
 					     REGSET_FPU,
 					     0, sizeof(struct user_fpu_struct),
-					     (const void __user *)data);
+					     datap);
 #endif
 	default:
 		ret = ptrace_request(child, request, addr, data);
-- 
1.7.2.2

[PATCH v2 21/24] ptrace: cleanup arch_ptrace() on sparc

[Namhyung Kim]

Factor out struct fps and remove redundant castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 arch/sparc/kernel/ptrace_32.c |   54 +++++++++++++++-------------------------
 arch/sparc/kernel/ptrace_64.c |    8 +++---
 2 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/arch/sparc/kernel/ptrace_32.c b/arch/sparc/kernel/ptrace_32.c
index e08ba4a..27b9e93 100644
--- a/arch/sparc/kernel/ptrace_32.c
+++ b/arch/sparc/kernel/ptrace_32.c
@@ -323,19 +323,35 @@ const struct user_regset_view *task_user_regset_view(struct task_struct *task)
 	return &user_sparc32_view;
 }
 
+struct fps {
+	unsigned long regs[32];
+	unsigned long fsr;
+	unsigned long flags;
+	unsigned long extra;
+	unsigned long fpqd;
+	struct fq {
+		unsigned long *insnaddr;
+		unsigned long insn;
+	} fpq[16];
+};
+
 long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	unsigned long addr2 = current->thread.kregs->u_regs[UREG_I4];
+	void __user *addr2p;
 	const struct user_regset_view *view;
+	struct pt_regs __user *pregs;
+	struct fps __user *fps;
 	int ret;
 
 	view = task_user_regset_view(current);
+	addr2p = (void __user *) addr2;
+	pregs = (struct pt_regs __user *) addr;
+	fps = (struct fps __user *) addr;
 
 	switch(request) {
 	case PTRACE_GETREGS: {
-		struct pt_regs __user *pregs = (struct pt_regs __user *) addr;
-
 		ret = copy_regset_to_user(child, view, REGSET_GENERAL,
 					  32 * sizeof(u32),
 					  4 * sizeof(u32),
@@ -349,8 +365,6 @@ long arch_ptrace(struct task_struct *child, long request,
 	}
 
 	case PTRACE_SETREGS: {
-		struct pt_regs __user *pregs = (struct pt_regs __user *) addr;
-
 		ret = copy_regset_from_user(child, view, REGSET_GENERAL,
 					    32 * sizeof(u32),
 					    4 * sizeof(u32),
@@ -364,19 +378,6 @@ long arch_ptrace(struct task_struct *child, long request,
 	}
 
 	case PTRACE_GETFPREGS: {
-		struct fps {
-			unsigned long regs[32];
-			unsigned long fsr;
-			unsigned long flags;
-			unsigned long extra;
-			unsigned long fpqd;
-			struct fq {
-				unsigned long *insnaddr;
-				unsigned long insn;
-			} fpq[16];
-		};
-		struct fps __user *fps = (struct fps __user *) addr;
-
 		ret = copy_regset_to_user(child, view, REGSET_FP,
 					  0 * sizeof(u32),
 					  32 * sizeof(u32),
@@ -398,19 +399,6 @@ long arch_ptrace(struct task_struct *child, long request,
 	}
 
 	case PTRACE_SETFPREGS: {
-		struct fps {
-			unsigned long regs[32];
-			unsigned long fsr;
-			unsigned long flags;
-			unsigned long extra;
-			unsigned long fpqd;
-			struct fq {
-				unsigned long *insnaddr;
-				unsigned long insn;
-			} fpq[16];
-		};
-		struct fps __user *fps = (struct fps __user *) addr;
-
 		ret = copy_regset_from_user(child, view, REGSET_FP,
 					    0 * sizeof(u32),
 					    32 * sizeof(u32),
@@ -425,8 +413,7 @@ long arch_ptrace(struct task_struct *child, long request,
 
 	case PTRACE_READTEXT:
 	case PTRACE_READDATA:
-		ret = ptrace_readdata(child, addr,
-				      (void __user *) addr2, data);
+		ret = ptrace_readdata(child, addr, addr2p, data);
 
 		if (ret == data)
 			ret = 0;
@@ -436,8 +423,7 @@ long arch_ptrace(struct task_struct *child, long request,
 
 	case PTRACE_WRITETEXT:
 	case PTRACE_WRITEDATA:
-		ret = ptrace_writedata(child, (void __user *) addr2,
-				       addr, data);
+		ret = ptrace_writedata(child, addr2p, addr, data);
 
 		if (ret == data)
 			ret = 0;
diff --git a/arch/sparc/kernel/ptrace_64.c b/arch/sparc/kernel/ptrace_64.c
index d9db5a4..9ccc812 100644
--- a/arch/sparc/kernel/ptrace_64.c
+++ b/arch/sparc/kernel/ptrace_64.c
@@ -976,10 +976,12 @@ long arch_ptrace(struct task_struct *child, long request,
 	unsigned long addr2 = task_pt_regs(current)->u_regs[UREG_I4];
 	struct pt_regs __user *pregs;
 	struct fps __user *fps;
+	void __user *addr2p;
 	int ret;
 
 	pregs = (struct pt_regs __user *) addr;
 	fps = (struct fps __user *) addr;
+	addr2p = (void __user *) addr2;
 
 	switch (request) {
 	case PTRACE_PEEKUSR:
@@ -1030,8 +1032,7 @@ long arch_ptrace(struct task_struct *child, long request,
 
 	case PTRACE_READTEXT:
 	case PTRACE_READDATA:
-		ret = ptrace_readdata(child, addr,
-				      (char __user *)addr2, data);
+		ret = ptrace_readdata(child, addr, addr2p, data);
 		if (ret == data)
 			ret = 0;
 		else if (ret >= 0)
@@ -1040,8 +1041,7 @@ long arch_ptrace(struct task_struct *child, long request,
 
 	case PTRACE_WRITETEXT:
 	case PTRACE_WRITEDATA:
-		ret = ptrace_writedata(child, (char __user *) addr2,
-				       addr, data);
+		ret = ptrace_writedata(child, addr2p, addr, data);
 		if (ret == data)
 			ret = 0;
 		else if (ret >= 0)
-- 
1.7.2.2

[PATCH v2 22/24] ptrace: cleanup arch_ptrace() on tile

[Namhyung Kim]

Remove checking @addr less than 0 because @addr is now unsigned.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Chris Metcalf <cmetcalf@tilera.com>
---
 arch/tile/kernel/ptrace.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/tile/kernel/ptrace.c b/arch/tile/kernel/ptrace.c
index aed9256..704bf11 100644
--- a/arch/tile/kernel/ptrace.c
+++ b/arch/tile/kernel/ptrace.c
@@ -85,7 +85,7 @@ long arch_ptrace(struct task_struct *child, long request,
 	case PTRACE_PEEKUSR:  /* Read register from pt_regs. */
 		if (addr & (sizeof(data)-1))
 			break;
-		if (addr < 0 || addr >= PTREGS_SIZE)
+		if (addr >= PTREGS_SIZE)
 			break;
 		tmp = getreg(child, addr);   /* Read register */
 		ret = put_user(tmp, datap);
@@ -94,7 +94,7 @@ long arch_ptrace(struct task_struct *child, long request,
 	case PTRACE_POKEUSR:  /* Write register in pt_regs. */
 		if (addr & (sizeof(data)-1))
 			break;
-		if (addr < 0 || addr >= PTREGS_SIZE)
+		if (addr >= PTREGS_SIZE)
 			break;
 		putreg(child, addr, data);   /* Write register */
 		ret = 0;
-- 
1.7.2.2

Re: [PATCH v2 22/24] ptrace: cleanup arch_ptrace() on tile

[Chris Metcalf]

 On 9/2/2010 11:46 AM, Namhyung Kim wrote:
> Remove checking @addr less than 0 because @addr is now unsigned.
>
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> ---
>  arch/tile/kernel/ptrace.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/tile/kernel/ptrace.c b/arch/tile/kernel/ptrace.c
> index aed9256..704bf11 100644
> --- a/arch/tile/kernel/ptrace.c
> +++ b/arch/tile/kernel/ptrace.c
> @@ -85,7 +85,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  	case PTRACE_PEEKUSR:  /* Read register from pt_regs. */
>  		if (addr & (sizeof(data)-1))
>  			break;
> -		if (addr < 0 || addr >= PTREGS_SIZE)
> +		if (addr >= PTREGS_SIZE)
>  			break;
>  		tmp = getreg(child, addr);   /* Read register */
>  		ret = put_user(tmp, datap);
> @@ -94,7 +94,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  	case PTRACE_POKEUSR:  /* Write register in pt_regs. */
>  		if (addr & (sizeof(data)-1))
>  			break;
> -		if (addr < 0 || addr >= PTREGS_SIZE)
> +		if (addr >= PTREGS_SIZE)
>  			break;
>  		putreg(child, addr, data);   /* Write register */
>  		ret = 0;

This omits the change to the actual function definition, which was present
in the previous version of this patch.

On the up side, it also removes the change to the sizeof() values in
GETREGS/SETREGS, which seemed unnecessary in the previous version.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH v2 22/24] ptrace: cleanup arch_ptrace() on tile

[Namhyung Kim]

Hello,

On Fri, Sep 3, 2010 at 01:35, Chris Metcalf <cmetcalf@tilera.com> wrote:
>  On 9/2/2010 11:46 AM, Namhyung Kim wrote:
>> Remove checking @addr less than 0 because @addr is now unsigned.
>>
>> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
>> Cc: Chris Metcalf <cmetcalf@tilera.com>
>> ---
>>  arch/tile/kernel/ptrace.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/tile/kernel/ptrace.c b/arch/tile/kernel/ptrace.c
>> index aed9256..704bf11 100644
>> --- a/arch/tile/kernel/ptrace.c
>> +++ b/arch/tile/kernel/ptrace.c
>> @@ -85,7 +85,7 @@ long arch_ptrace(struct task_struct *child, long request,
>>       case PTRACE_PEEKUSR:  /* Read register from pt_regs. */
>>               if (addr & (sizeof(data)-1))
>>                       break;
>> -             if (addr < 0 || addr >= PTREGS_SIZE)
>> +             if (addr >= PTREGS_SIZE)
>>                       break;
>>               tmp = getreg(child, addr);   /* Read register */
>>               ret = put_user(tmp, datap);
>> @@ -94,7 +94,7 @@ long arch_ptrace(struct task_struct *child, long request,
>>       case PTRACE_POKEUSR:  /* Write register in pt_regs. */
>>               if (addr & (sizeof(data)-1))
>>                       break;
>> -             if (addr < 0 || addr >= PTREGS_SIZE)
>> +             if (addr >= PTREGS_SIZE)
>>                       break;
>>               putreg(child, addr, data);   /* Write register */
>>               ret = 0;
>
> This omits the change to the actual function definition, which was present
> in the previous version of this patch.
>

That change was moved/combined into previous patch in this series.
Please check out 03/24 in this patchset also.


> On the up side, it also removes the change to the sizeof() values in
> GETREGS/SETREGS, which seemed unnecessary in the previous version.
>

Yes, it is unnecessary but little bit more accurate although there will be no
machine that has diferent size of long and unsigned long. Anyway if you
really hate it, I'll discard it. :-)


-- 
Regards,
Namhyung Kim

Re: [PATCH v2 22/24] ptrace: cleanup arch_ptrace() on tile

[Chris Metcalf]

 On 9/2/2010 12:55 PM, Namhyung Kim wrote:
> On Fri, Sep 3, 2010 at 01:35, Chris Metcalf <cmetcalf@tilera.com> wrote:
>> This omits the change to the actual function definition, which was present
>> in the previous version of this patch.
> That change was moved/combined into previous patch in this series.
> Please check out 03/24 in this patchset also.

Sure, that looks fine.  And sizeof(unsigned long) is OK; I don't have a
real axe to grind there.

Acked-by: Chris Metcalf <cmetcalf@tilera.com>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

[PATCH v2 23/24] ptrace: cleanup arch_ptrace() on um

[Namhyung Kim]

Remove unnecessary castings using void pointer and fix copy_to_user()
return value. Also add missing __user markup on the argument of
arch_ptrctl().

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Jeff Dike <jdike@addtoit.com>
---
 arch/um/kernel/ptrace.c     |   18 ++++++++----------
 arch/um/sys-x86_64/ptrace.c |    7 +++----
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/um/kernel/ptrace.c b/arch/um/kernel/ptrace.c
index 963d82b..8b85c97 100644
--- a/arch/um/kernel/ptrace.c
+++ b/arch/um/kernel/ptrace.c
@@ -47,6 +47,7 @@ long arch_ptrace(struct task_struct *child, long request,
 {
 	int i, ret;
 	unsigned long __user *p = (void __user *)data;
+	void __user *vp = p;
 
 	switch (request) {
 	/* read word at location addr. */
@@ -108,24 +109,20 @@ long arch_ptrace(struct task_struct *child, long request,
 #endif
 #ifdef PTRACE_GETFPREGS
 	case PTRACE_GETFPREGS: /* Get the child FPU state. */
-		ret = get_fpregs((struct user_i387_struct __user *) data,
-				 child);
+		ret = get_fpregs(vp, child);
 		break;
 #endif
 #ifdef PTRACE_SETFPREGS
 	case PTRACE_SETFPREGS: /* Set the child FPU state. */
-	        ret = set_fpregs((struct user_i387_struct __user *) data,
-				 child);
+	        ret = set_fpregs(vp, child);
 		break;
 #endif
 	case PTRACE_GET_THREAD_AREA:
-		ret = ptrace_get_thread_area(child, addr,
-					     (struct user_desc __user *) data);
+		ret = ptrace_get_thread_area(child, addr, vp);
 		break;
 
 	case PTRACE_SET_THREAD_AREA:
-		ret = ptrace_set_thread_area(child, addr,
-					     (struct user_desc __user *) data);
+		ret = ptrace_set_thread_area(child, addr, datavp);
 		break;
 
 	case PTRACE_FAULTINFO: {
@@ -135,7 +132,8 @@ long arch_ptrace(struct task_struct *child, long request,
 		 * On i386, ptrace_faultinfo is smaller!
 		 */
 		ret = copy_to_user(p, &child->thread.arch.faultinfo,
-				   sizeof(struct ptrace_faultinfo));
+				   sizeof(struct ptrace_faultinfo)) ?
+			-EIO : 0;
 		break;
 	}
 
@@ -159,7 +157,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #ifdef PTRACE_ARCH_PRCTL
 	case PTRACE_ARCH_PRCTL:
 		/* XXX Calls ptrace on the host - needs some SMP thinking */
-		ret = arch_prctl(child, data, (void *) addr);
+		ret = arch_prctl(child, data, (void __user *) addr);
 		break;
 #endif
 	default:
diff --git a/arch/um/sys-x86_64/ptrace.c b/arch/um/sys-x86_64/ptrace.c
index 67e6368..f436136 100644
--- a/arch/um/sys-x86_64/ptrace.c
+++ b/arch/um/sys-x86_64/ptrace.c
@@ -179,15 +179,14 @@ long subarch_ptrace(struct task_struct *child, long request,
 		    unsigned long addr, unsigned long data)
 {
 	int ret = -EIO;
+	void __user *datap = (void __user *) data;
 
 	switch (request) {
 	case PTRACE_GETFPXREGS: /* Get the child FPU state. */
-		ret = get_fpregs((struct user_i387_struct __user *) data,
-				 child);
+		ret = get_fpregs(datap, child);
 		break;
 	case PTRACE_SETFPXREGS: /* Set the child FPU state. */
-		ret = set_fpregs((struct user_i387_struct __user *) data,
-				 child);
+		ret = set_fpregs(datap, child);
 		break;
 	}
 
-- 
1.7.2.2

[PATCH v2 24/24] ptrace: cleanup arch_ptrace() on xtensa

[Namhyung Kim]

Use new 'datap' variable in order to remove unnecessary castings.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Chris Zankel <chris@zankel.net>
---
 arch/xtensa/kernel/ptrace.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/xtensa/kernel/ptrace.c b/arch/xtensa/kernel/ptrace.c
index af9ba80..c72c947 100644
--- a/arch/xtensa/kernel/ptrace.c
+++ b/arch/xtensa/kernel/ptrace.c
@@ -260,6 +260,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	int ret = -EPERM;
+	void __user *datap = (void __user *) data;
 
 	switch (request) {
 	case PTRACE_PEEKTEXT:	/* read word at location addr. */
@@ -268,7 +269,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		break;
 
 	case PTRACE_PEEKUSR:	/* read register specified by addr. */
-		ret = ptrace_peekusr(child, addr, (void __user *) data);
+		ret = ptrace_peekusr(child, addr, datap);
 		break;
 
 	case PTRACE_POKETEXT:	/* write the word at location addr. */
@@ -281,19 +282,19 @@ long arch_ptrace(struct task_struct *child, long request,
 		break;
 
 	case PTRACE_GETREGS:
-		ret = ptrace_getregs(child, (void __user *) data);
+		ret = ptrace_getregs(child, datap);
 		break;
 
 	case PTRACE_SETREGS:
-		ret = ptrace_setregs(child, (void __user *) data);
+		ret = ptrace_setregs(child, datap);
 		break;
 
 	case PTRACE_GETXTREGS:
-		ret = ptrace_getxregs(child, (void __user *) data);
+		ret = ptrace_getxregs(child, datap);
 		break;
 
 	case PTRACE_SETXTREGS:
-		ret = ptrace_setxregs(child, (void __user *) data);
+		ret = ptrace_setxregs(child, datap);
 		break;
 
 	default:
-- 
1.7.2.2

Re: [PATCH v2 03/24] ptrace: change signature of arch_ptrace()

[David Howells]

Namhyung Kim <namhyung@gmail.com> wrote:

> Fix up the arguments to arch_ptrace() to take account of the fact that
> @addr and @data are now unsigned long rather than long as of a preceding
> patch in this series.

Acked-by: David Howells <dhowells@redhat.com>