Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/28/2014 05:58 AM, Andy Lutomirski wrote: On 01/26/2014 01:08 AM, Qiaowei Ren wrote: (Why on earth does Intel not expose this stuff in cr2 or an MSR or something?) I guess it is due to some design reason. Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/26/2014 01:08 AM, Qiaowei Ren wrote:
> This patch adds new fields about bound violation into siginfo
> structure. si_lower and si_upper are respectively lower bound
> and upper bound when bound violation is caused.
>
> These fields will be set in #BR exception handler by decoding
> the user instruction and constructing the faulting pointer.
> A userspace application can get violation address, lower bound
> and upper bound for bound violation from this new siginfo structure.
>
> Signed-off-by: Qiaowei Ren
> ---
> arch/x86/include/asm/mpx.h | 19 +++
> arch/x86/kernel/mpx.c | 287
>
> arch/x86/kernel/traps.c|6 +
> include/uapi/asm-generic/siginfo.h |9 +-
> kernel/signal.c|4 +
> 5 files changed, 324 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
> index 9652e9e..e099573 100644
> --- a/arch/x86/include/asm/mpx.h
> +++ b/arch/x86/include/asm/mpx.h
> @@ -3,6 +3,7 @@
>
> #include
> #include
> +#include
>
> #ifdef CONFIG_X86_64
>
> @@ -30,6 +31,22 @@
>
> #endif
>
> +struct mpx_insn {
> + struct insn_field rex_prefix; /* REX prefix */
> + struct insn_field modrm;
> + struct insn_field sib;
> + struct insn_field displacement;
> +
> + unsigned char addr_bytes; /* effective address size */
> + unsigned char limit;
> + unsigned char x86_64;
> +
> + const unsigned char *kaddr; /* kernel address of insn to analyze */
> + const unsigned char *next_byte;
> +};
> +
> +#define MAX_MPX_INSN_SIZE15
> +
> typedef union {
> struct {
> unsigned long ignored:MPX_IGN_BITS;
> @@ -40,5 +57,7 @@ typedef union {
> } mpx_addr;
>
> void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
> +void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
> + struct xsave_struct *xsave_buf);
>
> #endif /* _ASM_X86_MPX_H */
> diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
> index 9e91178..983abf7 100644
> --- a/arch/x86/kernel/mpx.c
> +++ b/arch/x86/kernel/mpx.c
> @@ -91,6 +91,269 @@ int mpx_release(struct task_struct *tsk)
> return 0;
> }
>
> +typedef enum {REG_TYPE_RM, REG_TYPE_INDEX, REG_TYPE_BASE} reg_type_t;
> +static unsigned long get_reg(struct mpx_insn *insn, struct pt_regs *regs,
> + reg_type_t type)
> +{
> + int regno = 0;
> + unsigned char modrm = (unsigned char)insn->modrm.value;
> + unsigned char sib = (unsigned char)insn->sib.value;
> +
> + static const int regoff[] = {
> + offsetof(struct pt_regs, ax),
> + offsetof(struct pt_regs, cx),
> + offsetof(struct pt_regs, dx),
> + offsetof(struct pt_regs, bx),
> + offsetof(struct pt_regs, sp),
> + offsetof(struct pt_regs, bp),
> + offsetof(struct pt_regs, si),
> + offsetof(struct pt_regs, di),
> +#ifdef CONFIG_X86_64
> + offsetof(struct pt_regs, r8),
> + offsetof(struct pt_regs, r9),
> + offsetof(struct pt_regs, r10),
> + offsetof(struct pt_regs, r11),
> + offsetof(struct pt_regs, r12),
> + offsetof(struct pt_regs, r13),
> + offsetof(struct pt_regs, r14),
> + offsetof(struct pt_regs, r15),
> +#endif
> + };
> +
> + switch (type) {
> + case REG_TYPE_RM:
> + regno = X86_MODRM_RM(modrm);
> + if (X86_REX_B(insn->rex_prefix.value) == 1)
> + regno += 8;
> + break;
> +
> + case REG_TYPE_INDEX:
> + regno = X86_SIB_INDEX(sib);
> + if (X86_REX_X(insn->rex_prefix.value) == 1)
> + regno += 8;
> + break;
> +
> + case REG_TYPE_BASE:
> + regno = X86_SIB_BASE(sib);
> + if (X86_REX_B(insn->rex_prefix.value) == 1)
> + regno += 8;
> + break;
> +
> + default:
> + break;
> + }
> +
> + return regs_get_register(regs, regoff[regno]);
> +}
This (and the rest of the decoder) is IMO hideous. Is there any reason
that this belongs in the kernel and not in, say, a libmpx?
(Why on earth does Intel not expose this stuff in cr2 or an MSR or
something?)
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/26/2014 01:08 AM, Qiaowei Ren wrote:
This patch adds new fields about bound violation into siginfo
structure. si_lower and si_upper are respectively lower bound
and upper bound when bound violation is caused.
These fields will be set in #BR exception handler by decoding
the user instruction and constructing the faulting pointer.
A userspace application can get violation address, lower bound
and upper bound for bound violation from this new siginfo structure.
Signed-off-by: Qiaowei Ren qiaowei@intel.com
---
arch/x86/include/asm/mpx.h | 19 +++
arch/x86/kernel/mpx.c | 287
arch/x86/kernel/traps.c|6 +
include/uapi/asm-generic/siginfo.h |9 +-
kernel/signal.c|4 +
5 files changed, 324 insertions(+), 1 deletions(-)
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index 9652e9e..e099573 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -3,6 +3,7 @@
#include linux/types.h
#include asm/ptrace.h
+#include asm/insn.h
#ifdef CONFIG_X86_64
@@ -30,6 +31,22 @@
#endif
+struct mpx_insn {
+ struct insn_field rex_prefix; /* REX prefix */
+ struct insn_field modrm;
+ struct insn_field sib;
+ struct insn_field displacement;
+
+ unsigned char addr_bytes; /* effective address size */
+ unsigned char limit;
+ unsigned char x86_64;
+
+ const unsigned char *kaddr; /* kernel address of insn to analyze */
+ const unsigned char *next_byte;
+};
+
+#define MAX_MPX_INSN_SIZE15
+
typedef union {
struct {
unsigned long ignored:MPX_IGN_BITS;
@@ -40,5 +57,7 @@ typedef union {
} mpx_addr;
void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
+void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
+ struct xsave_struct *xsave_buf);
#endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
index 9e91178..983abf7 100644
--- a/arch/x86/kernel/mpx.c
+++ b/arch/x86/kernel/mpx.c
@@ -91,6 +91,269 @@ int mpx_release(struct task_struct *tsk)
return 0;
}
+typedef enum {REG_TYPE_RM, REG_TYPE_INDEX, REG_TYPE_BASE} reg_type_t;
+static unsigned long get_reg(struct mpx_insn *insn, struct pt_regs *regs,
+ reg_type_t type)
+{
+ int regno = 0;
+ unsigned char modrm = (unsigned char)insn-modrm.value;
+ unsigned char sib = (unsigned char)insn-sib.value;
+
+ static const int regoff[] = {
+ offsetof(struct pt_regs, ax),
+ offsetof(struct pt_regs, cx),
+ offsetof(struct pt_regs, dx),
+ offsetof(struct pt_regs, bx),
+ offsetof(struct pt_regs, sp),
+ offsetof(struct pt_regs, bp),
+ offsetof(struct pt_regs, si),
+ offsetof(struct pt_regs, di),
+#ifdef CONFIG_X86_64
+ offsetof(struct pt_regs, r8),
+ offsetof(struct pt_regs, r9),
+ offsetof(struct pt_regs, r10),
+ offsetof(struct pt_regs, r11),
+ offsetof(struct pt_regs, r12),
+ offsetof(struct pt_regs, r13),
+ offsetof(struct pt_regs, r14),
+ offsetof(struct pt_regs, r15),
+#endif
+ };
+
+ switch (type) {
+ case REG_TYPE_RM:
+ regno = X86_MODRM_RM(modrm);
+ if (X86_REX_B(insn-rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_INDEX:
+ regno = X86_SIB_INDEX(sib);
+ if (X86_REX_X(insn-rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_BASE:
+ regno = X86_SIB_BASE(sib);
+ if (X86_REX_B(insn-rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ default:
+ break;
+ }
+
+ return regs_get_register(regs, regoff[regno]);
+}
This (and the rest of the decoder) is IMO hideous. Is there any reason
that this belongs in the kernel and not in, say, a libmpx?
(Why on earth does Intel not expose this stuff in cr2 or an MSR or
something?)
--Andy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/28/2014 05:58 AM, Andy Lutomirski wrote: On 01/26/2014 01:08 AM, Qiaowei Ren wrote: (Why on earth does Intel not expose this stuff in cr2 or an MSR or something?) I guess it is due to some design reason. Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/27/2014 09:53 AM, H. Peter Anvin wrote: On 01/26/2014 05:34 PM, Ren Qiaowei wrote: I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit. info->si_lower = (void __user *)(unsigned long) (xsave_buf->bndregs.bndregs[2*bndregno]); info->si_upper = (void __user *)(unsigned long) (~xsave_buf->bndregs.bndregs[2*bndregno+1]); That is the way it is usually done, yes. Add a comment saying something like: /* Note: the upper 32 bits are ignored in 32-bit mode. */ It is worth watching out a bit here, though, because you could be running a 32-bit process on top of a 64-bit kernel. Ok. I will update it in next version. Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/26/2014 05:34 PM, Ren Qiaowei wrote: >> > I agree with you and we should suppress all the warnings as possible. If > I use (unsgined long) to cast like the following code, what do you think > about it? sizeof(long) will be 4 for 32-bit. > > info->si_lower = (void __user *)(unsigned long) > (xsave_buf->bndregs.bndregs[2*bndregno]); > info->si_upper = (void __user *)(unsigned long) > (~xsave_buf->bndregs.bndregs[2*bndregno+1]); > That is the way it is usually done, yes. Add a comment saying something like: /* Note: the upper 32 bits are ignored in 32-bit mode. */ It is worth watching out a bit here, though, because you could be running a 32-bit process on top of a 64-bit kernel. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/27/2014 05:38 AM, David Rientjes wrote: On Sun, 26 Jan 2014, Ren Qiaowei wrote: arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] and the documentation says you explicitly want to support this config. These types of warnings are usually indicative of real problems when you're storing upper and lower bits in 32-bit fields after casting them from 64-bit values. I'm also not sure if the added fields to the generic struct siginfo can be justified for this. According to MPX spec, for 32-bit case, the upper 32-bits of 64-bits bound register are ignored, and so casting to pointer from 64-bit values should be not produce any problems. Ok, so this is intended per the spec which nobody reading the code is going to know and people who report the compile warnings are going to continue to question it. How are you planning on suppressing the warnings? It will probably require either - separate 64-bit and 32-bit helper functions to do_mpx_bounds() to do appropriate casts before casting to a pointer, or - a macro defined as a no-op for 64-bit and as a cast to 32-bit value for 32-bit configs that will be used in do_mpx_bounds() and casted to the pointer. I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit. info->si_lower = (void __user *)(unsigned long) (xsave_buf->bndregs.bndregs[2*bndregno]); info->si_upper = (void __user *)(unsigned long) (~xsave_buf->bndregs.bndregs[2*bndregno+1]); Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On Sun, 26 Jan 2014, Ren Qiaowei wrote: > > arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: > > arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of > > different size [-Wint-to-pointer-cast] > > arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of > > different size [-Wint-to-pointer-cast] > > > > and the documentation says you explicitly want to support this config. > > > > These types of warnings are usually indicative of real problems when > > you're storing upper and lower bits in 32-bit fields after casting them > > from 64-bit values. > > > > I'm also not sure if the added fields to the generic struct siginfo can be > > justified for this. > > > According to MPX spec, for 32-bit case, the upper 32-bits of 64-bits bound > register are ignored, and so casting to pointer from 64-bit values should be > not produce any problems. > Ok, so this is intended per the spec which nobody reading the code is going to know and people who report the compile warnings are going to continue to question it. How are you planning on suppressing the warnings? It will probably require either - separate 64-bit and 32-bit helper functions to do_mpx_bounds() to do appropriate casts before casting to a pointer, or - a macro defined as a no-op for 64-bit and as a cast to 32-bit value for 32-bit configs that will be used in do_mpx_bounds() and casted to the pointer.
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On Sun, 26 Jan 2014, Ren Qiaowei wrote: arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] and the documentation says you explicitly want to support this config. These types of warnings are usually indicative of real problems when you're storing upper and lower bits in 32-bit fields after casting them from 64-bit values. I'm also not sure if the added fields to the generic struct siginfo can be justified for this. According to MPX spec, for 32-bit case, the upper 32-bits of 64-bits bound register are ignored, and so casting to pointer from 64-bit values should be not produce any problems. Ok, so this is intended per the spec which nobody reading the code is going to know and people who report the compile warnings are going to continue to question it. How are you planning on suppressing the warnings? It will probably require either - separate 64-bit and 32-bit helper functions to do_mpx_bounds() to do appropriate casts before casting to a pointer, or - a macro defined as a no-op for 64-bit and as a cast to 32-bit value for 32-bit configs that will be used in do_mpx_bounds() and casted to the pointer.
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/27/2014 05:38 AM, David Rientjes wrote: On Sun, 26 Jan 2014, Ren Qiaowei wrote: arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] and the documentation says you explicitly want to support this config. These types of warnings are usually indicative of real problems when you're storing upper and lower bits in 32-bit fields after casting them from 64-bit values. I'm also not sure if the added fields to the generic struct siginfo can be justified for this. According to MPX spec, for 32-bit case, the upper 32-bits of 64-bits bound register are ignored, and so casting to pointer from 64-bit values should be not produce any problems. Ok, so this is intended per the spec which nobody reading the code is going to know and people who report the compile warnings are going to continue to question it. How are you planning on suppressing the warnings? It will probably require either - separate 64-bit and 32-bit helper functions to do_mpx_bounds() to do appropriate casts before casting to a pointer, or - a macro defined as a no-op for 64-bit and as a cast to 32-bit value for 32-bit configs that will be used in do_mpx_bounds() and casted to the pointer. I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit. info-si_lower = (void __user *)(unsigned long) (xsave_buf-bndregs.bndregs[2*bndregno]); info-si_upper = (void __user *)(unsigned long) (~xsave_buf-bndregs.bndregs[2*bndregno+1]); Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/26/2014 05:34 PM, Ren Qiaowei wrote: I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit. info-si_lower = (void __user *)(unsigned long) (xsave_buf-bndregs.bndregs[2*bndregno]); info-si_upper = (void __user *)(unsigned long) (~xsave_buf-bndregs.bndregs[2*bndregno+1]); That is the way it is usually done, yes. Add a comment saying something like: /* Note: the upper 32 bits are ignored in 32-bit mode. */ It is worth watching out a bit here, though, because you could be running a 32-bit process on top of a 64-bit kernel. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/27/2014 09:53 AM, H. Peter Anvin wrote: On 01/26/2014 05:34 PM, Ren Qiaowei wrote: I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit. info-si_lower = (void __user *)(unsigned long) (xsave_buf-bndregs.bndregs[2*bndregno]); info-si_upper = (void __user *)(unsigned long) (~xsave_buf-bndregs.bndregs[2*bndregno+1]); That is the way it is usually done, yes. Add a comment saying something like: /* Note: the upper 32 bits are ignored in 32-bit mode. */ It is worth watching out a bit here, though, because you could be running a 32-bit process on top of a 64-bit kernel. Ok. I will update it in next version. Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/26/2014 12:22 PM, David Rientjes wrote: On Sun, 26 Jan 2014, Qiaowei Ren wrote: This patch adds new fields about bound violation into siginfo structure. si_lower and si_upper are respectively lower bound and upper bound when bound violation is caused. These fields will be set in #BR exception handler by decoding the user instruction and constructing the faulting pointer. A userspace application can get violation address, lower bound and upper bound for bound violation from this new siginfo structure. Signed-off-by: Qiaowei Ren Same 32-bit warnings I reported for v2: arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] and the documentation says you explicitly want to support this config. These types of warnings are usually indicative of real problems when you're storing upper and lower bits in 32-bit fields after casting them from 64-bit values. I'm also not sure if the added fields to the generic struct siginfo can be justified for this. According to MPX spec, for 32-bit case, the upper 32-bits of 64-bits bound register are ignored, and so casting to pointer from 64-bit values should be not produce any problems. Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On Sun, 26 Jan 2014, Qiaowei Ren wrote: > This patch adds new fields about bound violation into siginfo > structure. si_lower and si_upper are respectively lower bound > and upper bound when bound violation is caused. > > These fields will be set in #BR exception handler by decoding > the user instruction and constructing the faulting pointer. > A userspace application can get violation address, lower bound > and upper bound for bound violation from this new siginfo structure. > > Signed-off-by: Qiaowei Ren Same 32-bit warnings I reported for v2: arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] and the documentation says you explicitly want to support this config. These types of warnings are usually indicative of real problems when you're storing upper and lower bits in 32-bit fields after casting them from 64-bit values. I'm also not sure if the added fields to the generic struct siginfo can be justified for this.
[PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
This patch adds new fields about bound violation into siginfo
structure. si_lower and si_upper are respectively lower bound
and upper bound when bound violation is caused.
These fields will be set in #BR exception handler by decoding
the user instruction and constructing the faulting pointer.
A userspace application can get violation address, lower bound
and upper bound for bound violation from this new siginfo structure.
Signed-off-by: Qiaowei Ren
---
arch/x86/include/asm/mpx.h | 19 +++
arch/x86/kernel/mpx.c | 287
arch/x86/kernel/traps.c|6 +
include/uapi/asm-generic/siginfo.h |9 +-
kernel/signal.c|4 +
5 files changed, 324 insertions(+), 1 deletions(-)
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index 9652e9e..e099573 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -3,6 +3,7 @@
#include
#include
+#include
#ifdef CONFIG_X86_64
@@ -30,6 +31,22 @@
#endif
+struct mpx_insn {
+ struct insn_field rex_prefix; /* REX prefix */
+ struct insn_field modrm;
+ struct insn_field sib;
+ struct insn_field displacement;
+
+ unsigned char addr_bytes; /* effective address size */
+ unsigned char limit;
+ unsigned char x86_64;
+
+ const unsigned char *kaddr; /* kernel address of insn to analyze */
+ const unsigned char *next_byte;
+};
+
+#define MAX_MPX_INSN_SIZE 15
+
typedef union {
struct {
unsigned long ignored:MPX_IGN_BITS;
@@ -40,5 +57,7 @@ typedef union {
} mpx_addr;
void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
+void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
+ struct xsave_struct *xsave_buf);
#endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
index 9e91178..983abf7 100644
--- a/arch/x86/kernel/mpx.c
+++ b/arch/x86/kernel/mpx.c
@@ -91,6 +91,269 @@ int mpx_release(struct task_struct *tsk)
return 0;
}
+typedef enum {REG_TYPE_RM, REG_TYPE_INDEX, REG_TYPE_BASE} reg_type_t;
+static unsigned long get_reg(struct mpx_insn *insn, struct pt_regs *regs,
+reg_type_t type)
+{
+ int regno = 0;
+ unsigned char modrm = (unsigned char)insn->modrm.value;
+ unsigned char sib = (unsigned char)insn->sib.value;
+
+ static const int regoff[] = {
+ offsetof(struct pt_regs, ax),
+ offsetof(struct pt_regs, cx),
+ offsetof(struct pt_regs, dx),
+ offsetof(struct pt_regs, bx),
+ offsetof(struct pt_regs, sp),
+ offsetof(struct pt_regs, bp),
+ offsetof(struct pt_regs, si),
+ offsetof(struct pt_regs, di),
+#ifdef CONFIG_X86_64
+ offsetof(struct pt_regs, r8),
+ offsetof(struct pt_regs, r9),
+ offsetof(struct pt_regs, r10),
+ offsetof(struct pt_regs, r11),
+ offsetof(struct pt_regs, r12),
+ offsetof(struct pt_regs, r13),
+ offsetof(struct pt_regs, r14),
+ offsetof(struct pt_regs, r15),
+#endif
+ };
+
+ switch (type) {
+ case REG_TYPE_RM:
+ regno = X86_MODRM_RM(modrm);
+ if (X86_REX_B(insn->rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_INDEX:
+ regno = X86_SIB_INDEX(sib);
+ if (X86_REX_X(insn->rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_BASE:
+ regno = X86_SIB_BASE(sib);
+ if (X86_REX_B(insn->rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ default:
+ break;
+ }
+
+ return regs_get_register(regs, regoff[regno]);
+}
+
+/*
+ * return the address being referenced be instruction
+ * for rm=3 returning the content of the rm reg
+ * for rm!=3 calculates the address using SIB and Disp
+ */
+static unsigned long get_addr_ref(struct mpx_insn *insn, struct pt_regs *regs)
+{
+ unsigned long addr;
+ unsigned long base;
+ unsigned long indx;
+ unsigned char modrm = (unsigned char)insn->modrm.value;
+ unsigned char sib = (unsigned char)insn->sib.value;
+
+ if (X86_MODRM_MOD(modrm) == 3) {
+ addr = get_reg(insn, regs, REG_TYPE_RM);
+ } else {
+ if (insn->sib.nbytes) {
+ base = get_reg(insn, regs, REG_TYPE_BASE);
+ indx = get_reg(insn, regs, REG_TYPE_INDEX);
+ addr = base + indx * (1 << X86_SIB_SCALE(sib));
+ } else {
+ addr = get_reg(insn, regs, REG_TYPE_RM);
+ }
+ addr += insn->displacement.value;
+ }
+
+ return addr;
+}
+
+/* Verify next
[PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
This patch adds new fields about bound violation into siginfo
structure. si_lower and si_upper are respectively lower bound
and upper bound when bound violation is caused.
These fields will be set in #BR exception handler by decoding
the user instruction and constructing the faulting pointer.
A userspace application can get violation address, lower bound
and upper bound for bound violation from this new siginfo structure.
Signed-off-by: Qiaowei Ren qiaowei@intel.com
---
arch/x86/include/asm/mpx.h | 19 +++
arch/x86/kernel/mpx.c | 287
arch/x86/kernel/traps.c|6 +
include/uapi/asm-generic/siginfo.h |9 +-
kernel/signal.c|4 +
5 files changed, 324 insertions(+), 1 deletions(-)
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index 9652e9e..e099573 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -3,6 +3,7 @@
#include linux/types.h
#include asm/ptrace.h
+#include asm/insn.h
#ifdef CONFIG_X86_64
@@ -30,6 +31,22 @@
#endif
+struct mpx_insn {
+ struct insn_field rex_prefix; /* REX prefix */
+ struct insn_field modrm;
+ struct insn_field sib;
+ struct insn_field displacement;
+
+ unsigned char addr_bytes; /* effective address size */
+ unsigned char limit;
+ unsigned char x86_64;
+
+ const unsigned char *kaddr; /* kernel address of insn to analyze */
+ const unsigned char *next_byte;
+};
+
+#define MAX_MPX_INSN_SIZE 15
+
typedef union {
struct {
unsigned long ignored:MPX_IGN_BITS;
@@ -40,5 +57,7 @@ typedef union {
} mpx_addr;
void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
+void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
+ struct xsave_struct *xsave_buf);
#endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
index 9e91178..983abf7 100644
--- a/arch/x86/kernel/mpx.c
+++ b/arch/x86/kernel/mpx.c
@@ -91,6 +91,269 @@ int mpx_release(struct task_struct *tsk)
return 0;
}
+typedef enum {REG_TYPE_RM, REG_TYPE_INDEX, REG_TYPE_BASE} reg_type_t;
+static unsigned long get_reg(struct mpx_insn *insn, struct pt_regs *regs,
+reg_type_t type)
+{
+ int regno = 0;
+ unsigned char modrm = (unsigned char)insn-modrm.value;
+ unsigned char sib = (unsigned char)insn-sib.value;
+
+ static const int regoff[] = {
+ offsetof(struct pt_regs, ax),
+ offsetof(struct pt_regs, cx),
+ offsetof(struct pt_regs, dx),
+ offsetof(struct pt_regs, bx),
+ offsetof(struct pt_regs, sp),
+ offsetof(struct pt_regs, bp),
+ offsetof(struct pt_regs, si),
+ offsetof(struct pt_regs, di),
+#ifdef CONFIG_X86_64
+ offsetof(struct pt_regs, r8),
+ offsetof(struct pt_regs, r9),
+ offsetof(struct pt_regs, r10),
+ offsetof(struct pt_regs, r11),
+ offsetof(struct pt_regs, r12),
+ offsetof(struct pt_regs, r13),
+ offsetof(struct pt_regs, r14),
+ offsetof(struct pt_regs, r15),
+#endif
+ };
+
+ switch (type) {
+ case REG_TYPE_RM:
+ regno = X86_MODRM_RM(modrm);
+ if (X86_REX_B(insn-rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_INDEX:
+ regno = X86_SIB_INDEX(sib);
+ if (X86_REX_X(insn-rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_BASE:
+ regno = X86_SIB_BASE(sib);
+ if (X86_REX_B(insn-rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ default:
+ break;
+ }
+
+ return regs_get_register(regs, regoff[regno]);
+}
+
+/*
+ * return the address being referenced be instruction
+ * for rm=3 returning the content of the rm reg
+ * for rm!=3 calculates the address using SIB and Disp
+ */
+static unsigned long get_addr_ref(struct mpx_insn *insn, struct pt_regs *regs)
+{
+ unsigned long addr;
+ unsigned long base;
+ unsigned long indx;
+ unsigned char modrm = (unsigned char)insn-modrm.value;
+ unsigned char sib = (unsigned char)insn-sib.value;
+
+ if (X86_MODRM_MOD(modrm) == 3) {
+ addr = get_reg(insn, regs, REG_TYPE_RM);
+ } else {
+ if (insn-sib.nbytes) {
+ base = get_reg(insn, regs, REG_TYPE_BASE);
+ indx = get_reg(insn, regs, REG_TYPE_INDEX);
+ addr = base + indx * (1 X86_SIB_SCALE(sib));
+ } else {
+ addr = get_reg(insn, regs, REG_TYPE_RM);
+ }
+ addr += insn-displacement.value;
+
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On Sun, 26 Jan 2014, Qiaowei Ren wrote: This patch adds new fields about bound violation into siginfo structure. si_lower and si_upper are respectively lower bound and upper bound when bound violation is caused. These fields will be set in #BR exception handler by decoding the user instruction and constructing the faulting pointer. A userspace application can get violation address, lower bound and upper bound for bound violation from this new siginfo structure. Signed-off-by: Qiaowei Ren qiaowei@intel.com Same 32-bit warnings I reported for v2: arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] and the documentation says you explicitly want to support this config. These types of warnings are usually indicative of real problems when you're storing upper and lower bits in 32-bit fields after casting them from 64-bit values. I'm also not sure if the added fields to the generic struct siginfo can be justified for this.
Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to include bound violation information
On 01/26/2014 12:22 PM, David Rientjes wrote: On Sun, 26 Jan 2014, Qiaowei Ren wrote: This patch adds new fields about bound violation into siginfo structure. si_lower and si_upper are respectively lower bound and upper bound when bound violation is caused. These fields will be set in #BR exception handler by decoding the user instruction and constructing the faulting pointer. A userspace application can get violation address, lower bound and upper bound for bound violation from this new siginfo structure. Signed-off-by: Qiaowei Ren qiaowei@intel.com Same 32-bit warnings I reported for v2: arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’: arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] and the documentation says you explicitly want to support this config. These types of warnings are usually indicative of real problems when you're storing upper and lower bits in 32-bit fields after casting them from 64-bit values. I'm also not sure if the added fields to the generic struct siginfo can be justified for this. According to MPX spec, for 32-bit case, the upper 32-bits of 64-bits bound register are ignored, and so casting to pointer from 64-bit values should be not produce any problems. Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
