[PATCH] arch/x86_64/kernel/ptrace.c linux-2.6.14.4

[PATCH] arch/x86_64/kernel/ptrace.c linux-2.6.14.4

[John Blackwood]

Hi Andi,

Our debugger guys noticed that the fs_base and/or gs_base values that
are returned on a ptrace(PTRACE_GETREGS, ...) call on x86_64 systems
don't always seem to be accurate.

It seems that some applications, and also the pthread library, may modify
the fs_base or gs_base values via the modify_ldt() or arch_prctl() system
service calls.  In these cases, the fs_base/gs_base value is saved off
in a different location than in the usual thread.fs or thread.gs fields.

After looking at the process.c and ldt.c source code, I came up with a
attempt at a patch below that seems to figure out the right location to
get the fs/gs base values from.

The read_tls_array() routine below is based off of the read_32bit_tls()
routine in process.c, and the read_ldt() routine below is based off of
the read_ldt() routine in ldt.c.

I suppose that the read_32bit_tls() code could be instead pulled into
a header file such as asm-x86_64/desc.h and shared between process.c
and ptrace.c instead of having the read_tls_array() code below, but I
didn't do that in this version of the patch.

Thank you for your time and considerations in this matter.



--- linux-2.6.14.4/arch/x86_64/kernel/ptrace.c	2005-12-14 
18:50:41.000000000 -0500
+++ new/arch/x86_64/kernel/ptrace.c	2005-12-21 16:32:25.000000000 -0500
@@ -287,6 +287,35 @@ static int putreg(struct task_struct *ch
  	return 0;
  }

+static inline unsigned long read_tls_array(struct task_struct *child, 
int idx)
+{
+	struct desc_struct *desc = (void *)child->thread.tls_array;
+	desc += idx;
+	return (unsigned long)((u32)desc->base0 |
+		(((u32)desc->base1) << 16) | (((u32)desc->base2) << 24));
+}
+
+static unsigned long read_ldt(struct task_struct *child)
+{
+	struct desc_struct *desc;
+	unsigned short index = child->thread.gsindex >> 3;
+	struct mm_struct *mm = child->mm;
+	unsigned long val;
+
+	if (!mm)
+		return 0;
+	down(&mm->context.sem);
+	if (index >= mm->context.size) {
+		up(&mm->context.sem);
+		return 0;
+	}
+	desc = mm->context.ldt;
+	desc += index;
+	val = desc->base0|((u32)desc->base1 << 16)|((u32)desc->base2 << 16);
+	up(&mm->context.sem);
+	return val;
+}
+
  static unsigned long getreg(struct task_struct *child, unsigned long 
regno)
  {
  	unsigned long val;
@@ -300,9 +329,17 @@ static unsigned long getreg(struct task_
  		case offsetof(struct user_regs_struct, es):
  			return child->thread.es;
  		case offsetof(struct user_regs_struct, fs_base):
-			return child->thread.fs;
+			if (!child->thread.fsindex)
+				return child->thread.fs;
+			if (child->thread.fsindex == FS_TLS_SEL)
+				return read_tls_array(child, FS_TLS);
+			return read_ldt(child);
  		case offsetof(struct user_regs_struct, gs_base):
-			return child->thread.gs;
+			if (!child->thread.gsindex)
+				return child->thread.gs;
+			if (child->thread.gsindex == GS_TLS_SEL)
+				return read_tls_array(child, GS_TLS);
+			return read_ldt(child);
  		default:
  			regno = regno - sizeof(struct pt_regs);
  			val = get_stack_long(child, regno);

Re: [PATCH] arch/x86_64/kernel/ptrace.c linux-2.6.14.4

[Andi Kleen]

> I suppose that the read_32bit_tls() code could be instead pulled into
> a header file such as asm-x86_64/desc.h and shared between process.c
> and ptrace.c instead of having the read_tls_array() code below, but I
> didn't do that in this version of the patch.

Please do that. But not as an inline - make it out of line.
And read_ldt also doesn't need to be copied, it's already
in ldt.c and can be exported from there.

Thanks,
-Andi