2021-10-14 13:31:58

by Xiongwei Song

Subject: [PATCH RFC] x86/elf: Fix incorrect STACK_RND_MASK for x86_64

From: Xiongwei Song <[email protected]>

According to the comment for stack randomization of x86, the range of
randomization is 1GB, which occupies 30 valid bits in binary. And in
x86_64, PAGE_SHIFT occupies 12 bits, then the STACK_RND_MASK should
occupy 18 bits for 1GB.

The current range of randomization is :
0x3fffff << PAGE_SHIFT
, which occupies 34 bits.

This patch changed 0x3fffff to 0x3ffff, which makes the range of stack
randomization is real 1GB.

Before this patch(bit0 ~ bit33 randomized):
root@qemux86-64:~# for i in {1..20};do cat /proc/self/maps | grep stack;done
7ffddf971000-7ffddf992000 rw-p 00000000 00:00 0 [stack]
7ffc4e0d4000-7ffc4e0f5000 rw-p 00000000 00:00 0 [stack]
7fff5898e000-7fff589af000 rw-p 00000000 00:00 0 [stack]
7ffcf910b000-7ffcf912c000 rw-p 00000000 00:00 0 [stack]
7ffce829d000-7ffce82be000 rw-p 00000000 00:00 0 [stack]
7ffec72d9000-7ffec72fa000 rw-p 00000000 00:00 0 [stack]
7ffd6e256000-7ffd6e277000 rw-p 00000000 00:00 0 [stack]
7ffe14120000-7ffe14141000 rw-p 00000000 00:00 0 [stack]
7ffe21549000-7ffe2156a000 rw-p 00000000 00:00 0 [stack]
7ffdc9d33000-7ffdc9d54000 rw-p 00000000 00:00 0 [stack]
7ffe1ced4000-7ffe1cef5000 rw-p 00000000 00:00 0 [stack]
7ffcb0440000-7ffcb0461000 rw-p 00000000 00:00 0 [stack]
7ffc84515000-7ffc84536000 rw-p 00000000 00:00 0 [stack]
7ffe557dd000-7ffe557fe000 rw-p 00000000 00:00 0 [stack]
7ffdcb7e4000-7ffdcb805000 rw-p 00000000 00:00 0 [stack]
7ffc6f989000-7ffc6f9aa000 rw-p 00000000 00:00 0 [stack]
7ffd9322a000-7ffd9324b000 rw-p 00000000 00:00 0 [stack]
7ffea8e34000-7ffea8e55000 rw-p 00000000 00:00 0 [stack]
7ffc594ff000-7ffc59520000 rw-p 00000000 00:00 0 [stack]
7ffe8db65000-7ffe8db86000 rw-p 00000000 00:00 0 [stack]

After this patch(bit0 ~ bit29 randomized):
root@qemux86-64:~# for i in {1..20};do cat /proc/self/maps | grep stack;done
7fffd0ed2000-7fffd0ef3000 rw-p 00000000 00:00 0 [stack]
7fffdf555000-7fffdf576000 rw-p 00000000 00:00 0 [stack]
7ffffec0e000-7ffffec2f000 rw-p 00000000 00:00 0 [stack]
7fffcfa0f000-7fffcfa30000 rw-p 00000000 00:00 0 [stack]
7fffff75d000-7fffff77e000 rw-p 00000000 00:00 0 [stack]
7fffd1d11000-7fffd1d32000 rw-p 00000000 00:00 0 [stack]
7fffc7850000-7fffc7871000 rw-p 00000000 00:00 0 [stack]
7fffc956e000-7fffc958f000 rw-p 00000000 00:00 0 [stack]
7fffdf7be000-7fffdf7df000 rw-p 00000000 00:00 0 [stack]
7fffeffdf000-7ffff0000000 rw-p 00000000 00:00 0 [stack]
7fffd7d7f000-7fffd7da0000 rw-p 00000000 00:00 0 [stack]
7fffe3d23000-7fffe3d44000 rw-p 00000000 00:00 0 [stack]
7fffde4b7000-7fffde4d8000 rw-p 00000000 00:00 0 [stack]
7fffd477a000-7fffd479b000 rw-p 00000000 00:00 0 [stack]
7fffc0e95000-7fffc0eb6000 rw-p 00000000 00:00 0 [stack]
7fffdbdda000-7fffdbdfb000 rw-p 00000000 00:00 0 [stack]
7fffec737000-7fffec758000 rw-p 00000000 00:00 0 [stack]
7fffed163000-7fffed184000 rw-p 00000000 00:00 0 [stack]
7ffff9a9c000-7ffff9abd000 rw-p 00000000 00:00 0 [stack]
7fffff945000-7fffff966000 rw-p 00000000 00:00 0 [stack]

Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Gabriel Krisman Bertazi <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: "Chang S. Bae" <[email protected]>
Cc: Al Viro <[email protected]>
Cc: [email protected]
Signed-off-by: Xiongwei Song <[email protected]>
---
arch/x86/include/asm/elf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 29fea180a665..0b7a5510ebd5 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -343,7 +343,7 @@ extern unsigned long get_sigframe_size(void);
#else /* CONFIG_X86_32 */

/* 1GB for 64bit, 8MB for 32bit */
-#define __STACK_RND_MASK(is32bit) ((is32bit) ? 0x7ff : 0x3fffff)
+#define __STACK_RND_MASK(is32bit) ((is32bit) ? 0x7ff : 0x3ffff)
#define STACK_RND_MASK __STACK_RND_MASK(mmap_is_ia32())

#define ARCH_DLINFO \
--
2.30.2

2021-10-14 18:44:10

by Kees Cook

Subject: Re: [PATCH RFC] x86/elf: Fix incorrect STACK_RND_MASK for x86_64

On Thu, Oct 14, 2021 at 09:27:36PM +0800, [email protected] wrote:
> From: Xiongwei Song <[email protected]>
>
> According to the comment for stack randomization of x86, the range of
> randomization is 1GB, which occupies 30 valid bits in binary. And in
> x86_64, PAGE_SHIFT occupies 12 bits, then the STACK_RND_MASK should
> occupy 18 bits for 1GB.
>
> The current range of randomization is :
> 0x3fffff << PAGE_SHIFT
> , which occupies 34 bits.
>
> This patch changed 0x3fffff to 0x3ffff, which makes the range of stack
> randomization is real 1GB.

Why do we want to _reduce_ entropy here? Perhaps adjust the comment
instead?

-Kees

>
> Before this patch(bit0 ~ bit33 randomized):
> root@qemux86-64:~# for i in {1..20};do cat /proc/self/maps | grep stack;done
> 7ffddf971000-7ffddf992000 rw-p 00000000 00:00 0 [stack]
> 7ffc4e0d4000-7ffc4e0f5000 rw-p 00000000 00:00 0 [stack]
> 7fff5898e000-7fff589af000 rw-p 00000000 00:00 0 [stack]
> 7ffcf910b000-7ffcf912c000 rw-p 00000000 00:00 0 [stack]
> 7ffce829d000-7ffce82be000 rw-p 00000000 00:00 0 [stack]
> 7ffec72d9000-7ffec72fa000 rw-p 00000000 00:00 0 [stack]
> 7ffd6e256000-7ffd6e277000 rw-p 00000000 00:00 0 [stack]
> 7ffe14120000-7ffe14141000 rw-p 00000000 00:00 0 [stack]
> 7ffe21549000-7ffe2156a000 rw-p 00000000 00:00 0 [stack]
> 7ffdc9d33000-7ffdc9d54000 rw-p 00000000 00:00 0 [stack]
> 7ffe1ced4000-7ffe1cef5000 rw-p 00000000 00:00 0 [stack]
> 7ffcb0440000-7ffcb0461000 rw-p 00000000 00:00 0 [stack]
> 7ffc84515000-7ffc84536000 rw-p 00000000 00:00 0 [stack]
> 7ffe557dd000-7ffe557fe000 rw-p 00000000 00:00 0 [stack]
> 7ffdcb7e4000-7ffdcb805000 rw-p 00000000 00:00 0 [stack]
> 7ffc6f989000-7ffc6f9aa000 rw-p 00000000 00:00 0 [stack]
> 7ffd9322a000-7ffd9324b000 rw-p 00000000 00:00 0 [stack]
> 7ffea8e34000-7ffea8e55000 rw-p 00000000 00:00 0 [stack]
> 7ffc594ff000-7ffc59520000 rw-p 00000000 00:00 0 [stack]
> 7ffe8db65000-7ffe8db86000 rw-p 00000000 00:00 0 [stack]
>
> After this patch(bit0 ~ bit29 randomized):
> root@qemux86-64:~# for i in {1..20};do cat /proc/self/maps | grep stack;done
> 7fffd0ed2000-7fffd0ef3000 rw-p 00000000 00:00 0 [stack]
> 7fffdf555000-7fffdf576000 rw-p 00000000 00:00 0 [stack]
> 7ffffec0e000-7ffffec2f000 rw-p 00000000 00:00 0 [stack]
> 7fffcfa0f000-7fffcfa30000 rw-p 00000000 00:00 0 [stack]
> 7fffff75d000-7fffff77e000 rw-p 00000000 00:00 0 [stack]
> 7fffd1d11000-7fffd1d32000 rw-p 00000000 00:00 0 [stack]
> 7fffc7850000-7fffc7871000 rw-p 00000000 00:00 0 [stack]
> 7fffc956e000-7fffc958f000 rw-p 00000000 00:00 0 [stack]
> 7fffdf7be000-7fffdf7df000 rw-p 00000000 00:00 0 [stack]
> 7fffeffdf000-7ffff0000000 rw-p 00000000 00:00 0 [stack]
> 7fffd7d7f000-7fffd7da0000 rw-p 00000000 00:00 0 [stack]
> 7fffe3d23000-7fffe3d44000 rw-p 00000000 00:00 0 [stack]
> 7fffde4b7000-7fffde4d8000 rw-p 00000000 00:00 0 [stack]
> 7fffd477a000-7fffd479b000 rw-p 00000000 00:00 0 [stack]
> 7fffc0e95000-7fffc0eb6000 rw-p 00000000 00:00 0 [stack]
> 7fffdbdda000-7fffdbdfb000 rw-p 00000000 00:00 0 [stack]
> 7fffec737000-7fffec758000 rw-p 00000000 00:00 0 [stack]
> 7fffed163000-7fffed184000 rw-p 00000000 00:00 0 [stack]
> 7ffff9a9c000-7ffff9abd000 rw-p 00000000 00:00 0 [stack]
> 7fffff945000-7fffff966000 rw-p 00000000 00:00 0 [stack]
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Gabriel Krisman Bertazi <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: "Chang S. Bae" <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: [email protected]
> Signed-off-by: Xiongwei Song <[email protected]>
> ---
> arch/x86/include/asm/elf.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> index 29fea180a665..0b7a5510ebd5 100644
> --- a/arch/x86/include/asm/elf.h
> +++ b/arch/x86/include/asm/elf.h
> @@ -343,7 +343,7 @@ extern unsigned long get_sigframe_size(void);
> #else /* CONFIG_X86_32 */
>
> /* 1GB for 64bit, 8MB for 32bit */
> -#define __STACK_RND_MASK(is32bit) ((is32bit) ? 0x7ff : 0x3fffff)
> +#define __STACK_RND_MASK(is32bit) ((is32bit) ? 0x7ff : 0x3ffff)
> #define STACK_RND_MASK __STACK_RND_MASK(mmap_is_ia32())
>
> #define ARCH_DLINFO \
> --
> 2.30.2
>

--
Kees Cook

2021-10-15 10:55:14

by Xiongwei Song

Subject: Re: [PATCH RFC] x86/elf: Fix incorrect STACK_RND_MASK for x86_64

On Fri, Oct 15, 2021 at 12:10 AM Kees Cook <[email protected]> wrote:
>
> On Thu, Oct 14, 2021 at 09:27:36PM +0800, [email protected] wrote:
> > From: Xiongwei Song <[email protected]>
> >
> > According to the comment for stack randomization of x86, the range of
> > randomization is 1GB, which occupies 30 valid bits in binary. And in
> > x86_64, PAGE_SHIFT occupies 12 bits, then the STACK_RND_MASK should
> > occupy 18 bits for 1GB.
> >
> > The current range of randomization is :
> > 0x3fffff << PAGE_SHIFT
> > , which occupies 34 bits.
> >
> > This patch changed 0x3fffff to 0x3ffff, which makes the range of stack
> > randomization is real 1GB.
>
> Why do we want to _reduce_ entropy here? Perhaps adjust the comment
> instead?

Hi Kees,

Shouldn't x86_64 have consistent behavior like arm64, riscv or powerpc
on this? I'm not sure if 34 bits is too wasteful for stack. If 34 bits have no
side effects, then I have no objection with your suggestion.

Regards,
Xiongwei


>
> -Kees
>
> >
> > Before this patch(bit0 ~ bit33 randomized):
> > root@qemux86-64:~# for i in {1..20};do cat /proc/self/maps | grep stack;done
> > 7ffddf971000-7ffddf992000 rw-p 00000000 00:00 0 [stack]
> > 7ffc4e0d4000-7ffc4e0f5000 rw-p 00000000 00:00 0 [stack]
> > 7fff5898e000-7fff589af000 rw-p 00000000 00:00 0 [stack]
> > 7ffcf910b000-7ffcf912c000 rw-p 00000000 00:00 0 [stack]
> > 7ffce829d000-7ffce82be000 rw-p 00000000 00:00 0 [stack]
> > 7ffec72d9000-7ffec72fa000 rw-p 00000000 00:00 0 [stack]
> > 7ffd6e256000-7ffd6e277000 rw-p 00000000 00:00 0 [stack]
> > 7ffe14120000-7ffe14141000 rw-p 00000000 00:00 0 [stack]
> > 7ffe21549000-7ffe2156a000 rw-p 00000000 00:00 0 [stack]
> > 7ffdc9d33000-7ffdc9d54000 rw-p 00000000 00:00 0 [stack]
> > 7ffe1ced4000-7ffe1cef5000 rw-p 00000000 00:00 0 [stack]
> > 7ffcb0440000-7ffcb0461000 rw-p 00000000 00:00 0 [stack]
> > 7ffc84515000-7ffc84536000 rw-p 00000000 00:00 0 [stack]
> > 7ffe557dd000-7ffe557fe000 rw-p 00000000 00:00 0 [stack]
> > 7ffdcb7e4000-7ffdcb805000 rw-p 00000000 00:00 0 [stack]
> > 7ffc6f989000-7ffc6f9aa000 rw-p 00000000 00:00 0 [stack]
> > 7ffd9322a000-7ffd9324b000 rw-p 00000000 00:00 0 [stack]
> > 7ffea8e34000-7ffea8e55000 rw-p 00000000 00:00 0 [stack]
> > 7ffc594ff000-7ffc59520000 rw-p 00000000 00:00 0 [stack]
> > 7ffe8db65000-7ffe8db86000 rw-p 00000000 00:00 0 [stack]
> >
> > After this patch(bit0 ~ bit29 randomized):
> > root@qemux86-64:~# for i in {1..20};do cat /proc/self/maps | grep stack;done
> > 7fffd0ed2000-7fffd0ef3000 rw-p 00000000 00:00 0 [stack]
> > 7fffdf555000-7fffdf576000 rw-p 00000000 00:00 0 [stack]
> > 7ffffec0e000-7ffffec2f000 rw-p 00000000 00:00 0 [stack]
> > 7fffcfa0f000-7fffcfa30000 rw-p 00000000 00:00 0 [stack]
> > 7fffff75d000-7fffff77e000 rw-p 00000000 00:00 0 [stack]
> > 7fffd1d11000-7fffd1d32000 rw-p 00000000 00:00 0 [stack]
> > 7fffc7850000-7fffc7871000 rw-p 00000000 00:00 0 [stack]
> > 7fffc956e000-7fffc958f000 rw-p 00000000 00:00 0 [stack]
> > 7fffdf7be000-7fffdf7df000 rw-p 00000000 00:00 0 [stack]
> > 7fffeffdf000-7ffff0000000 rw-p 00000000 00:00 0 [stack]
> > 7fffd7d7f000-7fffd7da0000 rw-p 00000000 00:00 0 [stack]
> > 7fffe3d23000-7fffe3d44000 rw-p 00000000 00:00 0 [stack]
> > 7fffde4b7000-7fffde4d8000 rw-p 00000000 00:00 0 [stack]
> > 7fffd477a000-7fffd479b000 rw-p 00000000 00:00 0 [stack]
> > 7fffc0e95000-7fffc0eb6000 rw-p 00000000 00:00 0 [stack]
> > 7fffdbdda000-7fffdbdfb000 rw-p 00000000 00:00 0 [stack]
> > 7fffec737000-7fffec758000 rw-p 00000000 00:00 0 [stack]
> > 7fffed163000-7fffed184000 rw-p 00000000 00:00 0 [stack]
> > 7ffff9a9c000-7ffff9abd000 rw-p 00000000 00:00 0 [stack]
> > 7fffff945000-7fffff966000 rw-p 00000000 00:00 0 [stack]
> >
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: "H. Peter Anvin" <[email protected]>
> > Cc: Gabriel Krisman Bertazi <[email protected]>
> > Cc: Andy Lutomirski <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: "Chang S. Bae" <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Xiongwei Song <[email protected]>
> > ---
> > arch/x86/include/asm/elf.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> > index 29fea180a665..0b7a5510ebd5 100644
> > --- a/arch/x86/include/asm/elf.h
> > +++ b/arch/x86/include/asm/elf.h
> > @@ -343,7 +343,7 @@ extern unsigned long get_sigframe_size(void);
> > #else /* CONFIG_X86_32 */
> >
> > /* 1GB for 64bit, 8MB for 32bit */
> > -#define __STACK_RND_MASK(is32bit) ((is32bit) ? 0x7ff : 0x3fffff)
> > +#define __STACK_RND_MASK(is32bit) ((is32bit) ? 0x7ff : 0x3ffff)
> > #define STACK_RND_MASK __STACK_RND_MASK(mmap_is_ia32())
> >
> > #define ARCH_DLINFO \
> > --
> > 2.30.2
> >
>
> --
> Kees Cook