[PATCH -mm 1/4] powerpc copy_siginfo_from_user32

[PATCH -mm 1/4] powerpc copy_siginfo_from_user32

[Roland McGrath]

Define the copy_siginfo_from_user32 entry point for powerpc, so
that generic CONFIG_COMPAT code can call it.  We already had the
code rolled into compat_sys_rt_sigqueueinfo, this just moves it
out into the canonical function that other arch's define.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/powerpc/kernel/signal_32.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index d840bc7..ad69434 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -621,6 +621,18 @@ int copy_siginfo_to_user32(struct compat_siginfo __user *d, siginfo_t *s)
 
 #define copy_siginfo_to_user	copy_siginfo_to_user32
 
+int copy_siginfo_from_user32(siginfo_t *to, struct compat_siginfo __user *from)
+{
+	memset(to, 0, sizeof *to);
+
+	if (copy_from_user(to, from, 3*sizeof(int)) ||
+	    copy_from_user(to->_sifields._pad,
+			   from->_sifields._pad, SI_PAD_SIZE32))
+		return -EFAULT;
+
+	return 0;
+}
+
 /*
  * Note: it is necessary to treat pid and sig as unsigned ints, with the
  * corresponding cast to a signed int to insure that the proper conversion
@@ -634,9 +646,10 @@ long compat_sys_rt_sigqueueinfo(u32 pid, u32 sig, compat_siginfo_t __user *uinfo
 	int ret;
 	mm_segment_t old_fs = get_fs();
 
-	if (copy_from_user (&info, uinfo, 3*sizeof(int)) ||
-	    copy_from_user (info._sifields._pad, uinfo->_sifields._pad, SI_PAD_SIZE32))
-		return -EFAULT;
+	ret = copy_siginfo_from_user32(&info, uinfo);
+	if (unlikely(ret))
+		return ret;
+
 	set_fs (KERNEL_DS);
 	/* The __user pointer cast is valid becasuse of the set_fs() */
 	ret = sys_rt_sigqueueinfo((int)pid, (int)sig, (siginfo_t __user *) &info);

[PATCH -mm 2/4] ptrace: compat_ptrace_request siginfo

[Roland McGrath]

This adds support for PTRACE_GETSIGINFO and PTRACE_SETSIGINFO in
compat_ptrace_request.  It relies on existing arch definitions for
copy_siginfo_to_user32 and copy_siginfo_from_user32.

On powerpc, this fixes a longstanding regression of 32-bit ptrace
calls on 64-bit kernels vs native calls (64-bit calls or 32-bit
kernels).  This can be seen in a 32-bit call using PTRACE_GETSIGINFO
to examine e.g. siginfo_t.si_addr from a signal that sets it.
(This was broken as of 2.6.24 and, I presume, many or all prior versions.)

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 kernel/ptrace.c |   48 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index fdb34e8..67e392e 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -323,9 +323,8 @@ static int ptrace_setoptions(struct task_struct *child, long data)
 	return (data & ~PTRACE_O_MASK) ? -EINVAL : 0;
 }
 
-static int ptrace_getsiginfo(struct task_struct *child, siginfo_t __user * data)
+static int ptrace_getsiginfo(struct task_struct *child, siginfo_t *info)
 {
-	siginfo_t lastinfo;
 	int error = -ESRCH;
 
 	read_lock(&tasklist_lock);
@@ -333,31 +332,25 @@ static int ptrace_getsiginfo(struct task_struct *child, siginfo_t __user * data)
 		error = -EINVAL;
 		spin_lock_irq(&child->sighand->siglock);
 		if (likely(child->last_siginfo != NULL)) {
-			lastinfo = *child->last_siginfo;
+			*info = *child->last_siginfo;
 			error = 0;
 		}
 		spin_unlock_irq(&child->sighand->siglock);
 	}
 	read_unlock(&tasklist_lock);
-	if (!error)
-		return copy_siginfo_to_user(data, &lastinfo);
 	return error;
 }
 
-static int ptrace_setsiginfo(struct task_struct *child, siginfo_t __user * data)
+static int ptrace_setsiginfo(struct task_struct *child, const siginfo_t *info)
 {
-	siginfo_t newinfo;
 	int error = -ESRCH;
 
-	if (copy_from_user(&newinfo, data, sizeof (siginfo_t)))
-		return -EFAULT;
-
 	read_lock(&tasklist_lock);
 	if (likely(child->sighand != NULL)) {
 		error = -EINVAL;
 		spin_lock_irq(&child->sighand->siglock);
 		if (likely(child->last_siginfo != NULL)) {
-			*child->last_siginfo = newinfo;
+			*child->last_siginfo = *info;
 			error = 0;
 		}
 		spin_unlock_irq(&child->sighand->siglock);
@@ -424,6 +417,7 @@ int ptrace_request(struct task_struct *child, long request,
 		   long addr, long data)
 {
 	int ret = -EIO;
+	siginfo_t siginfo;
 
 	switch (request) {
 	case PTRACE_PEEKTEXT:
@@ -442,12 +436,22 @@ int ptrace_request(struct task_struct *child, long request,
 	case PTRACE_GETEVENTMSG:
 		ret = put_user(child->ptrace_message, (unsigned long __user *) data);
 		break;
+
 	case PTRACE_GETSIGINFO:
-		ret = ptrace_getsiginfo(child, (siginfo_t __user *) data);
+		ret = ptrace_getsiginfo(child, &siginfo);
+		if (!ret)
+			ret = copy_siginfo_to_user((siginfo_t __user *) data,
+						   &siginfo);
 		break;
+
 	case PTRACE_SETSIGINFO:
-		ret = ptrace_setsiginfo(child, (siginfo_t __user *) data);
+		if (copy_from_user(&siginfo, (siginfo_t __user *) data,
+				   sizeof siginfo))
+			ret = -EFAULT;
+		else
+			ret = ptrace_setsiginfo(child, &siginfo);
 		break;
+
 	case PTRACE_DETACH:	 /* detach a process that was attached. */
 		ret = ptrace_detach(child, data);
 		break;
@@ -616,6 +620,7 @@ int compat_ptrace_request(struct task_struct *child, compat_long_t request,
 {
 	compat_ulong_t __user *datap = compat_ptr(data);
 	compat_ulong_t word;
+	siginfo_t siginfo;
 	int ret;
 
 	switch (request) {
@@ -638,6 +643,23 @@ int compat_ptrace_request(struct task_struct *child, compat_long_t request,
 		ret = put_user((compat_ulong_t) child->ptrace_message, datap);
 		break;
 
+	case PTRACE_GETSIGINFO:
+		ret = ptrace_getsiginfo(child, &siginfo);
+		if (!ret)
+			ret = copy_siginfo_to_user32(
+				(struct compat_siginfo __user *) datap,
+				&siginfo);
+		break;
+
+	case PTRACE_SETSIGINFO:
+		memset(&siginfo, 0, sizeof siginfo);
+		if (copy_siginfo_from_user32(
+			    &siginfo, (struct compat_siginfo __user *) datap))
+			ret = -EFAULT;
+		else
+			ret = ptrace_setsiginfo(child, &siginfo);
+		break;
+
 	default:
 		ret = ptrace_request(child, request, addr, data);
 	}

Re: [PATCH -mm 2/4] ptrace: compat_ptrace_request siginfo

[Jeff Dike]

On Thu, Mar 13, 2008 at 01:32:43AM -0700, Roland McGrath wrote:
> On powerpc, this fixes a longstanding regression of 32-bit ptrace
> calls on 64-bit kernels vs native calls (64-bit calls or 32-bit
> kernels).  This can be seen in a 32-bit call using PTRACE_GETSIGINFO
> to examine e.g. siginfo_t.si_addr from a signal that sets it.
> (This was broken as of 2.6.24 and, I presume, many or all prior versions.)

BTW, this also fixes a long-standing bug in x86_64 ptrace32_siginfo:

	ret = sys_ptrace(request, pid, addr, (unsigned long)si);
	if (ret)
		return ret;
	if (request == PTRACE_GETSIGINFO) {
		if (copy_from_user(&ssi, si, sizeof(siginfo_t)))
			return -EFAULT;
		ret = copy_siginfo_to_user32(si32, &ssi);
	}

si comes back with the upper bits of si_code missing, courtesy of
copy_siginfo_to_user:

	err |= __put_user((short)from->si_code, &to->si_code);

causing copy_siginfo_to_user32 to not copy any fields of the union
past the first word because the upper 16 bits are used to figure out
what needs copying.

			Jeff

-- 
Work email - jdike at linux dot intel dot com

[PATCH -mm 3/4] x86_64 ia32 ptrace: use compat_ptrace_request for siginfo

[Roland McGrath]

This removes the special-case handling for PTRACE_GETSIGINFO
and PTRACE_SETSIGINFO from x86_64's sys32_ptrace.  The generic
compat_ptrace_request code handles these.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/x86/kernel/ptrace.c |   30 +-----------------------------
 1 files changed, 1 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 77d9ddd..42305fa 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1184,32 +1184,6 @@ static int genregs32_set(struct task_struct *target,
 	return ret;
 }
 
-static long ptrace32_siginfo(unsigned request, u32 pid, u32 addr, u32 data)
-{
-	siginfo_t __user *si = compat_alloc_user_space(sizeof(siginfo_t));
-	compat_siginfo_t __user *si32 = compat_ptr(data);
-	siginfo_t ssi;
-	int ret;
-
-	if (request == PTRACE_SETSIGINFO) {
-		memset(&ssi, 0, sizeof(siginfo_t));
-		ret = copy_siginfo_from_user32(&ssi, si32);
-		if (ret)
-			return ret;
-		if (copy_to_user(si, &ssi, sizeof(siginfo_t)))
-			return -EFAULT;
-	}
-	ret = sys_ptrace(request, pid, addr, (unsigned long)si);
-	if (ret)
-		return ret;
-	if (request == PTRACE_GETSIGINFO) {
-		if (copy_from_user(&ssi, si, sizeof(siginfo_t)))
-			return -EFAULT;
-		ret = copy_siginfo_to_user32(si32, &ssi);
-	}
-	return ret;
-}
-
 asmlinkage long sys32_ptrace(long request, u32 pid, u32 addr, u32 data)
 {
 	struct task_struct *child;
@@ -1255,11 +1229,9 @@ asmlinkage long sys32_ptrace(long request, u32 pid, u32 addr, u32 data)
 	case PTRACE_SETFPXREGS:
 	case PTRACE_GETFPXREGS:
 	case PTRACE_GETEVENTMSG:
-		break;
-
 	case PTRACE_SETSIGINFO:
 	case PTRACE_GETSIGINFO:
-		return ptrace32_siginfo(request, pid, addr, data);
+		break;
 	}
 
 	child = ptrace_get_task_struct(pid);

[PATCH -mm 4/4] x86_64 ia32 ptrace: convert to compat_arch_ptrace

[Roland McGrath]

Now that there are no more special cases in sys32_ptrace, we
can convert to using the generic compat_sys_ptrace entry point.
The sys32_ptrace function gets simpler and becomes compat_arch_ptrace.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/x86/ia32/ia32entry.S |    2 +-
 arch/x86/kernel/ptrace.c  |   65 +++++---------------------------------------
 include/asm-x86/ptrace.h  |    2 +
 3 files changed, 11 insertions(+), 58 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 8022d3c..b42d009 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -426,7 +426,7 @@ ia32_sys_call_table:
 	.quad sys_setuid16
 	.quad sys_getuid16
 	.quad compat_sys_stime	/* stime */		/* 25 */
-	.quad sys32_ptrace	/* ptrace */
+	.quad compat_sys_ptrace	/* ptrace */
 	.quad sys_alarm
 	.quad sys_fstat	/* (old)fstat */
 	.quad sys_pause
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 42305fa..e36c0f3 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1184,67 +1184,16 @@ static int genregs32_set(struct task_struct *target,
 	return ret;
 }
 
-asmlinkage long sys32_ptrace(long request, u32 pid, u32 addr, u32 data)
+long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
+			compat_ulong_t caddr, compat_ulong_t cdata)
 {
-	struct task_struct *child;
-	struct pt_regs *childregs;
+	unsigned long addr = caddr;
+	unsigned long data = cdata;
 	void __user *datap = compat_ptr(data);
 	int ret;
 	__u32 val;
 
 	switch (request) {
-	case PTRACE_TRACEME:
-	case PTRACE_ATTACH:
-	case PTRACE_KILL:
-	case PTRACE_CONT:
-	case PTRACE_SINGLESTEP:
-	case PTRACE_SINGLEBLOCK:
-	case PTRACE_DETACH:
-	case PTRACE_SYSCALL:
-	case PTRACE_OLDSETOPTIONS:
-	case PTRACE_SETOPTIONS:
-	case PTRACE_SET_THREAD_AREA:
-	case PTRACE_GET_THREAD_AREA:
-	case PTRACE_BTS_CONFIG:
-	case PTRACE_BTS_STATUS:
-	case PTRACE_BTS_SIZE:
-	case PTRACE_BTS_GET:
-	case PTRACE_BTS_CLEAR:
-	case PTRACE_BTS_DRAIN:
-		return sys_ptrace(request, pid, addr, data);
-
-	default:
-		return -EINVAL;
-
-	case PTRACE_PEEKTEXT:
-	case PTRACE_PEEKDATA:
-	case PTRACE_POKEDATA:
-	case PTRACE_POKETEXT:
-	case PTRACE_POKEUSR:
-	case PTRACE_PEEKUSR:
-	case PTRACE_GETREGS:
-	case PTRACE_SETREGS:
-	case PTRACE_SETFPREGS:
-	case PTRACE_GETFPREGS:
-	case PTRACE_SETFPXREGS:
-	case PTRACE_GETFPXREGS:
-	case PTRACE_GETEVENTMSG:
-	case PTRACE_SETSIGINFO:
-	case PTRACE_GETSIGINFO:
-		break;
-	}
-
-	child = ptrace_get_task_struct(pid);
-	if (IS_ERR(child))
-		return PTR_ERR(child);
-
-	ret = ptrace_check_attach(child, request == PTRACE_KILL);
-	if (ret < 0)
-		goto out;
-
-	childregs = task_pt_regs(child);
-
-	switch (request) {
 	case PTRACE_PEEKUSR:
 		ret = getreg32(child, addr, &val);
 		if (ret == 0)
@@ -1290,12 +1239,14 @@ asmlinkage long sys32_ptrace(long request, u32 pid, u32 addr, u32 data)
 					     sizeof(struct user32_fxsr_struct),
 					     datap);
 
+	case PTRACE_GET_THREAD_AREA:
+	case PTRACE_SET_THREAD_AREA:
+		return arch_ptrace(child, request, addr, data);
+
 	default:
 		return compat_ptrace_request(child, request, addr, data);
 	}
 
- out:
-	put_task_struct(child);
 	return ret;
 }
 
diff --git a/include/asm-x86/ptrace.h b/include/asm-x86/ptrace.h
index bc44246..b57dc69 100644
--- a/include/asm-x86/ptrace.h
+++ b/include/asm-x86/ptrace.h
@@ -230,6 +230,8 @@ extern int do_get_thread_area(struct task_struct *p, int idx,
 extern int do_set_thread_area(struct task_struct *p, int idx,
 			      struct user_desc __user *info, int can_allocate);
 
+#define __ARCH_WANT_COMPAT_SYS_PTRACE
+
 #endif /* __KERNEL__ */
 
 #endif /* !__ASSEMBLY__ */

Re: [PATCH -mm 1/4] powerpc copy_siginfo_from_user32

[Andrew Morton]

Problems.

> Subject: [PATCH -mm 1/4] powerpc copy_siginfo_from_user32

This is advertised as a -mm patch but afacit it isn't against -mm.  And it
doesn't seem to be against mainline either?  At least, the fourth patch
fails to apply.

When trying to apply the fourth patch to -mm I hit a reject due to the new
BTS support in git-x86.  I stopped there because the patch might be invalid
because of this.

On Thu, 13 Mar 2008 01:31:07 -0700 (PDT)
Roland McGrath <roland@redhat.com> wrote:

> 
> Define the copy_siginfo_from_user32 entry point for powerpc, so
> that generic CONFIG_COMPAT code can call it.  We already had the
> code rolled into compat_sys_rt_sigqueueinfo, this just moves it
> out into the canonical function that other arch's define.
> 

Even though this appears to be a signal-related patch it is actually
ptrace-related, yes?

This patch is a prerequisite for "ptrace: compat_ptrace_request siginfo",
but this patch is independent from that patch (and from all others) and
hence this patch can be merged on its own into powerpc tree if we wish to go that
way, yes?  (Although probably it would be better not to do it that way, for
sanity's sake).


Anwyay, please help me out with the dependencies here, and take a look at
the x86 BTS stuff?

Thanks.

Re: [PATCH -mm 1/4] powerpc copy_siginfo_from_user32

[Roland McGrath]

> This is advertised as a -mm patch but afacit it isn't against -mm.  And it
> doesn't seem to be against mainline either?  At least, the fourth patch
> fails to apply.

Oops, sorry for the bad labeling.  The baseline I used was actually x86/mm.
(This was originally going to be just some x86_64 cleanups destined for
post-2.6.25, before I noticed that powerpc had issues.)

> When trying to apply the fourth patch to -mm I hit a reject due to the new
> BTS support in git-x86.  I stopped there because the patch might be invalid
> because of this.

The only differences for that stuff should be to pass any new BTS requests
through to arch_ptrace as is done for e.g. PTRACE_GET_THREAD_AREA.  I'll
admit I tried not to think about that since it's disabled now and still in
so much flux.

> Even though this appears to be a signal-related patch it is actually
> ptrace-related, yes?

It is a prerequisite for ptrace cleanups/fixes, yes.  The change does not
affect any signals code behavior.  It may also be useful for other generic
signals compat code later (though I haven't looked into that yet).  I'd
call it "compat-related" really.

> This patch is a prerequisite for "ptrace: compat_ptrace_request siginfo",
> but this patch is independent from that patch (and from all others) and
> hence this patch can be merged on its own into powerpc tree if we wish to
> go that way, yes?  

Yes.

> Anwyay, please help me out with the dependencies here, and take a look at
> the x86 BTS stuff?

2/4 depends on 1/4 for powerpc64 to keep building.
3/4 depends on 2/4 for x86_64 kernel's 32-bit ptrace calls to keep working.
4/4 depends on 3/4 for same.

I've become fairly confused about the various trees and the state of the
ill-fated BTS code.  It's hard to figure out when it's coming and going
from where lately.  But resolving the actual code bits in any conflicts
with that will be trivial.


Thanks,
Roland