2021-10-11 16:39:29

by Xiongwei Song

[permalink] [raw]
Subject: [PATCH v2 0/6] Use generic code for randomization of virtual address of x86

From: Xiongwei Song <[email protected]>

Hello,

This patchset are to use generic code for randomization of virtual address
of x86. Since the basic code logic of x86 is same as generic code, so no
need to implement these functions on x86.

Patch 1~3 are prepared to change the generic code to apply to x86.

Patch 4 is to switch to generic arch_pick_mmap_layout() with
ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT enabled. Also provided basically
test and the result was put in commit message too.

Patch 5~6 are used to handle the legacy things.

Test programs(to verify if the entropy of return value of mmap is kept
after applying the patchset):
- C code for mmap test:
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>

int main(int argc, char *argv[])
{
unsigned long *addr;

addr = mmap(NULL, 4096, PROT_READ, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
if (addr == MAP_FAILED) {
printf("NULL\n");
} else {
printf("%lx\n", (unsigned long)addr);
munmap(addr, 4096);
}

return 0;
}

- Shell script for collecting output of C progarm above and give a
statistics:
#!/bin/bash
declare -a COUNT

if [ "$1" == "" ]; then
echo "Please give a test number!"
exit 1
fi
number=$1

for ((i=0; i<$number; i++))
do
addr=$(mmaptest)
addr=$(((16#$addr&0xf000000000)>>36))
COUNT[$addr]=$((COUNT[$addr]+1))
done

echo " Virtual Address Range | hit times "
echo "----------------------------------------"
for ((i=0; i<16; i++))
do
j=`echo "obase=16; $i" | bc`
echo "0x7f${j,,}000000000 - 0x7f${j,,}ffffff000 | ${COUNT[i]}"
done

Run 10 thousands times C progam, collect the output with shell script, get
the test results below:
Before the patchset:
Virtual Address Range | hit times
----------------------------------------
0x7f0000000000 - 0x7f0ffffff000 | 655
0x7f1000000000 - 0x7f1ffffff000 | 617
0x7f2000000000 - 0x7f2ffffff000 | 636
0x7f3000000000 - 0x7f3ffffff000 | 625
0x7f4000000000 - 0x7f4ffffff000 | 651
0x7f5000000000 - 0x7f5ffffff000 | 591
0x7f6000000000 - 0x7f6ffffff000 | 623
0x7f7000000000 - 0x7f7ffffff000 | 627
0x7f8000000000 - 0x7f8ffffff000 | 638
0x7f9000000000 - 0x7f9ffffff000 | 586
0x7fa000000000 - 0x7faffffff000 | 637
0x7fb000000000 - 0x7fbffffff000 | 607
0x7fc000000000 - 0x7fcffffff000 | 618
0x7fd000000000 - 0x7fdffffff000 | 656
0x7fe000000000 - 0x7feffffff000 | 614
0x7ff000000000 - 0x7ffffffff000 | 619

After the patchset:
Virtual Address Range | hit times
----------------------------------------
0x7f0000000000 - 0x7f0ffffff000 | 661
0x7f1000000000 - 0x7f1ffffff000 | 645
0x7f2000000000 - 0x7f2ffffff000 | 609
0x7f3000000000 - 0x7f3ffffff000 | 594
0x7f4000000000 - 0x7f4ffffff000 | 616
0x7f5000000000 - 0x7f5ffffff000 | 622
0x7f6000000000 - 0x7f6ffffff000 | 617
0x7f7000000000 - 0x7f7ffffff000 | 582
0x7f8000000000 - 0x7f8ffffff000 | 618
0x7f9000000000 - 0x7f9ffffff000 | 629
0x7fa000000000 - 0x7faffffff000 | 635
0x7fb000000000 - 0x7fbffffff000 | 625
0x7fc000000000 - 0x7fcffffff000 | 614
0x7fd000000000 - 0x7fdffffff000 | 610
0x7fe000000000 - 0x7feffffff000 | 648
0x7ff000000000 - 0x7ffffffff000 | 675

v1 -> v2:
- Spilt the patch 2 of v1 as Kees suggested.
- Drop patch 1 of v1, which renamed TIF_ADDR32 to TIF_32BIT, which is
unreasonable for x86. Because in x86, 64bit process can call 32bit
syscall. Thanks Peterz for pointing this out.

v1:
- https://lkml.org/lkml/2021/9/21/482
- https://lkml.org/lkml/2021/9/21/484
- https://lkml.org/lkml/2021/9/27/688

Please review.

Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Gabriel Krisman Bertazi <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Huang Rui <[email protected]>
Cc: Yazen Ghannam <[email protected]>
Cc: Kim Phillips <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Cc: [email protected]

Xiongwei Song (6):
mm/util: Assign a meaningful value to mmap_legacy_base
mm/util: Allow to pass a specific task size when getting mmapping base
mm/util: Support CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES
x86/mm: Randomize VA with generit arch_pick_mmap_layout()
x86/mm: Discard the defination of HAVE_ARCH_PICK_MMAP_LAYOUT
x86/elf: Discard ARCH_HAS_ELF_RANDOMIZE selection

arch/x86/Kconfig | 2 +-
arch/x86/include/asm/compat.h | 5 ++
arch/x86/include/asm/processor.h | 5 +-
arch/x86/mm/mmap.c | 112 -------------------------------
mm/util.c | 35 +++++++---
5 files changed, 37 insertions(+), 122 deletions(-)

--
2.30.2


2021-10-11 16:39:33

by Xiongwei Song

[permalink] [raw]
Subject: [PATCH v2 4/6] x86/mm: Randomize VA with generic arch_pick_mmap_layout()

From: Xiongwei Song <[email protected]>

The code logic of arch_pick_mmap_layout() of x86 is basiclly same as
arch_pick_mmap_layout() where is in mm/util.c. Let's enable
ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT for x86 to use generic
arch_pick_mmap_layout(). Meanwhile, delete the arch_pick_mmap_layout()
and the related assistant functions in x86.

To verify if the entropy of mmap is kept after the patch, I did basically
test with the following c code:
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>

int main(int argc, char *argv[])
{
unsigned long *addr;

addr = mmap(NULL, 4096, PROT_READ, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
if (addr == MAP_FAILED) {
printf("NULL\n");
} else {
printf("%lx\n", (unsigned long)addr);
munmap(addr, 4096);
}

return 0;
}

Run the program above 10 thousands times to get the mmap address, please
see the results below.

Before this patch:
Virtual Address Range | hit times
----------------------------------------
0x7f0000000000 - 0x7f0ffffff000 | 655
0x7f1000000000 - 0x7f1ffffff000 | 617
0x7f2000000000 - 0x7f2ffffff000 | 636
0x7f3000000000 - 0x7f3ffffff000 | 625
0x7f4000000000 - 0x7f4ffffff000 | 651
0x7f5000000000 - 0x7f5ffffff000 | 591
0x7f6000000000 - 0x7f6ffffff000 | 623
0x7f7000000000 - 0x7f7ffffff000 | 627
0x7f8000000000 - 0x7f8ffffff000 | 638
0x7f9000000000 - 0x7f9ffffff000 | 586
0x7fa000000000 - 0x7faffffff000 | 637
0x7fb000000000 - 0x7fbffffff000 | 607
0x7fc000000000 - 0x7fcffffff000 | 618
0x7fd000000000 - 0x7fdffffff000 | 656
0x7fe000000000 - 0x7feffffff000 | 614
0x7ff000000000 - 0x7ffffffff000 | 619

After this patch:
Virtual Address Range | hit times
----------------------------------------
0x7f0000000000 - 0x7f0ffffff000 | 661
0x7f1000000000 - 0x7f1ffffff000 | 645
0x7f2000000000 - 0x7f2ffffff000 | 609
0x7f3000000000 - 0x7f3ffffff000 | 594
0x7f4000000000 - 0x7f4ffffff000 | 616
0x7f5000000000 - 0x7f5ffffff000 | 622
0x7f6000000000 - 0x7f6ffffff000 | 617
0x7f7000000000 - 0x7f7ffffff000 | 582
0x7f8000000000 - 0x7f8ffffff000 | 618
0x7f9000000000 - 0x7f9ffffff000 | 629
0x7fa000000000 - 0x7faffffff000 | 635
0x7fb000000000 - 0x7fbffffff000 | 625
0x7fc000000000 - 0x7fcffffff000 | 614
0x7fd000000000 - 0x7fdffffff000 | 610
0x7fe000000000 - 0x7feffffff000 | 648
0x7ff000000000 - 0x7ffffffff000 | 675

It looks like the result after the patch is reasonable.

Furthermore, define a is_compat_task() for x86 to fix function undefined
issue.

Add __weak attribute to generic arch_randomize_brk() to ensure the kernel
always uses the arch_randomize_brk() of x86 in x86 arch.

Signed-off-by: Xiongwei Song <[email protected]>
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/compat.h | 5 ++
arch/x86/mm/mmap.c | 112 ----------------------------------
mm/util.c | 2 +-
4 files changed, 7 insertions(+), 113 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a52e81cb256e..01a40b710103 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -114,6 +114,7 @@ config X86
select ARCH_USE_SYM_ANNOTATIONS
select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
+ select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
select ARCH_WANTS_DYNAMIC_TASK_STRUCT
select ARCH_WANTS_NO_INSTR
select ARCH_WANT_HUGE_PMD_SHARE
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 7516e4199b3c..22714a202794 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -156,6 +156,11 @@ struct compat_shmid64_ds {
(!!(task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT))
#endif

+static inline int is_compat_task(void)
+{
+ return IS_ENABLED(CONFIG_COMPAT) && test_thread_flag(TIF_ADDR32);
+}
+
static inline bool in_x32_syscall(void)
{
#ifdef CONFIG_X86_X32_ABI
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index c90c20904a60..daf65cc5e5b1 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -38,118 +38,6 @@ unsigned long task_size_64bit(int full_addr_space)
return full_addr_space ? TASK_SIZE_MAX : DEFAULT_MAP_WINDOW;
}

-static unsigned long stack_maxrandom_size(unsigned long task_size)
-{
- unsigned long max = 0;
- if (current->flags & PF_RANDOMIZE) {
- max = (-1UL) & __STACK_RND_MASK(task_size == task_size_32bit());
- max <<= PAGE_SHIFT;
- }
-
- return max;
-}
-
-#ifdef CONFIG_COMPAT
-# define mmap32_rnd_bits mmap_rnd_compat_bits
-# define mmap64_rnd_bits mmap_rnd_bits
-#else
-# define mmap32_rnd_bits mmap_rnd_bits
-# define mmap64_rnd_bits mmap_rnd_bits
-#endif
-
-#define SIZE_128M (128 * 1024 * 1024UL)
-
-static int mmap_is_legacy(void)
-{
- if (current->personality & ADDR_COMPAT_LAYOUT)
- return 1;
-
- return sysctl_legacy_va_layout;
-}
-
-static unsigned long arch_rnd(unsigned int rndbits)
-{
- if (!(current->flags & PF_RANDOMIZE))
- return 0;
- return (get_random_long() & ((1UL << rndbits) - 1)) << PAGE_SHIFT;
-}
-
-unsigned long arch_mmap_rnd(void)
-{
- return arch_rnd(mmap_is_ia32() ? mmap32_rnd_bits : mmap64_rnd_bits);
-}
-
-static unsigned long mmap_base(unsigned long rnd, unsigned long task_size,
- struct rlimit *rlim_stack)
-{
- unsigned long gap = rlim_stack->rlim_cur;
- unsigned long pad = stack_maxrandom_size(task_size) + stack_guard_gap;
- unsigned long gap_min, gap_max;
-
- /* Values close to RLIM_INFINITY can overflow. */
- if (gap + pad > gap)
- gap += pad;
-
- /*
- * Top of mmap area (just below the process stack).
- * Leave an at least ~128 MB hole with possible stack randomization.
- */
- gap_min = SIZE_128M;
- gap_max = (task_size / 6) * 5;
-
- if (gap < gap_min)
- gap = gap_min;
- else if (gap > gap_max)
- gap = gap_max;
-
- return PAGE_ALIGN(task_size - gap - rnd);
-}
-
-static unsigned long mmap_legacy_base(unsigned long rnd,
- unsigned long task_size)
-{
- return __TASK_UNMAPPED_BASE(task_size) + rnd;
-}
-
-/*
- * This function, called very early during the creation of a new
- * process VM image, sets up which VM layout function to use:
- */
-static void arch_pick_mmap_base(unsigned long *base, unsigned long *legacy_base,
- unsigned long random_factor, unsigned long task_size,
- struct rlimit *rlim_stack)
-{
- *legacy_base = mmap_legacy_base(random_factor, task_size);
- if (mmap_is_legacy())
- *base = *legacy_base;
- else
- *base = mmap_base(random_factor, task_size, rlim_stack);
-}
-
-void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
-{
- if (mmap_is_legacy())
- mm->get_unmapped_area = arch_get_unmapped_area;
- else
- mm->get_unmapped_area = arch_get_unmapped_area_topdown;
-
- arch_pick_mmap_base(&mm->mmap_base, &mm->mmap_legacy_base,
- arch_rnd(mmap64_rnd_bits), task_size_64bit(0),
- rlim_stack);
-
-#ifdef CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES
- /*
- * The mmap syscall mapping base decision depends solely on the
- * syscall type (64-bit or compat). This applies for 64bit
- * applications and 32bit applications. The 64bit syscall uses
- * mmap_base, the compat syscall uses mmap_compat_base.
- */
- arch_pick_mmap_base(&mm->mmap_compat_base, &mm->mmap_compat_legacy_base,
- arch_rnd(mmap32_rnd_bits), task_size_32bit(),
- rlim_stack);
-#endif
-}
-
unsigned long get_mmap_base(int is_legacy)
{
struct mm_struct *mm = current->mm;
diff --git a/mm/util.c b/mm/util.c
index ab3711c445e6..91a26da501d3 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -344,7 +344,7 @@ unsigned long randomize_stack_top(unsigned long stack_top)
}

#ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
-unsigned long arch_randomize_brk(struct mm_struct *mm)
+unsigned long __weak arch_randomize_brk(struct mm_struct *mm)
{
/* Is the current task 32bit ? */
if (!IS_ENABLED(CONFIG_64BIT) || is_compat_task())
--
2.30.2

2021-10-11 16:39:57

by Xiongwei Song

[permalink] [raw]
Subject: [PATCH v2 3/6] mm/util: Support CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES

From: Xiongwei Song <[email protected]>

With CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES enabled, archs can support 64bit
process invoke 32bit syscall via compatible mmap(), which uses
mmap_compat_base and mmap_compat_legacy_base of mm_struct, which only
x86 uses so far.

Here we assume other archs will support CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES
as well.

Signed-off-by: Xiongwei Song <[email protected]>
---
arch/x86/include/asm/processor.h | 4 ++++
mm/util.c | 21 +++++++++++++++++++--
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 577f342dbfb2..fb7a4f21d412 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -787,6 +787,10 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
#define __TASK_UNMAPPED_BASE(task_size) (PAGE_ALIGN(task_size / 3))
#define TASK_UNMAPPED_BASE __TASK_UNMAPPED_BASE(TASK_SIZE_LOW)

+#ifdef CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES
+#define TASK_UNMAPPED_COMPAT_BASE __TASK_UNMAPPED_BASE(task_size_32bit())
+#endif
+
#define KSTK_EIP(task) (task_pt_regs(task)->ip)

/* Get/set a process' ability to use the timestamp counter instruction */
diff --git a/mm/util.c b/mm/util.c
index 38326ef21a3b..ab3711c445e6 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -357,8 +357,9 @@ unsigned long arch_mmap_rnd(void)
{
unsigned long rnd;

-#ifdef CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS
- if (is_compat_task())
+#if defined(CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS) \
+ || defined(CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES)
+ if (is_compat_task() || in_compat_syscall())
rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
else
#endif /* CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS */
@@ -422,6 +423,22 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
mm->mmap_base = mmap_base(random_factor, STACK_TOP, rlim_stack);
mm->get_unmapped_area = arch_get_unmapped_area_topdown;
}
+#ifdef CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES
+ /*
+ * The mmap syscall mapping base decision depends solely on the
+ * syscall type (64-bit or compat). This applies for 64bit
+ * applications and 32bit applications. The 64bit syscall uses
+ * mmap_base, the compat syscall uses mmap_compat_base.
+ */
+ if (mmap_is_legacy(rlim_stack)) {
+ mm->mmap_compat_legacy_base =
+ TASK_UNMAPPED_COMPAT_BASE + random_factor;
+ mm->mmap_compat_base = mm->mmap_compat_legacy_base;
+ } else {
+ mm->mmap_compat_base = mmap_base(random_factor,
+ task_size_32bit(), rlim_stack);
+ }
+#endif
}
#elif defined(CONFIG_MMU) && !defined(HAVE_ARCH_PICK_MMAP_LAYOUT)
void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
--
2.30.2

2021-10-11 16:40:21

by Xiongwei Song

[permalink] [raw]
Subject: [PATCH v2 1/6] mm/util: Assign a meaningful value to mmap_legacy_base

From: Xiongwei Song <[email protected]>

Let's assign a value to mmap_legacy_base in case the mmap() of some
archs needs mmap_legacy_base, like x86.

Signed-off-by: Xiongwei Song <[email protected]>
---
mm/util.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/util.c b/mm/util.c
index e58151a61255..40b1a8837c0b 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -414,7 +414,8 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
random_factor = arch_mmap_rnd();

if (mmap_is_legacy(rlim_stack)) {
- mm->mmap_base = TASK_UNMAPPED_BASE + random_factor;
+ mm->mmap_legacy_base = TASK_UNMAPPED_BASE + random_factor;
+ mm->mmap_base = mm->mmap_legacy_base;
mm->get_unmapped_area = arch_get_unmapped_area;
} else {
mm->mmap_base = mmap_base(random_factor, rlim_stack);
--
2.30.2

2021-10-11 16:40:58

by Xiongwei Song

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Use generic code for randomization of virtual address of x86



> On Oct 11, 2021, at 10:31 PM, [email protected] wrote:
>
> From: Xiongwei Song <[email protected]>
>
> Hello,
>
> This patchset are to use generic code for randomization of virtual address
> of x86. Since the basic code logic of x86 is same as generic code, so no
> need to implement these functions on x86.
>
> Patch 1~3 are prepared to change the generic code to apply to x86.
>
> Patch 4 is to switch to generic arch_pick_mmap_layout() with
> ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT enabled. Also provided basically
> test and the result was put in commit message too.
>
> Patch 5~6 are used to handle the legacy things.
>
> Test programs(to verify if the entropy of return value of mmap is kept
> after applying the patchset):
> - C code for mmap test:
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/mman.h>
>
> int main(int argc, char *argv[])
> {
> unsigned long *addr;
>
> addr = mmap(NULL, 4096, PROT_READ, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> if (addr == MAP_FAILED) {
> printf("NULL\n");
> } else {
> printf("%lx\n", (unsigned long)addr);
> munmap(addr, 4096);
> }
>
> return 0;
> }
>
> - Shell script for collecting output of C progarm above and give a
> statistics:
> #!/bin/bash
> declare -a COUNT
>
> if [ "$1" == "" ]; then
> echo "Please give a test number!"
> exit 1
> fi
> number=$1
>
> for ((i=0; i<$number; i++))
> do
> addr=$(mmaptest)
> addr=$(((16#$addr&0xf000000000)>>36))
> COUNT[$addr]=$((COUNT[$addr]+1))
> done
>
> echo " Virtual Address Range | hit times "
> echo "----------------------------------------"
> for ((i=0; i<16; i++))
> do
> j=`echo "obase=16; $i" | bc`
> echo "0x7f${j,,}000000000 - 0x7f${j,,}ffffff000 | ${COUNT[i]}"
> done
>
> Run 10 thousands times C progam, collect the output with shell script, get
> the test results below:
> Before the patchset:
> Virtual Address Range | hit times
> ----------------------------------------
> 0x7f0000000000 - 0x7f0ffffff000 | 655
> 0x7f1000000000 - 0x7f1ffffff000 | 617
> 0x7f2000000000 - 0x7f2ffffff000 | 636
> 0x7f3000000000 - 0x7f3ffffff000 | 625
> 0x7f4000000000 - 0x7f4ffffff000 | 651
> 0x7f5000000000 - 0x7f5ffffff000 | 591
> 0x7f6000000000 - 0x7f6ffffff000 | 623
> 0x7f7000000000 - 0x7f7ffffff000 | 627
> 0x7f8000000000 - 0x7f8ffffff000 | 638
> 0x7f9000000000 - 0x7f9ffffff000 | 586
> 0x7fa000000000 - 0x7faffffff000 | 637
> 0x7fb000000000 - 0x7fbffffff000 | 607
> 0x7fc000000000 - 0x7fcffffff000 | 618
> 0x7fd000000000 - 0x7fdffffff000 | 656
> 0x7fe000000000 - 0x7feffffff000 | 614
> 0x7ff000000000 - 0x7ffffffff000 | 619
>
> After the patchset:
> Virtual Address Range | hit times
> ----------------------------------------
> 0x7f0000000000 - 0x7f0ffffff000 | 661
> 0x7f1000000000 - 0x7f1ffffff000 | 645
> 0x7f2000000000 - 0x7f2ffffff000 | 609
> 0x7f3000000000 - 0x7f3ffffff000 | 594
> 0x7f4000000000 - 0x7f4ffffff000 | 616
> 0x7f5000000000 - 0x7f5ffffff000 | 622
> 0x7f6000000000 - 0x7f6ffffff000 | 617
> 0x7f7000000000 - 0x7f7ffffff000 | 582
> 0x7f8000000000 - 0x7f8ffffff000 | 618
> 0x7f9000000000 - 0x7f9ffffff000 | 629
> 0x7fa000000000 - 0x7faffffff000 | 635
> 0x7fb000000000 - 0x7fbffffff000 | 625
> 0x7fc000000000 - 0x7fcffffff000 | 614
> 0x7fd000000000 - 0x7fdffffff000 | 610
> 0x7fe000000000 - 0x7feffffff000 | 648
> 0x7ff000000000 - 0x7ffffffff000 | 675

Hi Kees,

Sorry, I have no idea about the entropy measure tools, so I designed a test program
myself. I’m not sure if my test is enough. Or could you please share a better method to
measure entropy?

Regards,
Xiongwei


2021-10-11 16:41:15

by Xiongwei Song

[permalink] [raw]
Subject: [PATCH v2 5/6] x86/mm: Discard the defination of HAVE_ARCH_PICK_MMAP_LAYOUT

From: Xiongwei Song <[email protected]>

We've enabled ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT, we will always use
generic arch_pick_mmap_layout(), therefore don't need
HAVE_ARCH_PICK_MMAP_LAYOUT any more.

Signed-off-by: Xiongwei Song <[email protected]>
---
arch/x86/include/asm/processor.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index fb7a4f21d412..2360366ad328 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -709,7 +709,6 @@ extern int bootloader_version;

extern char ignore_fpu_irq;

-#define HAVE_ARCH_PICK_MMAP_LAYOUT 1
#define ARCH_HAS_PREFETCHW
#define ARCH_HAS_SPINLOCK_PREFETCH

--
2.30.2

2021-10-11 16:41:36

by Xiongwei Song

[permalink] [raw]
Subject: [PATCH v2 2/6] mm/util: Allow to pass a specific task size when getting mmapping base

From: Xiongwei Song <[email protected]>

In x86, a 64bit task may invoke a 32 bit syscall, which is in compat
syscall. Then we have to provide 32bit mapping base.

Signed-off-by: Xiongwei Song <[email protected]>
---
mm/util.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/util.c b/mm/util.c
index 40b1a8837c0b..38326ef21a3b 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -385,14 +385,15 @@ static int mmap_is_legacy(struct rlimit *rlim_stack)
#define MIN_GAP (SZ_128M)
#define MAX_GAP (STACK_TOP / 6 * 5)

-static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
+static unsigned long mmap_base(unsigned long rnd, unsigned long task_size,
+ struct rlimit *rlim_stack)
{
unsigned long gap = rlim_stack->rlim_cur;
unsigned long pad = stack_guard_gap;

/* Account for stack randomization if necessary */
if (current->flags & PF_RANDOMIZE)
- pad += (STACK_RND_MASK << PAGE_SHIFT);
+ pad += (in_compat_syscall() ? 0x7ff : STACK_RND_MASK) << PAGE_SHIFT;

/* Values close to RLIM_INFINITY can overflow. */
if (gap + pad > gap)
@@ -403,7 +404,7 @@ static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
else if (gap > MAX_GAP)
gap = MAX_GAP;

- return PAGE_ALIGN(STACK_TOP - gap - rnd);
+ return PAGE_ALIGN(task_size - gap - rnd);
}

void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
@@ -418,7 +419,7 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
mm->mmap_base = mm->mmap_legacy_base;
mm->get_unmapped_area = arch_get_unmapped_area;
} else {
- mm->mmap_base = mmap_base(random_factor, rlim_stack);
+ mm->mmap_base = mmap_base(random_factor, STACK_TOP, rlim_stack);
mm->get_unmapped_area = arch_get_unmapped_area_topdown;
}
}
--
2.30.2

2021-10-16 09:36:54

by Xiongwei Song

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Use generic code for randomization of virtual address of x86

Hi Experts,

Gentle reminder. Any comments would be appreciated.
Thank you so much.

Regards,
Xiongwei

> On Oct 11, 2021, at 10:31 PM, [email protected] wrote:
>
> From: Xiongwei Song <[email protected]>
>
> Hello,
>
> This patchset are to use generic code for randomization of virtual address
> of x86. Since the basic code logic of x86 is same as generic code, so no
> need to implement these functions on x86.
>
> Patch 1~3 are prepared to change the generic code to apply to x86.
>
> Patch 4 is to switch to generic arch_pick_mmap_layout() with
> ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT enabled. Also provided basically
> test and the result was put in commit message too.
>
> Patch 5~6 are used to handle the legacy things.
>
> Test programs(to verify if the entropy of return value of mmap is kept
> after applying the patchset):
> - C code for mmap test:
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/mman.h>
>
> int main(int argc, char *argv[])
> {
> unsigned long *addr;
>
> addr = mmap(NULL, 4096, PROT_READ, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> if (addr == MAP_FAILED) {
> printf("NULL\n");
> } else {
> printf("%lx\n", (unsigned long)addr);
> munmap(addr, 4096);
> }
>
> return 0;
> }
>
> - Shell script for collecting output of C progarm above and give a
> statistics:
> #!/bin/bash
> declare -a COUNT
>
> if [ "$1" == "" ]; then
> echo "Please give a test number!"
> exit 1
> fi
> number=$1
>
> for ((i=0; i<$number; i++))
> do
> addr=$(mmaptest)
> addr=$(((16#$addr&0xf000000000)>>36))
> COUNT[$addr]=$((COUNT[$addr]+1))
> done
>
> echo " Virtual Address Range | hit times "
> echo "----------------------------------------"
> for ((i=0; i<16; i++))
> do
> j=`echo "obase=16; $i" | bc`
> echo "0x7f${j,,}000000000 - 0x7f${j,,}ffffff000 | ${COUNT[i]}"
> done
>
> Run 10 thousands times C progam, collect the output with shell script, get
> the test results below:
> Before the patchset:
> Virtual Address Range | hit times
> ----------------------------------------
> 0x7f0000000000 - 0x7f0ffffff000 | 655
> 0x7f1000000000 - 0x7f1ffffff000 | 617
> 0x7f2000000000 - 0x7f2ffffff000 | 636
> 0x7f3000000000 - 0x7f3ffffff000 | 625
> 0x7f4000000000 - 0x7f4ffffff000 | 651
> 0x7f5000000000 - 0x7f5ffffff000 | 591
> 0x7f6000000000 - 0x7f6ffffff000 | 623
> 0x7f7000000000 - 0x7f7ffffff000 | 627
> 0x7f8000000000 - 0x7f8ffffff000 | 638
> 0x7f9000000000 - 0x7f9ffffff000 | 586
> 0x7fa000000000 - 0x7faffffff000 | 637
> 0x7fb000000000 - 0x7fbffffff000 | 607
> 0x7fc000000000 - 0x7fcffffff000 | 618
> 0x7fd000000000 - 0x7fdffffff000 | 656
> 0x7fe000000000 - 0x7feffffff000 | 614
> 0x7ff000000000 - 0x7ffffffff000 | 619
>
> After the patchset:
> Virtual Address Range | hit times
> ----------------------------------------
> 0x7f0000000000 - 0x7f0ffffff000 | 661
> 0x7f1000000000 - 0x7f1ffffff000 | 645
> 0x7f2000000000 - 0x7f2ffffff000 | 609
> 0x7f3000000000 - 0x7f3ffffff000 | 594
> 0x7f4000000000 - 0x7f4ffffff000 | 616
> 0x7f5000000000 - 0x7f5ffffff000 | 622
> 0x7f6000000000 - 0x7f6ffffff000 | 617
> 0x7f7000000000 - 0x7f7ffffff000 | 582
> 0x7f8000000000 - 0x7f8ffffff000 | 618
> 0x7f9000000000 - 0x7f9ffffff000 | 629
> 0x7fa000000000 - 0x7faffffff000 | 635
> 0x7fb000000000 - 0x7fbffffff000 | 625
> 0x7fc000000000 - 0x7fcffffff000 | 614
> 0x7fd000000000 - 0x7fdffffff000 | 610
> 0x7fe000000000 - 0x7feffffff000 | 648
> 0x7ff000000000 - 0x7ffffffff000 | 675
>
> v1 -> v2:
> - Spilt the patch 2 of v1 as Kees suggested.
> - Drop patch 1 of v1, which renamed TIF_ADDR32 to TIF_32BIT, which is
> unreasonable for x86. Because in x86, 64bit process can call 32bit
> syscall. Thanks Peterz for pointing this out.
>
> v1:
> - https://lkml.org/lkml/2021/9/21/482
> - https://lkml.org/lkml/2021/9/21/484
> - https://lkml.org/lkml/2021/9/27/688
>
> Please review.
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Gabriel Krisman Bertazi <[email protected]>
> Cc: Lai Jiangshan <[email protected]>
> Cc: Huang Rui <[email protected]>
> Cc: Yazen Ghannam <[email protected]>
> Cc: Kim Phillips <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> Xiongwei Song (6):
> mm/util: Assign a meaningful value to mmap_legacy_base
> mm/util: Allow to pass a specific task size when getting mmapping base
> mm/util: Support CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES
> x86/mm: Randomize VA with generit arch_pick_mmap_layout()
> x86/mm: Discard the defination of HAVE_ARCH_PICK_MMAP_LAYOUT
> x86/elf: Discard ARCH_HAS_ELF_RANDOMIZE selection
>
> arch/x86/Kconfig | 2 +-
> arch/x86/include/asm/compat.h | 5 ++
> arch/x86/include/asm/processor.h | 5 +-
> arch/x86/mm/mmap.c | 112 -------------------------------
> mm/util.c | 35 +++++++---
> 5 files changed, 37 insertions(+), 122 deletions(-)
>
> --
> 2.30.2
>