2019-05-20 23:21:36

by Thomas Garnier

Subject: [PATCH v7 03/12] x86: Add macro to get symbol address for PIE support

From: Thomas Garnier <[email protected]>

Add a new _ASM_MOVABS macro to fetch a symbol address. It will be used
to replace "_ASM_MOV $<symbol>, %dst" code construct that are not
compatible with PIE.

Signed-off-by: Thomas Garnier <[email protected]>
---
arch/x86/include/asm/asm.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 3ff577c0b102..3a686057e882 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -30,6 +30,7 @@
#define _ASM_ALIGN __ASM_SEL(.balign 4, .balign 8)

#define _ASM_MOV __ASM_SIZE(mov)
+#define _ASM_MOVABS __ASM_SEL(movl, movabsq)
#define _ASM_INC __ASM_SIZE(inc)
#define _ASM_DEC __ASM_SIZE(dec)
#define _ASM_ADD __ASM_SIZE(add)
--
2.21.0.1020.gf2820cf01a-goog

2019-05-21 03:16:15

by H. Peter Anvin

Subject: Re: [PATCH v7 03/12] x86: Add macro to get symbol address for PIE support

On May 20, 2019 4:19:28 PM PDT, Thomas Garnier <[email protected]> wrote:
>From: Thomas Garnier <[email protected]>
>
>Add a new _ASM_MOVABS macro to fetch a symbol address. It will be used
>to replace "_ASM_MOV $<symbol>, %dst" code construct that are not
>compatible with PIE.
>
>Signed-off-by: Thomas Garnier <[email protected]>
>---
> arch/x86/include/asm/asm.h | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
>index 3ff577c0b102..3a686057e882 100644
>--- a/arch/x86/include/asm/asm.h
>+++ b/arch/x86/include/asm/asm.h
>@@ -30,6 +30,7 @@
> #define _ASM_ALIGN __ASM_SEL(.balign 4, .balign 8)
>
> #define _ASM_MOV __ASM_SIZE(mov)
>+#define _ASM_MOVABS __ASM_SEL(movl, movabsq)
> #define _ASM_INC __ASM_SIZE(inc)
> #define _ASM_DEC __ASM_SIZE(dec)
> #define _ASM_ADD __ASM_SIZE(add)

This is just about *always* wrong on x86-86. We should be using leaq sym(%rip),%reg. If it isn't reachable by leaq, then it is a non-PIE symbol like percpu. You do have to keep those distinct!
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2019-05-22 17:38:00

by Thomas Garnier

Subject: Re: [PATCH v7 03/12] x86: Add macro to get symbol address for PIE support

On Mon, May 20, 2019 at 8:13 PM <[email protected]> wrote:
>
> On May 20, 2019 4:19:28 PM PDT, Thomas Garnier <[email protected]> wrote:
> >From: Thomas Garnier <[email protected]>
> >
> >Add a new _ASM_MOVABS macro to fetch a symbol address. It will be used
> >to replace "_ASM_MOV $<symbol>, %dst" code construct that are not
> >compatible with PIE.
> >
> >Signed-off-by: Thomas Garnier <[email protected]>
> >---
> > arch/x86/include/asm/asm.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> >index 3ff577c0b102..3a686057e882 100644
> >--- a/arch/x86/include/asm/asm.h
> >+++ b/arch/x86/include/asm/asm.h
> >@@ -30,6 +30,7 @@
> > #define _ASM_ALIGN __ASM_SEL(.balign 4, .balign 8)
> >
> > #define _ASM_MOV __ASM_SIZE(mov)
> >+#define _ASM_MOVABS __ASM_SEL(movl, movabsq)
> > #define _ASM_INC __ASM_SIZE(inc)
> > #define _ASM_DEC __ASM_SIZE(dec)
> > #define _ASM_ADD __ASM_SIZE(add)
>
> This is just about *always* wrong on x86-86. We should be using leaq sym(%rip),%reg. If it isn't reachable by leaq, then it is a non-PIE symbol like percpu. You do have to keep those distinct!

Yes, I agree. This patch is just having a shortcut when it is a
non-PIE symbol. The other patches try to separate the use cases where
a leaq sym(%rip) would work versus the need for a movabsq. There are
multiple cases where relative references are not possible because the
memory layout is different (hibernation, early boot or others).

> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.