Generic support for PTRACE_GETREGSET/PTRACE_SETREGSET commands which
export the regsets supported by each architecture using the correponding
NT_* types.
'addr' parameter for the ptrace system call encode the REGSET type and
the buffer length. 'data' parameter points to the user buffer.
ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid,
(NT_TYPE << 20) | buf_length, buf);
For more information on how to use this API by users like debuggers and core
dump for accessing xstate registers, etc., please refer to comments in
arch/x86/include/asm/ptrace-abi.h
Signed-off-by: Suresh Siddha <[email protected]>
Acked-by: Hongjiu Lu <[email protected]>
---
arch/x86/include/asm/ptrace-abi.h | 51 ++++++++++++++++++++++++++++++++++++++
include/linux/elf.h | 25 +++++++++++++++++-
include/linux/ptrace.h | 14 ++++++++++
kernel/ptrace.c | 34 +++++++++++++++++++++++++
4 files changed, 122 insertions(+), 2 deletions(-)
Index: tip/include/linux/ptrace.h
===================================================================
--- tip.orig/include/linux/ptrace.h
+++ tip/include/linux/ptrace.h
@@ -26,6 +26,18 @@
#define PTRACE_GETEVENTMSG 0x4201
#define PTRACE_GETSIGINFO 0x4202
#define PTRACE_SETSIGINFO 0x4203
+#define PTRACE_GETREGSET 0x4204
+#define PTRACE_SETREGSET 0x4205
+
+/*
+ * First 20 bits of the addr in the PTRACE_GETREGSET/PTRACE_SETREGSET requests
+ * are reserved for communicating the user buffer size and the remaining 12 bits
+ * are reserved for the NT_* types (defined in include/linux/elf.h) which
+ * indicate the regset that the ptrace user is interested in.
+ */
+#define PTRACE_REGSET_BUF_SIZE(addr) (addr & 0xfffff)
+#define PTRACE_REGSET_TYPE(addr) ((addr >> 20) & 0xfff)
+#define NOTE_TO_REGSET_TYPE(note) (note & 0xfff)
/* options set using PTRACE_SETOPTIONS */
#define PTRACE_O_TRACESYSGOOD 0x00000001
@@ -114,6 +126,8 @@ static inline void ptrace_unlink(struct
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_regset(struct task_struct *child, long request, long addr,
+ long data);
/**
* task_ptrace - return %PT_* flags that apply to a task
Index: tip/kernel/ptrace.c
===================================================================
--- tip.orig/kernel/ptrace.c
+++ tip/kernel/ptrace.c
@@ -22,6 +22,7 @@
#include <linux/pid_namespace.h>
#include <linux/syscalls.h>
#include <linux/uaccess.h>
+#include <linux/regset.h>
/*
@@ -573,6 +574,11 @@ int ptrace_request(struct task_struct *c
return 0;
return ptrace_resume(child, request, SIGKILL);
+#ifdef PTRACE_EXPORT_REGSET
+ case PTRACE_GETREGSET:
+ case PTRACE_SETREGSET:
+ return generic_ptrace_regset(child, request, addr, data);
+#endif
default:
break;
}
@@ -664,6 +670,34 @@ int generic_ptrace_pokedata(struct task_
return (copied == sizeof(data)) ? 0 : -EIO;
}
+#ifdef PTRACE_EXPORT_REGSET
+int generic_ptrace_regset(struct task_struct *child, long request, long addr,
+ long data)
+{
+ const struct user_regset_view *view = task_user_regset_view(child);
+ unsigned int buf_size = PTRACE_REGSET_BUF_SIZE(addr);
+ int setno;
+
+ for (setno = 0; setno < view->n; setno++) {
+ const struct user_regset *regset = &view->regsets[setno];
+ if (NOTE_TO_REGSET_TYPE(regset->core_note_type) !=
+ PTRACE_REGSET_TYPE(addr))
+ continue;
+
+ if (request == PTRACE_GETREGSET)
+ return copy_regset_to_user(child, view, setno, 0,
+ buf_size,
+ (void __user *) data);
+ else if (request == PTRACE_SETREGSET)
+ return copy_regset_from_user(child, view, setno, 0,
+ buf_size,
+ (void __user *) data);
+ }
+
+ return -EIO;
+}
+#endif
+
#if defined CONFIG_COMPAT
#include <linux/compat.h>
Index: tip/arch/x86/include/asm/ptrace-abi.h
===================================================================
--- tip.orig/arch/x86/include/asm/ptrace-abi.h
+++ tip/arch/x86/include/asm/ptrace-abi.h
@@ -139,4 +139,54 @@ struct ptrace_bts_config {
Returns number of BTS records drained.
*/
+/*
+ * The extended register state using xsave/xrstor infrastructure can be
+ * be read and set by the user using the following ptrace command.
+ *
+ * ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid,
+ * (NT_X86_XSTATE << 20) | xstate_buf_length, xstate_buf);
+ *
+ * The structure layout used for the xstate_buf is same as
+ * the memory layout of xsave used by the processor (except for the bytes
+ * 464..511, which can be used by the software) and hence the size
+ * of this structure varies depending on the features supported by the processor
+ * and OS. The size of the structure that users need to use can be obtained by
+ * doing:
+ * cpuid_count(0xd, 0, &eax, &ptrace_xstateregs_struct_size, &ecx, &edx);
+ * i.e., cpuid.(eax=0xd,ecx=0).ebx will be the size that user (debuggers, etc.)
+ * need to use.
+ *
+ * The format of this structure will be like:
+ * struct {
+ * fxsave_bytes[0..463]
+ * sw_usable_bytes[464..511]
+ * xsave_hdr_bytes[512..575]
+ * avx_bytes[576..831]
+ * future_state etc
+ * }
+ *
+ * The same memory layout will be used for the core-dump NT_X86_XSTATE
+ * note representing the xstate registers.
+ *
+ * For now, only the first 8 bytes of the sw_usable_bytes[464..471] will
+ * be used and will be set to OS enabled xstate mask (which is same as the
+ * 64bit mask returned by the xgetbv's xCR0). Users (analyzing core dump
+ * remotely, etc.) can use this mask as well as the mask saved in the
+ * xstate_hdr bytes and interpret what states the processor/OS supports
+ * and what states are in modified/initialized conditions for the
+ * particular process/thread.
+ *
+ * Also when the user modifies certain state FP/SSE/etc through this
+ * PTRACE_SETXSTATEREGS, they must ensure that the xsave_hdr.xstate_bv
+ * bytes[512..519] of the above memory layout are updated correspondingly.
+ * i.e., for example when FP state is modified to a non-init state,
+ * xsave_hdr.xstate_bv's bit 0 must be set to '1', when SSE is modified to
+ * non-init state, xsave_hdr.xstate_bv's bit 1 must to be set to '1', etc.
+ */
+
+/*
+ * Enable PTRACE_GETREGSET/PTRACE_SETREGSET interface
+ */
+#define PTRACE_EXPORT_REGSET
+
#endif /* _ASM_X86_PTRACE_ABI_H */
Index: tip/include/linux/elf.h
===================================================================
--- tip.orig/include/linux/elf.h
+++ tip/include/linux/elf.h
@@ -349,7 +349,29 @@ typedef struct elf64_shdr {
#define ELF_OSABI ELFOSABI_NONE
#endif
-/* Notes used in ET_CORE */
+/*
+ * Notes used in ET_CORE. Architectures export some of the arch register sets
+ * using the corresponding note types via ptrace
+ * PTRACE_GETREGSET/PTRACE_SETREGSET requests.
+ *
+ * For example,
+ * long addr = (NT_X86_XSTATE << 20 | buf_length);
+ * ptrace(PTRACE_GETREGSET, pid, addr, buf);
+ *
+ * will obtain the x86 extended register state of the pid.
+ *
+ * On 32bit architectures, addr argument is 32bits wide and encodes the length
+ * of the buffer in the first 20 bits, followed by NT_* type encoded in the
+ * remaining 12 bits, representing an unique regset.
+ *
+ * Hence on 32bit architectures, NT_PRXFPREG (0x46e62b7f) will be seen as 0xb7f
+ * and we can't allocate 0xb7f to any other note.
+ *
+ * All the other existing NT_* types fall with in the 12 bits and we have
+ * plenty of room for more NT_* types. For now, on all architectures
+ * we use first 20 bits of the addr for the buffer size and the next 12 bits for
+ * the NT_* type representing the corresponding regset.
+ */
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
@@ -364,7 +386,6 @@ typedef struct elf64_shdr {
#define NT_X86_XSTATE 0x202 /* x86 extended state using xsave */
#define NT_S390_HIGH_GPRS 0x300 /* s390 upper register halves */
-
/* Note header in a PT_NOTE section */
typedef struct elf32_note {
Elf32_Word n_namesz; /* Name size */
Commit-ID: b26d3b32b860e4667df88dc2407f8823e2aa7cc6
Gitweb: http://git.kernel.org/tip/b26d3b32b860e4667df88dc2407f8823e2aa7cc6
Author: Suresh Siddha <[email protected]>
AuthorDate: Tue, 9 Feb 2010 12:13:13 -0800
Committer: H. Peter Anvin <[email protected]>
CommitDate: Tue, 9 Feb 2010 14:10:07 -0800
ptrace: Add support for generic PTRACE_GETREGSET/PTRACE_SETREGSET
Generic support for PTRACE_GETREGSET/PTRACE_SETREGSET commands which
export the regsets supported by each architecture using the correponding
NT_* types.
'addr' parameter for the ptrace system call encode the REGSET type and
the buffer length. 'data' parameter points to the user buffer.
ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid,
(NT_TYPE << 20) | buf_length, buf);
For more information on how to use this API by users like debuggers and core
dump for accessing xstate registers, etc., please refer to comments in
arch/x86/include/asm/ptrace-abi.h
Signed-off-by: Suresh Siddha <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: Hongjiu Lu <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/include/asm/ptrace-abi.h | 50 +++++++++++++++++++++++++++++++++++++
include/linux/elf.h | 25 +++++++++++++++++-
include/linux/ptrace.h | 14 ++++++++++
kernel/ptrace.c | 34 +++++++++++++++++++++++++
4 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/ptrace-abi.h b/arch/x86/include/asm/ptrace-abi.h
index 8672303..965a6ce 100644
--- a/arch/x86/include/asm/ptrace-abi.h
+++ b/arch/x86/include/asm/ptrace-abi.h
@@ -139,4 +139,54 @@ struct ptrace_bts_config {
Returns number of BTS records drained.
*/
+/*
+ * The extended register state using xsave/xrstor infrastructure can be
+ * be read and set by the user using the following ptrace command.
+ *
+ * ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid,
+ * (NT_X86_XSTATE << 20) | xstate_buf_length, xstate_buf);
+ *
+ * The structure layout used for the xstate_buf is same as
+ * the memory layout of xsave used by the processor (except for the bytes
+ * 464..511, which can be used by the software) and hence the size
+ * of this structure varies depending on the features supported by the processor
+ * and OS. The size of the structure that users need to use can be obtained by
+ * doing:
+ * cpuid_count(0xd, 0, &eax, &ptrace_xstateregs_struct_size, &ecx, &edx);
+ * i.e., cpuid.(eax=0xd,ecx=0).ebx will be the size that user (debuggers, etc.)
+ * need to use.
+ *
+ * The format of this structure will be like:
+ * struct {
+ * fxsave_bytes[0..463]
+ * sw_usable_bytes[464..511]
+ * xsave_hdr_bytes[512..575]
+ * avx_bytes[576..831]
+ * future_state etc
+ * }
+ *
+ * The same memory layout will be used for the core-dump NT_X86_XSTATE
+ * note representing the xstate registers.
+ *
+ * For now, only the first 8 bytes of the sw_usable_bytes[464..471] will
+ * be used and will be set to OS enabled xstate mask (which is same as the
+ * 64bit mask returned by the xgetbv's xCR0). Users (analyzing core dump
+ * remotely, etc.) can use this mask as well as the mask saved in the
+ * xstate_hdr bytes and interpret what states the processor/OS supports
+ * and what states are in modified/initialized conditions for the
+ * particular process/thread.
+ *
+ * Also when the user modifies certain state FP/SSE/etc through this
+ * PTRACE_SETXSTATEREGS, they must ensure that the xsave_hdr.xstate_bv
+ * bytes[512..519] of the above memory layout are updated correspondingly.
+ * i.e., for example when FP state is modified to a non-init state,
+ * xsave_hdr.xstate_bv's bit 0 must be set to '1', when SSE is modified to
+ * non-init state, xsave_hdr.xstate_bv's bit 1 must to be set to '1', etc.
+ */
+
+/*
+ * Enable PTRACE_GETREGSET/PTRACE_SETREGSET interface
+ */
+#define PTRACE_EXPORT_REGSET
+
#endif /* _ASM_X86_PTRACE_ABI_H */
diff --git a/include/linux/elf.h b/include/linux/elf.h
index a8c4af0..5fd7996 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -349,7 +349,29 @@ typedef struct elf64_shdr {
#define ELF_OSABI ELFOSABI_NONE
#endif
-/* Notes used in ET_CORE */
+/*
+ * Notes used in ET_CORE. Architectures export some of the arch register sets
+ * using the corresponding note types via ptrace
+ * PTRACE_GETREGSET/PTRACE_SETREGSET requests.
+ *
+ * For example,
+ * long addr = (NT_X86_XSTATE << 20 | buf_length);
+ * ptrace(PTRACE_GETREGSET, pid, addr, buf);
+ *
+ * will obtain the x86 extended register state of the pid.
+ *
+ * On 32bit architectures, addr argument is 32bits wide and encodes the length
+ * of the buffer in the first 20 bits, followed by NT_* type encoded in the
+ * remaining 12 bits, representing an unique regset.
+ *
+ * Hence on 32bit architectures, NT_PRXFPREG (0x46e62b7f) will be seen as 0xb7f
+ * and we can't allocate 0xb7f to any other note.
+ *
+ * All the other existing NT_* types fall with in the 12 bits and we have
+ * plenty of room for more NT_* types. For now, on all architectures
+ * we use first 20 bits of the addr for the buffer size and the next 12 bits for
+ * the NT_* type representing the corresponding regset.
+ */
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
@@ -364,7 +386,6 @@ typedef struct elf64_shdr {
#define NT_X86_XSTATE 0x202 /* x86 extended state using xsave */
#define NT_S390_HIGH_GPRS 0x300 /* s390 upper register halves */
-
/* Note header in a PT_NOTE section */
typedef struct elf32_note {
Elf32_Word n_namesz; /* Name size */
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 56f2d63..4522e5e 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -26,6 +26,18 @@
#define PTRACE_GETEVENTMSG 0x4201
#define PTRACE_GETSIGINFO 0x4202
#define PTRACE_SETSIGINFO 0x4203
+#define PTRACE_GETREGSET 0x4204
+#define PTRACE_SETREGSET 0x4205
+
+/*
+ * First 20 bits of the addr in the PTRACE_GETREGSET/PTRACE_SETREGSET requests
+ * are reserved for communicating the user buffer size and the remaining 12 bits
+ * are reserved for the NT_* types (defined in include/linux/elf.h) which
+ * indicate the regset that the ptrace user is interested in.
+ */
+#define PTRACE_REGSET_BUF_SIZE(addr) (addr & 0xfffff)
+#define PTRACE_REGSET_TYPE(addr) ((addr >> 20) & 0xfff)
+#define NOTE_TO_REGSET_TYPE(note) (note & 0xfff)
/* options set using PTRACE_SETOPTIONS */
#define PTRACE_O_TRACESYSGOOD 0x00000001
@@ -114,6 +126,8 @@ static inline void ptrace_unlink(struct task_struct *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_regset(struct task_struct *child, long request, long addr,
+ long data);
/**
* task_ptrace - return %PT_* flags that apply to a task
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 23bd09c..7040208 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -22,6 +22,7 @@
#include <linux/pid_namespace.h>
#include <linux/syscalls.h>
#include <linux/uaccess.h>
+#include <linux/regset.h>
/*
@@ -573,6 +574,11 @@ int ptrace_request(struct task_struct *child, long request,
return 0;
return ptrace_resume(child, request, SIGKILL);
+#ifdef PTRACE_EXPORT_REGSET
+ case PTRACE_GETREGSET:
+ case PTRACE_SETREGSET:
+ return generic_ptrace_regset(child, request, addr, data);
+#endif
default:
break;
}
@@ -664,6 +670,34 @@ int generic_ptrace_pokedata(struct task_struct *tsk, long addr, long data)
return (copied == sizeof(data)) ? 0 : -EIO;
}
+#ifdef PTRACE_EXPORT_REGSET
+int generic_ptrace_regset(struct task_struct *child, long request, long addr,
+ long data)
+{
+ const struct user_regset_view *view = task_user_regset_view(child);
+ unsigned int buf_size = PTRACE_REGSET_BUF_SIZE(addr);
+ int setno;
+
+ for (setno = 0; setno < view->n; setno++) {
+ const struct user_regset *regset = &view->regsets[setno];
+ if (NOTE_TO_REGSET_TYPE(regset->core_note_type) !=
+ PTRACE_REGSET_TYPE(addr))
+ continue;
+
+ if (request == PTRACE_GETREGSET)
+ return copy_regset_to_user(child, view, setno, 0,
+ buf_size,
+ (void __user *) data);
+ else if (request == PTRACE_SETREGSET)
+ return copy_regset_from_user(child, view, setno, 0,
+ buf_size,
+ (void __user *) data);
+ }
+
+ return -EIO;
+}
+#endif
+
#if defined CONFIG_COMPAT
#include <linux/compat.h>
> 'addr' parameter for the ptrace system call encode the REGSET type and
> the buffer length. 'data' parameter points to the user buffer.
>
> ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid,
> (NT_TYPE << 20) | buf_length, buf);
IMHO this bit swizzling is a non-starter. The NT_* codes can use a full 32
bits. NT_PRXFPREG uses 31 bits. The comments about ignoring the high bits
for this as a special case just seem insane to me. Pass a full 32 bit word
for NT_* and a full word for the buffer size. What's so terrible about
just having an obvious and comprehensible API?
IMHO if you insist on an insane bit swizzling approach, you should mix the
buffer size bits into the request code, leaving the "addr" argument free
for the unmolested NT_* code. There is just no earthly reason that we
should suddenly impose a new and arcane constraint on what NT_* values can
be, even if there is no particular reason for future values not to be small.
> +int generic_ptrace_regset(struct task_struct *child, long request, long addr,
> + long data);
There is no need for a global function for this. It should all be static
in kernel/ptrace.c, called only by ptrace_request()/compat_ptrace_request().
> +#ifdef PTRACE_EXPORT_REGSET
> + case PTRACE_GETREGSET:
> + case PTRACE_SETREGSET:
> + return generic_ptrace_regset(child, request, addr, data);
> +#endif
Just use CONFIG_HAVE_ARCH_TRACEHOOK. That means the arch supplies
task_user_regset_view(). Any additional per-arch conditionalization
defeats the essential purpose of having fully generic ptrace requests.
> + if (NOTE_TO_REGSET_TYPE(regset->core_note_type) !=
> + PTRACE_REGSET_TYPE(addr))
> + continue;
Here is where you should validate the buf_size value. You must not pass a
size that is > regset.size * regset.n or has % regset.size != 0. The arch
user_regset.get and .set functions are not required to check for bogus
offsets or sizes.
You could either just use:
buf_size = MIN(buf_size, regset->n * regset->size);
or you could diagnose it with:
if (buf_size > regset->n * regset->size)
return -EINVAL;
Possibly we might want to allow a PTRACE_GETREGSET with a buffer that is
larger than necessary. Then it would probably make sense to zero-fill the
rest of the buffer. Or else, use an API that can pass back to userland how
much we're actually filling in.
> --- tip.orig/include/linux/elf.h
> +++ tip/include/linux/elf.h
> @@ -349,7 +349,29 @@ typedef struct elf64_shdr {
> #define ELF_OSABI ELFOSABI_NONE
> #endif
>
> -/* Notes used in ET_CORE */
> +/*
> + * Notes used in ET_CORE. Architectures export some of the arch register sets
[...]
These comments don't really belong in linux/elf.h IMHO. They don't add
anything, except documenting the insanity about truncating NT_* values. I
don't think that nutty idea should survive. But regardless, everything
about it belongs alongside the macros used to construct and deconstruct the
argument word. No example should recommend using the bit-twiddling rather
than using some such macros that are made public in linux/ptrace.h.
Thanks,
Roland
On Tue, Feb 9, 2010 at 5:52 PM, Roland McGrath <[email protected]> wrote:
>> 'addr' parameter for the ptrace system call encode the REGSET type and
>> the buffer length. 'data' parameter points to the user buffer.
>>
>> ? ? ? ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid,
>> ? ? ? ? ? ? ?(NT_TYPE << 20) | buf_length, buf);
>
> IMHO this bit swizzling is a non-starter. ?The NT_* codes can use a full 32
> bits. ?NT_PRXFPREG uses 31 bits. ?The comments about ignoring the high bits
> for this as a special case just seem insane to me. ?Pass a full 32 bit word
> for NT_* and a full word for the buffer size. ?What's so terrible about
> just having an obvious and comprehensible API?
>
> IMHO if you insist on an insane bit swizzling approach, you should mix the
> buffer size bits into the request code, leaving the "addr" argument free
> for the unmolested NT_* code. ?There is just no earthly reason that we
> should suddenly impose a new and arcane constraint on what NT_* values can
> be, even if there is no particular reason for future values not to be small.
>
>> +int generic_ptrace_regset(struct task_struct *child, long request, long addr,
>> + ? ? ? ? ? ? ? ? ? ? ? long data);
>
> There is no need for a global function for this. ?It should all be static
> in kernel/ptrace.c, called only by ptrace_request()/compat_ptrace_request().
>
Won't it be called by ptrace emulation in utrace?
--
H.J.
> > There is no need for a global function for this. ?It should all be static
> > in kernel/ptrace.c, called only by ptrace_request()/compat_ptrace_request().
> >
>
> Won't it be called by ptrace emulation in utrace?
I guess I don't really understand the question. I'm not aware of any
patches where any such things are done anywhere except the existing
ptrace_request and compat_ptrace_request functions in kernel/ptrace.c.
Thanks,
Roland
On Tue, Feb 9, 2010 at 7:07 PM, Roland McGrath <[email protected]> wrote:
>> > There is no need for a global function for this. ?It should all be static
>> > in kernel/ptrace.c, called only by ptrace_request()/compat_ptrace_request().
>> >
>>
>> Won't it be called by ptrace emulation in utrace?
>
> I guess I don't really understand the question. ?I'm not aware of any
> patches where any such things are done anywhere except the existing
> ptrace_request and compat_ptrace_request functions in kernel/ptrace.c.
>
I guess it isn't needed to be global then.
--
H.J.
On 02/09, Suresh Siddha wrote:
>
> +#define PTRACE_REGSET_BUF_SIZE(addr) (addr & 0xfffff)
> +#define PTRACE_REGSET_TYPE(addr) ((addr >> 20) & 0xfff)
> +#define NOTE_TO_REGSET_TYPE(note) (note & 0xfff)
>
> /* options set using PTRACE_SETOPTIONS */
> #define PTRACE_O_TRACESYSGOOD 0x00000001
> @@ -114,6 +126,8 @@ static inline void ptrace_unlink(struct
Well. Personally, I like Roland's suggestion more.
How about something like the patch below?
I am not sure how should we check the size, see the comment in
ptrace_regset().
What do you think?
And, I don't understand NOTE_TO_REGSET_TYPE() logic. I mean, I don't know
why we should use "->core_note_type & 0xfff".
Oleg.
--- linux-2.6.32.2/include/linux/ptrace.h~REGSET 2009-12-18 23:27:07.000000000 +0100
+++ linux-2.6.32.2/include/linux/ptrace.h 2010-02-10 14:05:31.000000000 +0100
@@ -27,6 +27,9 @@
#define PTRACE_GETSIGINFO 0x4202
#define PTRACE_SETSIGINFO 0x4203
+#define PTRACE_GETREGSET 0x4204
+#define PTRACE_SETREGSET 0x4205
+
/* options set using PTRACE_SETOPTIONS */
#define PTRACE_O_TRACESYSGOOD 0x00000001
#define PTRACE_O_TRACEFORK 0x00000002
--- linux-2.6.32.2/kernel/ptrace.c~REGSET 2009-12-18 23:27:07.000000000 +0100
+++ linux-2.6.32.2/kernel/ptrace.c 2010-02-10 14:08:12.000000000 +0100
@@ -22,7 +22,7 @@
#include <linux/pid_namespace.h>
#include <linux/syscalls.h>
#include <linux/uaccess.h>
-
+#include <linux/regset.h>
/*
* ptrace a task: make the debugger its new parent and
@@ -397,6 +397,50 @@ int ptrace_writedata(struct task_struct
return copied;
}
+static const struct user_regset *
+find_regset(const struct user_regset_view *view, unsigned int type)
+{
+ const struct user_regset *regset;
+ int n;
+
+ for (n = 0; n < view->n; ++n) {
+ regset = view->regsets + n;
+ if (regset->core_note_type == type)
+ return regset;
+ }
+
+ return NULL;
+}
+
+static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
+ struct iovec *uiov)
+{
+ const struct user_regset_view *view = task_user_regset_view(task);
+ const struct user_regset *regset = find_regset(view, type);
+ struct iovec kiov;
+
+ if (!regset)
+ return -EIO;
+
+ if (copy_from_user(&kiov, uiov, sizeof kiov))
+ return -EFAULT;
+
+ // I am not sure. Afaics it is OK to pass the
+ // size which is less than n * size. If iov_len
+ // is bigger, we can silently truncate it, or
+ // even write the correct value back.
+
+ if (kiov.iov_len != regset->n * regset->size)
+ return -EINVAL;
+
+ if (req == PTRACE_GETREGSET)
+ return copy_regset_to_user(task, view, type, 0,
+ kiov.iov_len, kiov.iov_base);
+ else
+ return copy_regset_from_user(task, view, type, 0,
+ kiov.iov_len, kiov.iov_base);
+}
+
static int ptrace_setoptions(struct task_struct *child, long data)
{
child->ptrace &= ~PT_TRACE_MASK;
@@ -525,6 +569,10 @@ int ptrace_request(struct task_struct *c
case PTRACE_POKEDATA:
return generic_ptrace_pokedata(child, addr, data);
+ case PTRACE_GETREGSET:
+ case PTRACE_SETREGSET:
+ return ptrace_regset(child, request, addr, (void*)data);
+
#ifdef PTRACE_OLDSETOPTIONS
case PTRACE_OLDSETOPTIONS:
#endif
> +static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
> + struct iovec *uiov)
^__user
> +{
> + const struct user_regset_view *view = task_user_regset_view(task);
> + const struct user_regset *regset = find_regset(view, type);
> + struct iovec kiov;
> +
> + if (!regset)
> + return -EIO;
> +
> + if (copy_from_user(&kiov, uiov, sizeof kiov))
> + return -EFAULT;
Since it's just two words, most places handling struct iovec seem to just
use two get_user() calls, which is probably more efficient.
Also, here is where this function would need to be split in half for
compat_ptrace_request() calls where it has to use struct compat_iovec.
> + // I am not sure. Afaics it is OK to pass the
> + // size which is less than n * size. If iov_len
> + // is bigger, we can silently truncate it, or
> + // even write the correct value back.
Modifying iov_len to report how much we accessed seems good to me. If we
do that, we should certainly allow a larger size for get, so userland can
just use a generic large buffer before even knowing the real size. I'm not
sure whether we should allow a smaller size, especially for set. I could
go either way.
Thanks,
Roland
On 02/10/2010 11:12 AM, Roland McGrath wrote:
>
> Since it's just two words, most places handling struct iovec seem to just
> use two get_user() calls, which is probably more efficient.
>
> Also, here is where this function would need to be split in half for
> compat_ptrace_request() calls where it has to use struct compat_iovec.
>
>> + // I am not sure. Afaics it is OK to pass the
>> + // size which is less than n * size. If iov_len
>> + // is bigger, we can silently truncate it, or
>> + // even write the correct value back.
>
> Modifying iov_len to report how much we accessed seems good to me. If we
> do that, we should certainly allow a larger size for get, so userland can
> just use a generic large buffer before even knowing the real size. I'm not
> sure whether we should allow a smaller size, especially for set. I could
> go either way.
>
Allowing a larger size for get seems very sane. Allowing a smaller size
would be ok iff we make sure we handle corner cases right (i.e. a
partially overwritten subregister.)
-hpa
> Allowing a larger size for get seems very sane. Allowing a smaller size
> would be ok iff we make sure we handle corner cases right (i.e. a
> partially overwritten subregister.)
It should enforce the static constraints of the user_regset, which include
a starting position that's 0 % .align (if we had non-zero starting position
in the request) and a size that's 0 % .size. The arch code sets those and
then is not obliged to handle requests outside those constraints. It's
already documented that the arch code is obliged to handle any partial
regset access that meets them.
The usual thing is to set .size and .align to the size of a register.
This is exactly so that the arch code does not need to add gratuitous
corner case complications such as "partially overwritten subregister".
Thanks,
Roland