RE: [PATCH 5/5] x86, mpx: extend siginfo structure to include bound violation information
> -Original Message- > From: Borislav Petkov [mailto:b...@alien8.de] > Sent: Monday, January 13, 2014 6:43 PM > To: Ren, Qiaowei > Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x...@kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH 5/5] x86, mpx: extend siginfo structure to include bound > violation information > > On Mon, Jan 13, 2014 at 04:22:27PM +0800, Ren Qiaowei wrote: > > >I tried to use generic structure and macro, but many members of > > >generic struct insn are not used for MPX, > > I think that's ok - there are a lot of examples in the kernel where only a > subset > of the struct members are used by a particular functionality. > > > Because only struct insn_field and several macros may be replaced with > > generic version, I guess it maybe be confused easily to include > > generic insn header. What do you think about it? > > Yes, I think the idea is to use and, if needed, extend the generic > functionality > instead of growing our own here and there for obvious benefits. > Yes, I once tried to use and extend the generic functionality to implement the decoding, but I noticed it didn't work out well for this specific MPX case. Anyway, I will try to use generic version more. Thanks, Qiaowei
RE: [PATCH 5/5] x86, mpx: extend siginfo structure to include bound violation information
-Original Message- From: Borislav Petkov [mailto:b...@alien8.de] Sent: Monday, January 13, 2014 6:43 PM To: Ren, Qiaowei Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; x...@kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH 5/5] x86, mpx: extend siginfo structure to include bound violation information On Mon, Jan 13, 2014 at 04:22:27PM +0800, Ren Qiaowei wrote: I tried to use generic structure and macro, but many members of generic struct insn are not used for MPX, I think that's ok - there are a lot of examples in the kernel where only a subset of the struct members are used by a particular functionality. Because only struct insn_field and several macros may be replaced with generic version, I guess it maybe be confused easily to include generic insn header. What do you think about it? Yes, I think the idea is to use and, if needed, extend the generic functionality instead of growing our own here and there for obvious benefits. Yes, I once tried to use and extend the generic functionality to implement the decoding, but I noticed it didn't work out well for this specific MPX case. Anyway, I will try to use generic version more. Thanks, Qiaowei
Re: [PATCH 5/5] x86, mpx: extend siginfo structure to include bound violation information
On Mon, Jan 13, 2014 at 04:22:27PM +0800, Ren Qiaowei wrote: > >I tried to use generic structure and macro, but many members of generic > >struct insn are not used for MPX, I think that's ok - there are a lot of examples in the kernel where only a subset of the struct members are used by a particular functionality. > Because only struct insn_field and several macros may be replaced > with generic version, I guess it maybe be confused easily to include > generic insn header. What do you think about it? Yes, I think the idea is to use and, if needed, extend the generic functionality instead of growing our own here and there for obvious benefits. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On 01/13/2014 11:09 AM, Ren Qiaowei wrote: On 01/13/2014 01:03 AM, Borislav Petkov wrote: On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. I suspected as much. But, we still probably should use the generic struct insn, insn_field, etc and act on them in mpx.c instead of defining our own mpx_insn, mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less copied from insn.h, right? I tried to use generic structure and macro, but many members of generic struct insn are not used for MPX, and except this I have to add one member into this structure. So I define mpx specific struct insn. And so I guess only struct insn_field and several macros like X86_XXX may use generic version. Anyway, I will try to use their generic version in next version for this patchset. Because only struct insn_field and several macros may be replaced with generic version, I guess it maybe be confused easily to include generic insn header. What do you think about it? 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On 01/13/2014 11:09 AM, Ren Qiaowei wrote: On 01/13/2014 01:03 AM, Borislav Petkov wrote: On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. I suspected as much. But, we still probably should use the generic struct insn, insn_field, etc and act on them in mpx.c instead of defining our own mpx_insn, mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less copied from insn.h, right? I tried to use generic structure and macro, but many members of generic struct insn are not used for MPX, and except this I have to add one member into this structure. So I define mpx specific struct insn. And so I guess only struct insn_field and several macros like X86_XXX may use generic version. Anyway, I will try to use their generic version in next version for this patchset. Because only struct insn_field and several macros may be replaced with generic version, I guess it maybe be confused easily to include generic insn header. What do you think about it? 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On Mon, Jan 13, 2014 at 04:22:27PM +0800, Ren Qiaowei wrote: I tried to use generic structure and macro, but many members of generic struct insn are not used for MPX, I think that's ok - there are a lot of examples in the kernel where only a subset of the struct members are used by a particular functionality. Because only struct insn_field and several macros may be replaced with generic version, I guess it maybe be confused easily to include generic insn header. What do you think about it? Yes, I think the idea is to use and, if needed, extend the generic functionality instead of growing our own here and there for obvious benefits. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On 01/13/2014 01:03 AM, Borislav Petkov wrote: On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. I suspected as much. But, we still probably should use the generic struct insn, insn_field, etc and act on them in mpx.c instead of defining our own mpx_insn, mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less copied from insn.h, right? I tried to use generic structure and macro, but many members of generic struct insn are not used for MPX, and except this I have to add one member into this structure. So I define mpx specific struct insn. And so I guess only struct insn_field and several macros like X86_XXX may use generic version. Anyway, I will try to use their generic version in next version for this patchset. 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On 01/12/2014 09:03 AM, Borislav Petkov wrote: > On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: >> I saw a previous version of the code that did that, and it really >> didn't work out well -- it ended up being more complex and slower. > > I suspected as much. > > But, we still probably should use the generic struct insn, insn_field, > etc and act on them in mpx.c instead of defining our own mpx_insn, > mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less > copied from insn.h, right? > That may be. -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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: > I saw a previous version of the code that did that, and it really > didn't work out well -- it ended up being more complex and slower. I suspected as much. But, we still probably should use the generic struct insn, insn_field, etc and act on them in mpx.c instead of defining our own mpx_insn, mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less copied from insn.h, right? -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
> > This thing looks like a partial duplication of functionality which we > already have - inat.*/insn.*, etc. > > It would be cleaner to integrate the mpx pieces into the existing x86 > insn analysis code and use it instead of growing your own, IMHO. > I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. -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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On Sun, Jan 12, 2014 at 05:20:03PM +0800, 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 | 39 + > arch/x86/kernel/mpx.c | 289 > This thing looks like a partial duplication of functionality which we already have - inat.*/insn.*, etc. It would be cleaner to integrate the mpx pieces into the existing x86 insn analysis code and use it instead of growing your own, IMHO. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On Sun, Jan 12, 2014 at 05:20:03PM +0800, 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 | 39 + arch/x86/kernel/mpx.c | 289 This thing looks like a partial duplication of functionality which we already have - inat.*/insn.*, etc. It would be cleaner to integrate the mpx pieces into the existing x86 insn analysis code and use it instead of growing your own, IMHO. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
This thing looks like a partial duplication of functionality which we already have - inat.*/insn.*, etc. It would be cleaner to integrate the mpx pieces into the existing x86 insn analysis code and use it instead of growing your own, IMHO. I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. -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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. I suspected as much. But, we still probably should use the generic struct insn, insn_field, etc and act on them in mpx.c instead of defining our own mpx_insn, mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less copied from insn.h, right? -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On 01/12/2014 09:03 AM, Borislav Petkov wrote: On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. I suspected as much. But, we still probably should use the generic struct insn, insn_field, etc and act on them in mpx.c instead of defining our own mpx_insn, mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less copied from insn.h, right? That may be. -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 5/5] x86, mpx: extend siginfo structure to include bound violation information
On 01/13/2014 01:03 AM, Borislav Petkov wrote: On Sun, Jan 12, 2014 at 08:49:21AM -0800, H. Peter Anvin wrote: I saw a previous version of the code that did that, and it really didn't work out well -- it ended up being more complex and slower. I suspected as much. But, we still probably should use the generic struct insn, insn_field, etc and act on them in mpx.c instead of defining our own mpx_insn, mpx_insn_field, X86_MODRM_MOD, etc in the header which are more or less copied from insn.h, right? I tried to use generic structure and macro, but many members of generic struct insn are not used for MPX, and except this I have to add one member into this structure. So I define mpx specific struct insn. And so I guess only struct insn_field and several macros like X86_XXX may use generic version. Anyway, I will try to use their generic version in next version for this patchset. 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/
