Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1307370 > unrolled thread
| Started by | Borislav Petkov <bp@alien8.de> |
|---|---|
| First post | 2016-01-12 13:50 +0100 |
| Last post | 2016-01-12 15:50 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v15 21/25] x86/asm: Create stack frames in rwsem functions Borislav Petkov <bp@alien8.de> - 2016-01-12 13:50 +0100
Re: [PATCH v15 21/25] x86/asm: Create stack frames in rwsem functions Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-12 15:40 +0100
Re: [PATCH v15 21/25] x86/asm: Create stack frames in rwsem functions Borislav Petkov <bp@alien8.de> - 2016-01-12 15:50 +0100
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-01-12 13:50 +0100 |
| Subject | Re: [PATCH v15 21/25] x86/asm: Create stack frames in rwsem functions |
| Message-ID | <qQ6yB-WB-5@gated-at.bofh.it> |
On Fri, Dec 18, 2015 at 06:39:35AM -0600, Josh Poimboeuf wrote:
> rwsem.S has several callable non-leaf functions which don't honor
> CONFIG_FRAME_POINTER, which can result in bad stack traces.
>
> Create stack frames for them when CONFIG_FRAME_POINTER is enabled.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
> arch/x86/lib/rwsem.S | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/lib/rwsem.S b/arch/x86/lib/rwsem.S
> index 40027db..be110ef 100644
> --- a/arch/x86/lib/rwsem.S
> +++ b/arch/x86/lib/rwsem.S
> @@ -15,6 +15,7 @@
>
> #include <linux/linkage.h>
> #include <asm/alternative-asm.h>
> +#include <asm/frame.h>
>
> #define __ASM_HALF_REG(reg) __ASM_SEL(reg, e##reg)
> #define __ASM_HALF_SIZE(inst) __ASM_SEL(inst##w, inst##l)
> @@ -84,24 +85,29 @@
>
> /* Fix up special calling conventions */
> ENTRY(call_rwsem_down_read_failed)
> + FRAME_BEGIN
Remind me again, please, why aren't we hiding those
FRAME_BEGIN/FRAME_END macros in the ENTRY/ENDPROC ones?
> save_common_regs
> __ASM_SIZE(push,) %__ASM_REG(dx)
> movq %rax,%rdi
> call rwsem_down_read_failed
> __ASM_SIZE(pop,) %__ASM_REG(dx)
> restore_common_regs
> + FRAME_END
> ret
> ENDPROC(call_rwsem_down_read_failed)
Thanks!
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-01-12 15:40 +0100 |
| Message-ID | <qQ8h4-2fU-25@gated-at.bofh.it> |
| In reply to | #1307370 |
On Tue, Jan 12, 2016 at 01:41:58PM +0100, Borislav Petkov wrote: > On Fri, Dec 18, 2015 at 06:39:35AM -0600, Josh Poimboeuf wrote: > > rwsem.S has several callable non-leaf functions which don't honor > > CONFIG_FRAME_POINTER, which can result in bad stack traces. > > > > Create stack frames for them when CONFIG_FRAME_POINTER is enabled. > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > --- > > arch/x86/lib/rwsem.S | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/lib/rwsem.S b/arch/x86/lib/rwsem.S > > index 40027db..be110ef 100644 > > --- a/arch/x86/lib/rwsem.S > > +++ b/arch/x86/lib/rwsem.S > > @@ -15,6 +15,7 @@ > > > > #include <linux/linkage.h> > > #include <asm/alternative-asm.h> > > +#include <asm/frame.h> > > > > #define __ASM_HALF_REG(reg) __ASM_SEL(reg, e##reg) > > #define __ASM_HALF_SIZE(inst) __ASM_SEL(inst##w, inst##l) > > @@ -84,24 +85,29 @@ > > > > /* Fix up special calling conventions */ > > ENTRY(call_rwsem_down_read_failed) > > + FRAME_BEGIN > > Remind me again, please, why aren't we hiding those > FRAME_BEGIN/FRAME_END macros in the ENTRY/ENDPROC ones? Ingo made a similar suggestion a while back: https://lkml.kernel.org/r/20150717194307.GA26757@gmail.com But the frame stuff can't be folded into ENTRY/ENDPROC because we don't need to create a stack frame for *all* functions, but rather only for non-leaf functions. So then we considered something like: FUNCTION_ENTRY(func) FUNCTION_RETURN(func) for non-leaf functions, and: LEAF_FUNCTION_ENTRY(func) LEAF_FUNCTION_RETURN(func) for leaf functions. But that was too inflexible for the case where a function ends with a jump instead of a return. > > save_common_regs > > __ASM_SIZE(push,) %__ASM_REG(dx) > > movq %rax,%rdi > > call rwsem_down_read_failed > > __ASM_SIZE(pop,) %__ASM_REG(dx) > > restore_common_regs > > + FRAME_END > > ret > > ENDPROC(call_rwsem_down_read_failed) -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-01-12 15:50 +0100 |
| Message-ID | <qQ8qK-2kQ-11@gated-at.bofh.it> |
| In reply to | #1307487 |
On Tue, Jan 12, 2016 at 08:36:48AM -0600, Josh Poimboeuf wrote:
> Ingo made a similar suggestion a while back:
>
> https://lkml.kernel.org/r/20150717194307.GA26757@gmail.com
>
> But the frame stuff can't be folded into ENTRY/ENDPROC because we don't
> need to create a stack frame for *all* functions, but rather only for
> non-leaf functions.
>
> So then we considered something like:
>
> FUNCTION_ENTRY(func)
> FUNCTION_RETURN(func)
>
> for non-leaf functions, and:
>
> LEAF_FUNCTION_ENTRY(func)
> LEAF_FUNCTION_RETURN(func)
>
> for leaf functions.
>
> But that was too inflexible for the case where a function ends with a
> jump instead of a return.
Aah, there it is, thanks!
It actually does ring a bell. Ok. I'm guessing the expectation is that
we're not going to sprinkle those excessively but use them only in asm
code. Which should be relatively seldom, especially since we're moving
more and more stuff to C.
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web