PTRACE_GET_THREAD_AREA

PTRACE_GET_THREAD_AREA

[Roland McGrath]

This patch vs 2.5.51 (should apply fine to 2.5.52) adds two new ptrace
requests for i386, PTRACE_GET_THREAD_AREA and PTRACE_SET_THREAD_AREA.
These let another process using ptrace do the equivalent of performing
get_thread_area and set_thread_area system calls for another thread.

We are working on gdb support for the new threading code in the kernel
using the new NPTL library, and use PTRACE_GET_THREAD_AREA for that.
This patch has been working fine for that.

I added PTRACE_SET_THREAD_AREA just for completeness, so that you can
change all the state via ptrace that you can read via ptrace as has
previously been the case.  It doesn't have an equivalent of set_thread_area
with .entry_number = -1, but is otherwise the same.

Both requests use the ptrace `addr' argument for the entry number rather
than the entry_number field in the struct.  The `data' parameter gives the
address of a struct user_desc as used by the set/get_thread_area syscalls.

The code is quite simple, and doesn't need any special synchronization
because in the ptrace context the thread must be stopped already.

I chose the new request numbers arbitrarily from ones not used on i386.
I have no opinion on what values should be used.

People I talked to preferred adding this interface over putting an array of
struct user_desc in struct user as accessed by PTRACE_PEEKUSR/POKEUSR
(which would be a bit unnatural since those calls access one word at a time).


Thanks,
Roland



--- linux-2.5.51/include/asm-i386/ptrace.h.orig	Mon Dec  9 18:45:43 2002
+++ linux-2.5.51/include/asm-i386/ptrace.h	Thu Dec 12 04:42:06 2002
@@ -51,6 +51,10 @@ struct pt_regs {
 
 #define PTRACE_OLDSETOPTIONS         21
 
+#define PTRACE_GET_THREAD_AREA    25
+#define PTRACE_SET_THREAD_AREA    26
+
+
 #ifdef __KERNEL__
 #define user_mode(regs) ((VM_MASK & (regs)->eflags) || (3 & (regs)->xcs))
 #define instruction_pointer(regs) ((regs)->eip)
--- linux-2.5.51/arch/i386/kernel/ptrace.c.orig	Mon Dec  9 18:45:52 2002
+++ linux-2.5.51/arch/i386/kernel/ptrace.c	Thu Dec 12 04:42:12 2002
@@ -21,6 +21,8 @@
 #include <asm/processor.h>
 #include <asm/i387.h>
 #include <asm/debugreg.h>
+#include <asm/ldt.h>
+#include <asm/desc.h>
 
 /*
  * does not yet catch signals sent when the child dies.
@@ -416,6 +418,80 @@ asmlinkage int sys_ptrace(long request, 
 		break;
 	}
 
+	case PTRACE_GET_THREAD_AREA: {
+		int idx = addr;
+		struct user_desc info;
+		struct desc_struct *desc;
+
+/*
+ * Get the current Thread-Local Storage area:
+ */
+
+#define GET_BASE(desc) ( \
+	(((desc)->a >> 16) & 0x0000ffff) | \
+	(((desc)->b << 16) & 0x00ff0000) | \
+	( (desc)->b        & 0xff000000)   )
+
+#define GET_LIMIT(desc) ( \
+	((desc)->a & 0x0ffff) | \
+	 ((desc)->b & 0xf0000) )
+
+#define GET_32BIT(desc)		(((desc)->b >> 23) & 1)
+#define GET_CONTENTS(desc)	(((desc)->b >> 10) & 3)
+#define GET_WRITABLE(desc)	(((desc)->b >>  9) & 1)
+#define GET_LIMIT_PAGES(desc)	(((desc)->b >> 23) & 1)
+#define GET_PRESENT(desc)	(((desc)->b >> 15) & 1)
+#define GET_USEABLE(desc)	(((desc)->b >> 20) & 1)
+
+		if (idx < GDT_ENTRY_TLS_MIN || idx > GDT_ENTRY_TLS_MAX) {
+			ret = -EINVAL;
+			break;
+		}
+
+		desc = child->thread.tls_array + idx - GDT_ENTRY_TLS_MIN;
+
+		info.entry_number = idx;
+		info.base_addr = GET_BASE(desc);
+		info.limit = GET_LIMIT(desc);
+		info.seg_32bit = GET_32BIT(desc);
+		info.contents = GET_CONTENTS(desc);
+		info.read_exec_only = !GET_WRITABLE(desc);
+		info.limit_in_pages = GET_LIMIT_PAGES(desc);
+		info.seg_not_present = !GET_PRESENT(desc);
+		info.useable = GET_USEABLE(desc);
+
+		if (copy_to_user((struct user_desc *) data,
+				 &info, sizeof(info)))
+			ret = -EFAULT;
+		break;
+	}
+
+	case PTRACE_SET_THREAD_AREA: {
+		int idx = addr;
+		struct user_desc info;
+		struct desc_struct *desc;
+
+		if (copy_from_user(&info,
+				   (struct user_desc *) data, sizeof(info))) {
+			ret = -EFAULT;
+			break;
+		}
+		if (idx < GDT_ENTRY_TLS_MIN || idx > GDT_ENTRY_TLS_MAX) {
+			ret = -EINVAL;
+			break;
+		}
+		desc = current->thread.tls_array + idx - GDT_ENTRY_TLS_MIN;
+		if (LDT_empty(&info)) {
+			desc->a = 0;
+			desc->b = 0;
+		} else {
+			desc->a = LDT_entry_a(&info);
+			desc->b = LDT_entry_b(&info);
+		}
+		ret = 0;
+		break;
+	}
+
 	default:
 		ret = ptrace_request(child, request, addr, data);
 		break;

Re: PTRACE_GET_THREAD_AREA

[Christoph Hellwig]

On Fri, Dec 20, 2002 at 12:32:41AM -0800, Roland McGrath wrote:
> This patch vs 2.5.51 (should apply fine to 2.5.52) adds two new ptrace
> requests for i386, PTRACE_GET_THREAD_AREA and PTRACE_SET_THREAD_AREA.
> These let another process using ptrace do the equivalent of performing
> get_thread_area and set_thread_area system calls for another thread.

I don't think ptrace is the right interface for this.  Just changed
the get_thread_area/set_thread_area to take a new first pid_t argument.

Of course you might have to check privilegues if the first argument is
non-null (i.e. not yourself).

Re: PTRACE_GET_THREAD_AREA

[Daniel Jacobowitz]

On Fri, Dec 20, 2002 at 10:24:31AM +0000, Christoph Hellwig wrote:
> On Fri, Dec 20, 2002 at 12:32:41AM -0800, Roland McGrath wrote:
> > This patch vs 2.5.51 (should apply fine to 2.5.52) adds two new ptrace
> > requests for i386, PTRACE_GET_THREAD_AREA and PTRACE_SET_THREAD_AREA.
> > These let another process using ptrace do the equivalent of performing
> > get_thread_area and set_thread_area system calls for another thread.
> 
> I don't think ptrace is the right interface for this.  Just changed
> the get_thread_area/set_thread_area to take a new first pid_t argument.
> 
> Of course you might have to check privilegues if the first argument is
> non-null (i.e. not yourself).

I have to disagree.  These are things which you should never do to
another process unless you're ptracing them; get_thread_area with a PID
is not generally useful.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: PTRACE_GET_THREAD_AREA

[Linus Torvalds]

In article <20021220102431.A26923@infradead.org>,
Christoph Hellwig  <hch@infradead.org> wrote:
>
>I don't think ptrace is the right interface for this.  Just changed
>the get_thread_area/set_thread_area to take a new first pid_t argument.

No.  There is _no_ excuse for even looking at (much less changing)
another process' thread area unless you are tracing that process. 

Basically, there is only _one_ valid user of getting/setting the thread
area of somebody else, and that's for debugging. And debuggers use
ptrace. It's as simple as that. They use ptrace to read and set memory
contents, they use ptrace to read and set registers, and they should use
ptrace to check status bits of the process.

We do not introduce any extensions to existing system calls for
debuggers. We already have the interface, and one that does a lot better
at checking permissions in _one_ place than it would be to have magic
"can this process read/modify another process" things.

		Linus

Re: PTRACE_GET_THREAD_AREA

[Daniel Jacobowitz]

On Fri, Dec 20, 2002 at 12:32:41AM -0800, Roland McGrath wrote:
> This patch vs 2.5.51 (should apply fine to 2.5.52) adds two new ptrace
> requests for i386, PTRACE_GET_THREAD_AREA and PTRACE_SET_THREAD_AREA.
> These let another process using ptrace do the equivalent of performing
> get_thread_area and set_thread_area system calls for another thread.
> 
> We are working on gdb support for the new threading code in the kernel
> using the new NPTL library, and use PTRACE_GET_THREAD_AREA for that.
> This patch has been working fine for that.
> 
> I added PTRACE_SET_THREAD_AREA just for completeness, so that you can
> change all the state via ptrace that you can read via ptrace as has
> previously been the case.  It doesn't have an equivalent of set_thread_area
> with .entry_number = -1, but is otherwise the same.
> 
> Both requests use the ptrace `addr' argument for the entry number rather
> than the entry_number field in the struct.  The `data' parameter gives the
> address of a struct user_desc as used by the set/get_thread_area syscalls.
> 
> The code is quite simple, and doesn't need any special synchronization
> because in the ptrace context the thread must be stopped already.
> 
> I chose the new request numbers arbitrarily from ones not used on i386.
> I have no opinion on what values should be used.
> 
> People I talked to preferred adding this interface over putting an array of
> struct user_desc in struct user as accessed by PTRACE_PEEKUSR/POKEUSR
> (which would be a bit unnatural since those calls access one word at a time).

In general, I like this.  However, I have to ask one question: how much
of the i386-centrism of this patch is actually necessary?  What
information does GDB _use_ from this, and is there some way we can
expose it that will be useful in other places?

Eventually most or all targets will have thread-specific data
implemented; I don't want to have to redo this for each one.

Your choice of numbers is fine if this remains i386-centric; if we
expect there to be a common interface then it should go in
<linux/ptrace.h> and the 0x4200 range instead.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: PTRACE_GET_THREAD_AREA

[Jakub Jelinek]

On Fri, Dec 20, 2002 at 10:48:29AM -0500, Daniel Jacobowitz wrote:
> Eventually most or all targets will have thread-specific data
> implemented; I don't want to have to redo this for each one.

Well, but on most arches you don't need any kernel support for TLS.
On sparc32/sparc64/IA-64/s390/s390x and others you simply have one
general register reserved for it by the ABI, on Alpha you use a PAL
call.
set_thread_area/get_thread_area is IA-32/x86-64 specific.

	Jakub

Re: PTRACE_GET_THREAD_AREA

[Daniel Jacobowitz]

On Fri, Dec 20, 2002 at 10:55:16AM -0500, Jakub Jelinek wrote:
> On Fri, Dec 20, 2002 at 10:48:29AM -0500, Daniel Jacobowitz wrote:
> > Eventually most or all targets will have thread-specific data
> > implemented; I don't want to have to redo this for each one.
> 
> Well, but on most arches you don't need any kernel support for TLS.
> On sparc32/sparc64/IA-64/s390/s390x and others you simply have one
> general register reserved for it by the ABI, on Alpha you use a PAL
> call.
> set_thread_area/get_thread_area is IA-32/x86-64 specific.

Oh, right, I guess we won't need anything if it's just in a register :)
Architecture-specific it is, then.  For Alpha (and probably MIPS) we
can add a ptrace equivalent of the trap to fetch the pointer.  Patch
looks fine to me.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: PTRACE_GET_THREAD_AREA

[Roland McGrath]

> In general, I like this.  However, I have to ask one question: how much
> of the i386-centrism of this patch is actually necessary?  What
> information does GDB _use_ from this, and is there some way we can expose
> it that will be useful in other places?

GDB uses this only at the behest of NPTL's libthread_db, and it only ever
wants the base address of a descriptor presumed to be a flat 4GB data
segment.  So that is just one word per index that you might really be using
in practice.  The reason I made all the information available was the
simple principle that that's what ptrace is for--ideally gdb ought to fetch
the descriptors itself whenever a segment register has a nonstandard value,
and show the user the full details so as not to presume what is going on in
the program.  But I don't have a direct practical need to care about that.

It is also unusual to i386 that there are multiple slots you might want to
ask about, and that the number of potential slots actually available is a
kernel implementation detail rather than an instrinsic architecture limit,
and that the slot indices you know about from register state have a nonzero
origin whose value is also a kernel implementation detail.  Using
PTRACE_PEEKUSR, whether to access just the base-address word or all three
words of each virtual segment descriptor, would encode GDT_ENTRY_TLS_ENTRIES
into the struct user layout and require the user to know GDT_ENTRY_TLS_MIN.

> Eventually most or all targets will have thread-specific data
> implemented; I don't want to have to redo this for each one.

Most architectures use a normal register.  The only other architectures
that do something different AFAIK are x86-64 and Alpha.  I don't know of
anything other than i386 where there is anything other than a plain word
for each slot.  On x86-64 there are exactly two possible slots in the
architecture (fs_base, gs_base); these are already in struct user, so
nothing new is needed.  On Alpha, I suppose anything could be possible
since it's implemented in PAL code, but in reality there is exactly one
slot and if that's not in struct user then it could be.

> Your choice of numbers is fine if this remains i386-centric; if we
> expect there to be a common interface then it should go in
> <linux/ptrace.h> and the 0x4200 range instead.

The interface in my patches is i386-specific.  If we choose a common
interface, it probably won't look like that one or use those names.

A machine-independent interface without the problems cited above would be
something like ptrace(PTRACE_GET_TLS, idx, &word), where IDX is a
machine-dependent selector such as segment register value on i386, 0/1 for
fs/gs on x86-64, and ignored on Alpha.  (For i386, that could also fetch
LDT values I suppose.)

Re: PTRACE_GET_THREAD_AREA

[Linus Torvalds]

In article <200212200832.gBK8Wfg29816@magilla.sf.frob.com>,
Roland McGrath  <roland@redhat.com> wrote:
>This patch vs 2.5.51 (should apply fine to 2.5.52) adds two new ptrace
>requests for i386, PTRACE_GET_THREAD_AREA and PTRACE_SET_THREAD_AREA.
>These let another process using ptrace do the equivalent of performing
>get_thread_area and set_thread_area system calls for another thread.

Looks fine, except I'd ask you to split up the get/set logic as separate
functions, instead of making that case-statement thing horribly big.

Big functions are bad.

So please make it look something like

	case PTRACE_GET_THREAD_AREA:
		ret = ptrace_get_area(addr, (struct user_desc *) data);
		break;

	case PTRACE_SET_THREAD_AREA:
		ret = ptrace_set_area(addr, (struct user_desc *) data);
		break;

instead, ok?

		Linus

Re: PTRACE_GET_THREAD_AREA

[Roland McGrath]

Hi Linus.  I didn't get your reply until just recently when someone
forwarded it to me, because I'm not on the mailing list.  Please be sure to
include <roland@redhat.com> directly on any replies you make.

> Looks fine, except I'd ask you to split up the get/set logic as separate
> functions, instead of making that case-statement thing horribly big.

No problem.  Here is a new version of the patch.  Please let me know if
there is any reason this can't go in ASAP.


Thanks,
Roland


--- linux-2.5.54/include/asm-i386/ptrace.h.orig	Mon Dec  9 18:45:43 2002
+++ linux-2.5.54/include/asm-i386/ptrace.h	Fri Jan 10 16:39:44 2003
@@ -51,6 +51,9 @@ struct pt_regs {
 
 #define PTRACE_OLDSETOPTIONS         21
 
+#define PTRACE_GET_THREAD_AREA    25
+#define PTRACE_SET_THREAD_AREA    26
+
 #ifdef __KERNEL__
 #define user_mode(regs) ((VM_MASK & (regs)->eflags) || (3 & (regs)->xcs))
 #define instruction_pointer(regs) ((regs)->eip)
[Exit 1]
--- linux-2.5.54/arch/i386/kernel/ptrace.c.orig	Fri Jan 10 16:40:18 2003
+++ linux-2.5.54/arch/i386/kernel/ptrace.c	Fri Jan 10 12:38:30 2003
@@ -21,6 +21,8 @@
 #include <asm/processor.h>
 #include <asm/i387.h>
 #include <asm/debugreg.h>
+#include <asm/ldt.h>
+#include <asm/desc.h>
 
 /*
  * does not yet catch signals sent when the child dies.
@@ -148,6 +150,85 @@ void ptrace_disable(struct task_struct *
 	put_stack_long(child, EFL_OFFSET, tmp);
 }
 
+/*
+ * Perform get_thread_area on behalf of the traced child.
+ */
+static int
+ptrace_get_thread_area(struct task_struct *child,
+		       int idx, struct user_desc *user_desc)
+{
+	struct user_desc info;
+	struct desc_struct *desc;
+
+/*
+ * Get the current Thread-Local Storage area:
+ */
+
+#define GET_BASE(desc) ( \
+	(((desc)->a >> 16) & 0x0000ffff) | \
+	(((desc)->b << 16) & 0x00ff0000) | \
+	( (desc)->b        & 0xff000000)   )
+
+#define GET_LIMIT(desc) ( \
+	((desc)->a & 0x0ffff) | \
+	 ((desc)->b & 0xf0000) )
+
+#define GET_32BIT(desc)		(((desc)->b >> 23) & 1)
+#define GET_CONTENTS(desc)	(((desc)->b >> 10) & 3)
+#define GET_WRITABLE(desc)	(((desc)->b >>  9) & 1)
+#define GET_LIMIT_PAGES(desc)	(((desc)->b >> 23) & 1)
+#define GET_PRESENT(desc)	(((desc)->b >> 15) & 1)
+#define GET_USEABLE(desc)	(((desc)->b >> 20) & 1)
+
+	if (idx < GDT_ENTRY_TLS_MIN || idx > GDT_ENTRY_TLS_MAX)
+		return -EINVAL;
+
+	desc = child->thread.tls_array + idx - GDT_ENTRY_TLS_MIN;
+
+	info.entry_number = idx;
+	info.base_addr = GET_BASE(desc);
+	info.limit = GET_LIMIT(desc);
+	info.seg_32bit = GET_32BIT(desc);
+	info.contents = GET_CONTENTS(desc);
+	info.read_exec_only = !GET_WRITABLE(desc);
+	info.limit_in_pages = GET_LIMIT_PAGES(desc);
+	info.seg_not_present = !GET_PRESENT(desc);
+	info.useable = GET_USEABLE(desc);
+
+	if (copy_to_user(user_desc, &info, sizeof(info)))
+		return -EFAULT;
+
+	return 0;
+}
+
+/*
+ * Perform set_thread_area on behalf of the traced child.
+ */
+static int
+ptrace_set_thread_area(struct task_struct *child,
+		       int idx, struct user_desc *user_desc)
+{
+	struct user_desc info;
+	struct desc_struct *desc;
+
+	if (copy_from_user(&info, user_desc, sizeof(info)))
+		return -EFAULT;
+
+	if (idx < GDT_ENTRY_TLS_MIN || idx > GDT_ENTRY_TLS_MAX)
+		return -EINVAL;
+
+	desc = child->thread.tls_array + idx - GDT_ENTRY_TLS_MIN;
+	if (LDT_empty(&info)) {
+		desc->a = 0;
+		desc->b = 0;
+	} else {
+		desc->a = LDT_entry_a(&info);
+		desc->b = LDT_entry_b(&info);
+	}
+
+	return 0;
+}
+
 asmlinkage int sys_ptrace(long request, long pid, long addr, long data)
 {
 	struct task_struct *child;
@@ -416,6 +497,16 @@ asmlinkage int sys_ptrace(long request, 
 		break;
 	}
 
+	case PTRACE_GET_THREAD_AREA:
+		ret = ptrace_get_thread_area(child,
+					     addr, (struct user_desc *) data);
+		break;
+
+	case PTRACE_SET_THREAD_AREA:
+		ret = ptrace_set_thread_area(child,
+					     addr, (struct user_desc *) data);
+		break;
+
 	default:
 		ret = ptrace_request(child, request, addr, data);
 		break;

Re: PTRACE_GET_THREAD_AREA

[Linus Torvalds]

On Sun, 12 Jan 2003, Roland McGrath wrote:
> Hi Linus.  I didn't get your reply until just recently when someone
> forwarded it to me, because I'm not on the mailing list.  Please be sure to
> include <roland@redhat.com> directly on any replies you make.

Hmm.. I usually reply to all, but whatever. 

Anyway, applied.

		Linus

Re: PTRACE_GET_THREAD_AREA

[Roland McGrath]

> Hmm.. I usually reply to all, but whatever. 

I got this one.

> Anyway, applied.

Thanks muchly.


Roland

Re: PTRACE_GET_THREAD_AREA

[Roland McGrath]

> i've just seen your patch submitted to 2.5 and i think there's a mistake
> in there. this:
> 
> #define GET_32BIT(desc)		(((desc)->b >> 23) & 1)
> 
> should be
> 
> #define GET_32BIT(desc)		(((desc)->b >> 22) & 1)
> 
> i.e., bit 22 determines the segment bitness (default address/operand
> size), whereas bit 23 determines the granularity used to interpret the
> segment limit.

Those macros in my patch are copied directly from process.c where
get_thread_area is implemented.  Since the purpose of
PTRACE_GET_THREAD_AREA is to match what get_thread_area does, it's correct
that it match.  

But I think you are right that the definition is wrong.  It's obviously
suspect that two things are defined using the same bit:

#define GET_32BIT(desc)		(((desc)->b >> 23) & 1)
#define GET_LIMIT_PAGES(desc)	(((desc)->b >> 23) & 1)

This should be fixed in both process.c and ptrace.c together.