Skip to contentSkip to conversation

Fix compatible mmap() return pointer over 4Gb

PATCH Superseded Dmitry Safonov 4 participants 16 messages Jan 16, 2017 – Jan 18, 2017

Subsystems (5)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover Fix compatible mmap() return pointer over 4Gb

Changes since v1:
- Recalculate mmap_base instead of using max possible virtual address for compat/native syscall. That will make policy for allocation the same in 32-bit binaries and in 32-bit syscalls in 64-bit binaries. I need this because sys_mmap() in restored 32-bit process shouldn't hit the stack area.
- Fixed mmap() with MAP_32BIT flag in the same usecases
- used in_compat_syscall() helper rather TS_COMPAT check (Andy noticed)
- introduced find_top() helper as suggested by Andy to simplify code
- fixed test error-handeling: it checked the result of sys_mmap() with MMAP_FAILED, which is not correct, as it calls raw syscall - now checks return value to be aligned to PAGE_SIZE.

Description from v1 [2]:

A fix for bug in mmap() that I referenced in [1].
Also selftest for it.

I would like to mark the fix as for stable v4.9 kernel if it'll be accepted, as I try to support compatible 32-bit C/R after v4.9 and working compatible mmap() is really wanted there.

[1]: https://marc.info/?l=linux-kernel&m=148311451525315
[2]: https://marc.info/?l=linux-kernel&m=148415888707662

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>

Dmitry Safonov (5):
x86/mm: split arch_mmap_rnd() on compat/native versions x86/mm: introduce mmap_{,legacy}_base
x86/mm: fix native mmap() in compat bins and vice-versa x86/mm: for MAP_32BIT check in_compat_syscall() instead TIF_ADDR32 selftests/x86: add test to check compat mmap() return addr

 arch/x86/include/asm/elf.h                     |  14 +-
 arch/x86/include/asm/processor.h               |   2 +-
 arch/x86/kernel/sys_x86_64.c                   |  48 +++++-
 arch/x86/mm/hugetlbpage.c                      |   2 +-
 arch/x86/mm/mmap.c                             |  56 ++++---
 tools/testing/selftests/x86/Makefile           |   2 +-
 tools/testing/selftests/x86/test_compat_mmap.c | 208 +++++++++++++++++++++++++
 7 files changed, 301 insertions(+), 31 deletions(-)
 create mode 100644 tools/testing/selftests/x86/test_compat_mmap.c
Signature · 7 lines
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#1 lore raw

PATCH v2 1/5 x86/mm: split arch_mmap_rnd() on compat/native versions

I need those arch_{native,compat}_rnd() to compute separately random factor for mmap() in compat syscalls for 64-bit binaries and vice-versa for native syscall in 32-bit compat binaries. They will be used in the following patches.

---
 arch/x86/include/asm/elf.h |  5 +++++
 arch/x86/mm/mmap.c         | 25 ++++++++++++++++---------
 2 files changed, 21 insertions(+), 9 deletions(-)
arch/x86/include/asm/elf.h +5 -0
index e7f155c3045e..ee1a87782b2c 100644
@@ -349,6 +349,11 @@ static inline int mmap_is_ia32(void)
 		test_thread_flag(TIF_ADDR32));
 }
 
+#ifdef CONFIG_COMPAT
+extern unsigned long arch_compat_rnd(void);
+#endif
+extern unsigned long arch_native_rnd(void);
+
 /* Do not change the values. See get_align_mask() */
 enum align_flags {
 	ALIGN_VA_32	= BIT(0),
arch/x86/mm/mmap.c +16 -9
index d2dc0438d654..0b2007b08194 100644
@@ -65,20 +65,27 @@ static int mmap_is_legacy(void)
 	return sysctl_legacy_va_layout;
 }
 
-unsigned long arch_mmap_rnd(void)
+#ifdef CONFIG_COMPAT
+unsigned long arch_compat_rnd(void)
 {
-	unsigned long rnd;
+	return (get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1))
+		<< PAGE_SHIFT;
+}
+#endif
 
-	if (mmap_is_ia32())
+unsigned long arch_native_rnd(void)
+{
+	return (get_random_long() & ((1UL << mmap_rnd_bits) - 1)) << PAGE_SHIFT;
+}
+
+unsigned long arch_mmap_rnd(void)
+{
 #ifdef CONFIG_COMPAT
-		rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
-#else
-		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
+	if (mmap_is_ia32())
+		return arch_compat_rnd();
 #endif
-	else
-		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
 
-	return rnd << PAGE_SHIFT;
+	return arch_native_rnd();
 }
 
 static unsigned long mmap_base(unsigned long rnd)
Signature · 7 lines
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw

PATCH v2 2/5 x86/mm: introduce mmap_{,legacy}_base

In reply to Dmitry Safonov

In the following patch they will be used to compute:
- mmap_base in compat sys_mmap() in native 64-bit binary and vice-versa
- mmap_base for native sys_mmap() in compat x32/ia32-bit binary.

---
 arch/x86/include/asm/elf.h       |  9 +++++++--
 arch/x86/include/asm/processor.h |  2 +-
 arch/x86/mm/hugetlbpage.c        |  2 +-
 arch/x86/mm/mmap.c               | 31 +++++++++++++++++++------------
 4 files changed, 28 insertions(+), 16 deletions(-)
arch/x86/include/asm/elf.h +7 -2
index ee1a87782b2c..9655a8390da4 100644
@@ -286,6 +286,7 @@ do {									\
 
 #ifdef CONFIG_X86_32
 
+#define STACK_RND_MASK_MODE(native) (0x7ff)
 #define STACK_RND_MASK (0x7ff)
 
 #define ARCH_DLINFO		ARCH_DLINFO_IA32
@@ -295,7 +296,8 @@ do {									\
 #else /* CONFIG_X86_32 */
 
 /* 1GB for 64bit, 8MB for 32bit */
-#define STACK_RND_MASK (test_thread_flag(TIF_ADDR32) ? 0x7ff : 0x3fffff)
+#define STACK_RND_MASK_MODE(native) ((native) ? 0x3fffff : 0x7ff)
+#define STACK_RND_MASK STACK_RND_MASK_MODE(!test_thread_flag(TIF_ADDR32))
 
 #define ARCH_DLINFO							\
 do {									\
@@ -320,7 +322,7 @@ if (test_thread_flag(TIF_X32))						\
 else									\
 	ARCH_DLINFO_IA32
 
-#define COMPAT_ELF_ET_DYN_BASE	(TASK_UNMAPPED_BASE + 0x1000000)
+#define COMPAT_ELF_ET_DYN_BASE	(TASK_UNMAPPED_BASE(TASK_SIZE) + 0x1000000)
 
 #endif /* !CONFIG_X86_32 */
 
@@ -353,6 +355,9 @@ static inline int mmap_is_ia32(void)
 extern unsigned long arch_compat_rnd(void);
 #endif
 extern unsigned long arch_native_rnd(void);
+extern unsigned long mmap_base(unsigned long rnd, unsigned long task_size);
+extern unsigned long mmap_legacy_base(unsigned long rnd,
+					unsigned long task_size);
 
 /* Do not change the values. See get_align_mask() */
 enum align_flags {
arch/x86/include/asm/processor.h +1 -1
index eaf100508c36..2bf5787fac37 100644
@@ -844,7 +844,7 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
  * This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
  */
-#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 3))
+#define TASK_UNMAPPED_BASE(task_size)	(PAGE_ALIGN(task_size / 3))
 
 #define KSTK_EIP(task)		(task_pt_regs(task)->ip)
 
arch/x86/mm/hugetlbpage.c +1 -1
index 2ae8584b44c7..62dfa3fa3ee0 100644
@@ -113,7 +113,7 @@ static unsigned long hugetlb_get_unmapped_area_topdown(struct file *file,
 	if (addr & ~PAGE_MASK) {
 		VM_BUG_ON(addr != -ENOMEM);
 		info.flags = 0;
-		info.low_limit = TASK_UNMAPPED_BASE;
+		info.low_limit = TASK_UNMAPPED_BASE(TASK_SIZE);
 		info.high_limit = TASK_SIZE;
 		addr = vm_unmapped_area(&info);
 	}
arch/x86/mm/mmap.c +19 -12
index 0b2007b08194..b64362270165 100644
@@ -35,12 +35,14 @@ struct va_alignment __read_mostly va_align = {
 	.flags = -1,
 };
 
-static unsigned long stack_maxrandom_size(void)
+static unsigned long stack_maxrandom_size(unsigned long task_size)
 {
 	unsigned long max = 0;
 	if ((current->flags & PF_RANDOMIZE) &&
 		!(current->personality & ADDR_NO_RANDOMIZE)) {
-		max = ((-1UL) & STACK_RND_MASK) << PAGE_SHIFT;
+		max = (-1UL);
+		max &= STACK_RND_MASK_MODE(task_size == TASK_SIZE_MAX);
+		max <<= PAGE_SHIFT;
 	}
 
 	return max;
@@ -51,8 +53,8 @@ static unsigned long stack_maxrandom_size(void)
  *
  * Leave an at least ~128 MB hole with possible stack randomization.
  */
-#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
-#define MAX_GAP (TASK_SIZE/6*5)
+#define MIN_GAP(task_size) (128*1024*1024UL + stack_maxrandom_size(task_size))
+#define MAX_GAP(task_size) (task_size/6*5)
 
 static int mmap_is_legacy(void)
 {
@@ -88,16 +90,21 @@ unsigned long arch_mmap_rnd(void)
 	return arch_native_rnd();
 }
 
-static unsigned long mmap_base(unsigned long rnd)
+unsigned long mmap_base(unsigned long rnd, unsigned long task_size)
 {
 	unsigned long gap = rlimit(RLIMIT_STACK);
 
-	if (gap < MIN_GAP)
-		gap = MIN_GAP;
-	else if (gap > MAX_GAP)
-		gap = MAX_GAP;
+	if (gap < MIN_GAP(task_size))
+		gap = MIN_GAP(task_size);
+	else if (gap > MAX_GAP(task_size))
+		gap = MAX_GAP(task_size);
 
-	return PAGE_ALIGN(TASK_SIZE - gap - rnd);
+	return PAGE_ALIGN(task_size - gap - rnd);
+}
+
+unsigned long mmap_legacy_base(unsigned long rnd, unsigned long task_size)
+{
+	return TASK_UNMAPPED_BASE(task_size) + rnd;
 }
 
 /*
@@ -111,13 +118,13 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 	if (current->flags & PF_RANDOMIZE)
 		random_factor = arch_mmap_rnd();
 
-	mm->mmap_legacy_base = TASK_UNMAPPED_BASE + random_factor;
+	mm->mmap_legacy_base = mmap_legacy_base(random_factor, TASK_SIZE);
 
 	if (mmap_is_legacy()) {
 		mm->mmap_base = mm->mmap_legacy_base;
 		mm->get_unmapped_area = arch_get_unmapped_area;
 	} else {
-		mm->mmap_base = mmap_base(random_factor);
+		mm->mmap_base = mmap_base(random_factor, TASK_SIZE);
 		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
 	}
 }
Signature · 7 lines
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#3 lore raw

Re: [PATCHv2 2/5] x86/mm: introduce mmap_{,legacy}_base

url: https://github.com/0day-ci/linux/commits/Dmitry-Safonov/Fix-compatible-mmap-return-pointer-over-4Gb/20170116-204523 config: i386-randconfig-i1-201703 (attached as .config) compiler: gcc-4.8…

Hi Dmitry,

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.10-rc4 next-20170116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Dmitry-Safonov/Fix-compatible-mmap-return-pointer-over-4Gb/20170116-204523
config: i386-randconfig-i1-201703 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4 reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

mm/mmap.c: In function 'arch_get_unmapped_area_topdown':

>> mm/mmap.c:2039:20: error: 'TASK_UNMAPPED_BASE' undeclared (first use in this function)
      info.low_limit = TASK_UNMAPPED_BASE;
                       ^

mm/mmap.c:2039:20: note: each undeclared identifier is reported only once for each function it appears in

vim +/TASK_UNMAPPED_BASE +2039 mm/mmap.c

^1da177e Linus Torvalds     2005-04-16  2033  	 * can happen with large stack limits and large mmap()
^1da177e Linus Torvalds     2005-04-16  2034  	 * allocations.
^1da177e Linus Torvalds     2005-04-16  2035  	 */
de1741a1 Alexander Kuleshov 2015-11-05  2036  	if (offset_in_page(addr)) {
db4fbfb9 Michel Lespinasse  2012-12-11  2037  		VM_BUG_ON(addr != -ENOMEM);
db4fbfb9 Michel Lespinasse  2012-12-11  2038  		info.flags = 0;
db4fbfb9 Michel Lespinasse  2012-12-11 @2039  		info.low_limit = TASK_UNMAPPED_BASE;
db4fbfb9 Michel Lespinasse  2012-12-11  2040  		info.high_limit = TASK_SIZE;
db4fbfb9 Michel Lespinasse  2012-12-11  2041  		addr = vm_unmapped_area(&info);
db4fbfb9 Michel Lespinasse  2012-12-11  2042  	}

:::::: The code at line 2039 was first introduced by commit
:::::: db4fbfb9523c93583c339e66023506f651c1d54b mm: vm_unmapped_area() lookup function

:::::: TO: Michel Lespinasse <[email protected]> :::::: CC: Linus Torvalds <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#4 lore raw

Re: [PATCHv2 2/5] x86/mm: introduce mmap_{,legacy}_base

In reply to Dmitry Safonov
Quoted 4 lines · Dmitry Safonov

In the following patch they will be used to compute:
- mmap_base in compat sys_mmap() in native 64-bit binary and vice-versa
- mmap_base for native sys_mmap() in compat x32/ia32-bit binary.

I may be wrong here, but I suspect that you're repeating something that I consider to be a mistake that's all over the x86 code. Specifically, you're distinguishing "native" from "compat" instead of "32-bit" from "64-bit". If you did the latter, then you wouldn't need the "native" case to work differently on 32-bit kernels vs 64-bit kernels, I think. Would making this change make your code simpler?

The x86 signal code is the worst offender IMO.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw

Re: [PATCHv2 2/5] x86/mm: introduce mmap_{,legacy}_base

Quoted 6 lines · Andy Lutomirski
Quoted 4 lines · Dmitry Safonov

In the following patch they will be used to compute:
- mmap_base in compat sys_mmap() in native 64-bit binary and vice-versa
- mmap_base for native sys_mmap() in compat x32/ia32-bit binary.

Quoted 8 lines

I may be wrong here, but I suspect that you're repeating something that I consider to be a mistake that's all over the x86 code. Specifically, you're distinguishing "native" from "compat" instead of "32-bit" from "64-bit". If you did the latter, then you wouldn't need the "native" case to work differently on 32-bit kernels vs 64-bit kernels, I think. Would making this change make your code simpler?

The x86 signal code is the worst offender IMO.

Yes, I also don't like to differ them especially by TIF_ADDR32 flag. I did distinguishing for the reason that I needed to know for which task 64/32-bit was computed mm->mmap_base.
Otherwise I could introduce mm->mmap_compat_base and don't differ tasks by TIF_ADDR32 flag - only by in_compat_syscall(), but that would change mm_struct generic code (adding a field to mm). So, I thought it may have more opposition to add a field to mm in generic code and fixed it here, in x86.

Quoted 3 lines

--Andy

Signature · 7 lines
             Dmitry

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#6 lore raw

PATCH v2 3/5 x86/mm: fix native mmap() in compat bins and vice-versa

In reply to Dmitry Safonov

Fix 32-bit compat_sys_mmap() mapping VMA over 4Gb in 64-bit binaries and 64-bit sys_mmap() mapping VMA only under 4Gb in 32-bit binaries. Changed arch_get_unmapped_area{,_topdown}() to recompute mmap_base for those cases and use according high/low limits for vm_unmapped_area() The recomputing of mmap_base may make compat sys_mmap() in 64-bit binaries a little slower than native, which uses already known from exec time mmap_base - but, as it returned buggy address, that case seemed unused previously, so no performance degradation for already used ABI. Can be optimized in future by introducing mmap_compat_{,legacy}_base in mm_struct.

I discovered that bug on ZDTM tests for compat 32-bit C/R. Working compat sys_mmap() in 64-bit binaries is really needed for that purpose, as 32-bit applications are restored from 64-bit CRIU binary.

---
 arch/x86/kernel/sys_x86_64.c | 44 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 3 deletions(-)
arch/x86/kernel/sys_x86_64.c +41 -3
index a55ed63b9f91..1bf90cd1400c 100644
@@ -113,10 +113,31 @@ static void find_start_end(unsigned long flags, unsigned long *begin,
 		if (current->flags & PF_RANDOMIZE) {
 			*begin = randomize_page(*begin, 0x02000000);
 		}
+		return;
+	}
+
+	if (!test_thread_flag(TIF_ADDR32)) {
+#ifdef CONFIG_COMPAT
+		/* 64-bit native binary doing compat 32-bit syscall */
+		if (in_compat_syscall()) {
+			*begin = mmap_legacy_base(arch_compat_rnd(),
+						IA32_PAGE_OFFSET);
+			*end = IA32_PAGE_OFFSET;
+			return;
+		}
+#endif
 	} else {
-		*begin = current->mm->mmap_legacy_base;
-		*end = TASK_SIZE;
+		/* 32-bit binary doing 64-bit syscall */
+		if (!in_compat_syscall()) {
+			*begin = mmap_legacy_base(arch_native_rnd(),
+						IA32_PAGE_OFFSET);
+			*end = TASK_SIZE_MAX;
+			return;
+		}
 	}
+
+	*begin = current->mm->mmap_legacy_base;
+	*end = TASK_SIZE;
 }
 
 unsigned long
@@ -157,6 +178,23 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	return vm_unmapped_area(&info);
 }
 
+static unsigned long find_top(void)
+{
+	if (!test_thread_flag(TIF_ADDR32)) {
+#ifdef CONFIG_COMPAT
+		/* 64-bit native binary doing compat 32-bit syscall */
+		if (in_compat_syscall())
+			return mmap_base(arch_compat_rnd(), IA32_PAGE_OFFSET);
+#endif
+	} else {
+		/* 32-bit binary doing 64-bit syscall */
+		if (!in_compat_syscall())
+			return mmap_base(arch_native_rnd(), TASK_SIZE_MAX);
+	}
+
+	return current->mm->mmap_base;
+}
+
 unsigned long
 arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 			  const unsigned long len, const unsigned long pgoff,
@@ -190,7 +228,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
 	info.length = len;
 	info.low_limit = PAGE_SIZE;
-	info.high_limit = mm->mmap_base;
+	info.high_limit = find_top();
 	info.align_mask = 0;
 	info.align_offset = pgoff << PAGE_SHIFT;
 	if (filp) {
Signature · 7 lines
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#7 lore raw

Re: [PATCHv2 3/5] x86/mm: fix native mmap() in compat bins and vice-versa

url: https://github.com/0day-ci/linux/commits/Dmitry-Safonov/Fix-compatible-mmap-return-pointer-over-4Gb/20170116-204523 config: x86_64-randconfig-x001-201703 (attached as .config) compiler: gcc-6…

Hi Dmitry,

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.10-rc4 next-20170116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Dmitry-Safonov/Fix-compatible-mmap-return-pointer-over-4Gb/20170116-204523
config: x86_64-randconfig-x001-201703 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

arch/x86/kernel/sys_x86_64.c: In function 'find_start_end':

>> arch/x86/kernel/sys_x86_64.c:122:7: error: implicit declaration of function 'in_compat_syscall' [-Werror=implicit-function-declaration]
      if (in_compat_syscall()) {
          ^~~~~~~~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/in_compat_syscall +122 arch/x86/kernel/sys_x86_64.c

116 return;
117 }
118
119 if (!test_thread_flag(TIF_ADDR32)) {
120 #ifdef CONFIG_COMPAT
121 /* 64-bit native binary doing compat 32-bit syscall */

 > 122			if (in_compat_syscall()) {

123 *begin = mmap_legacy_base(arch_compat_rnd(),
124 IA32_PAGE_OFFSET);
125 *end = IA32_PAGE_OFFSET;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#8 lore raw

Re: [PATCHv2 3/5] x86/mm: fix native mmap() in compat bins and vice-versa

In reply to Dmitry Safonov
url: https://github.com/0day-ci/linux/commits/Dmitry-Safonov/Fix-compatible-mmap-return-pointer-over-4Gb/20170116-204523 config: x86_64-randconfig-ne0-01162147 (attached as .config) compiler: gcc-6…

Hi Dmitry,

[auto build test WARNING on tip/x86/core]
[also build test WARNING on v4.10-rc4 next-20170116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Dmitry-Safonov/Fix-compatible-mmap-return-pointer-over-4Gb/20170116-204523
config: x86_64-randconfig-ne0-01162147 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

In file included from include/uapi/linux/stddef.h:1:0,

                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/uapi/linux/capability.h:16,
                    from include/linux/capability.h:15,
                    from include/linux/sched.h:15,
                    from arch/x86/kernel/sys_x86_64.c:2:
   arch/x86/kernel/sys_x86_64.c: In function 'find_start_end':
   arch/x86/kernel/sys_x86_64.c:131:8: error: implicit declaration of function 'in_compat_syscall' [-Werror=implicit-function-declaration]
      if (!in_compat_syscall()) {
           ^
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> arch/x86/kernel/sys_x86_64.c:131:3: note: in expansion of macro 'if'
      if (!in_compat_syscall()) {
      ^~

cc1: some warnings being treated as errors

vim +/if +131 arch/x86/kernel/sys_x86_64.c

     1	#include <linux/errno.h>
   > 2	#include <linux/sched.h>
     3	#include <linux/syscalls.h>
     4	#include <linux/mm.h>
     5	#include <linux/fs.h>
     6	#include <linux/smp.h>
     7	#include <linux/sem.h>
     8	#include <linux/msg.h>
     9	#include <linux/shm.h>
    10	#include <linux/stat.h>
    11	#include <linux/mman.h>
    12	#include <linux/file.h>
    13	#include <linux/utsname.h>
    14	#include <linux/personality.h>
    15	#include <linux/random.h>
    16	#include <linux/uaccess.h>
    17	#include <linux/elf.h>
    18	
    19	#include <asm/ia32.h>
    20	#include <asm/syscalls.h>
    21	
    22	/*
    23	 * Align a virtual address to avoid aliasing in the I$ on AMD F15h.
    24	 */
    25	static unsigned long get_align_mask(void)
    26	{
    27		/* handle 32- and 64-bit case with a single conditional */
    28		if (va_align.flags < 0 || !(va_align.flags & (2 - mmap_is_ia32())))
    29			return 0;
    30	
    31		if (!(current->flags & PF_RANDOMIZE))
    32			return 0;
    33	
    34		return va_align.mask;
    35	}
    36	
    37	/*
    38	 * To avoid aliasing in the I$ on AMD F15h, the bits defined by the
    39	 * va_align.bits, [12:upper_bit), are set to a random value instead of
    40	 * zeroing them. This random value is computed once per boot. This form
    41	 * of ASLR is known as "per-boot ASLR".
    42	 *
    43	 * To achieve this, the random value is added to the info.align_offset
    44	 * value before calling vm_unmapped_area() or ORed directly to the
    45	 * address.
    46	 */
    47	static unsigned long get_align_bits(void)
    48	{
    49		return va_align.bits & get_align_mask();
    50	}
    51	
    52	unsigned long align_vdso_addr(unsigned long addr)
    53	{
    54		unsigned long align_mask = get_align_mask();
    55		addr = (addr + align_mask) & ~align_mask;
    56		return addr | get_align_bits();
    57	}
    58	
    59	static int __init control_va_addr_alignment(char *str)
    60	{
    61		/* guard against enabling this on other CPU families */
    62		if (va_align.flags < 0)
    63			return 1;
    64	
    65		if (*str == 0)
    66			return 1;
    67	
    68		if (*str == '=')
    69			str++;
    70	
    71		if (!strcmp(str, "32"))
    72			va_align.flags = ALIGN_VA_32;
    73		else if (!strcmp(str, "64"))
    74			va_align.flags = ALIGN_VA_64;
    75		else if (!strcmp(str, "off"))
    76			va_align.flags = 0;
    77		else if (!strcmp(str, "on"))
    78			va_align.flags = ALIGN_VA_32 | ALIGN_VA_64;
    79		else
    80			return 0;
    81	
    82		return 1;
    83	}
    84	__setup("align_va_addr", control_va_addr_alignment);
    85	
    86	SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
    87			unsigned long, prot, unsigned long, flags,
    88			unsigned long, fd, unsigned long, off)
    89	{
    90		long error;
    91		error = -EINVAL;
    92		if (off & ~PAGE_MASK)
    93			goto out;
    94	
    95		error = sys_mmap_pgoff(addr, len, prot, flags, fd, off >> PAGE_SHIFT);
    96	out:
    97		return error;
    98	}
    99	
   100	static void find_start_end(unsigned long flags, unsigned long *begin,
   101				   unsigned long *end)
   102	{
   103		if (!test_thread_flag(TIF_ADDR32) && (flags & MAP_32BIT)) {
   104			/* This is usually used needed to map code in small
   105			   model, so it needs to be in the first 31bit. Limit
   106			   it to that.  This means we need to move the
   107			   unmapped base down for this case. This can give
   108			   conflicts with the heap, but we assume that glibc
   109			   malloc knows how to fall back to mmap. Give it 1GB
   110			   of playground for now. -AK */
   111			*begin = 0x40000000;
   112			*end = 0x80000000;
   113			if (current->flags & PF_RANDOMIZE) {
   114				*begin = randomize_page(*begin, 0x02000000);
   115			}
   116			return;
   117		}
   118	
   119		if (!test_thread_flag(TIF_ADDR32)) {
   120	#ifdef CONFIG_COMPAT
   121			/* 64-bit native binary doing compat 32-bit syscall */
   122			if (in_compat_syscall()) {
   123				*begin = mmap_legacy_base(arch_compat_rnd(),
   124							IA32_PAGE_OFFSET);
   125				*end = IA32_PAGE_OFFSET;
   126				return;
   127			}
   128	#endif
   129		} else {
   130			/* 32-bit binary doing 64-bit syscall */
 > 131			if (!in_compat_syscall()) {

132 *begin = mmap_legacy_base(arch_native_rnd(),
133 IA32_PAGE_OFFSET);
134 *end = TASK_SIZE_MAX;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#9 lore raw

Re: [PATCHv2 3/5] x86/mm: fix native mmap() in compat bins and vice-versa

In reply to Dmitry Safonov
Quoted 8 lines · Dmitry Safonov

Fix 32-bit compat_sys_mmap() mapping VMA over 4Gb in 64-bit binaries and 64-bit sys_mmap() mapping VMA only under 4Gb in 32-bit binaries. Changed arch_get_unmapped_area{,_topdown}() to recompute mmap_base for those cases and use according high/low limits for vm_unmapped_area() The recomputing of mmap_base may make compat sys_mmap() in 64-bit binaries a little slower than native, which uses already known from exec time mmap_base - but, as it returned buggy address, that case seemed unused previously, so no performance degradation for already used ABI.

This looks plausibly correct but rather weird -- why does this code need to distinguish between all four cases (pure 32-bit, pure 64-bit, 64-bit mmap layout doing 32-bit call, 32-bit layout doing 64-bit call)?

Quoted 2 lines

Can be optimized in future by introducing mmap_compat_{,legacy}_base in mm_struct.

Hmm. Would it make sense to do it this way from the beginning?

If adding an in_32bit_syscall() helper would help, then by all means please do so.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#10 lore raw

Re: [PATCHv2 3/5] x86/mm: fix native mmap() in compat bins and vice-versa

Quoted 10 lines · Andy Lutomirski
Quoted 8 lines · Dmitry Safonov

Fix 32-bit compat_sys_mmap() mapping VMA over 4Gb in 64-bit binaries and 64-bit sys_mmap() mapping VMA only under 4Gb in 32-bit binaries. Changed arch_get_unmapped_area{,_topdown}() to recompute mmap_base for those cases and use according high/low limits for vm_unmapped_area() The recomputing of mmap_base may make compat sys_mmap() in 64-bit binaries a little slower than native, which uses already known from exec time mmap_base - but, as it returned buggy address, that case seemed unused previously, so no performance degradation for already used ABI.

Quoted 4 lines

This looks plausibly correct but rather weird -- why does this code need to distinguish between all four cases (pure 32-bit, pure 64-bit, 64-bit mmap layout doing 32-bit call, 32-bit layout doing 64-bit call)?

Only by need to know is mm->mmap_base computed initialy for 32-bit or for 64-bit.

Quoted 5 lines
Quoted 3 lines

Can be optimized in future by introducing mmap_compat_{,legacy}_base in mm_struct.

Hmm. Would it make sense to do it this way from the beginning?

That would, but mm_struct is in generic code, if adding those new bases is fine, than I'll do that in v3.

It will look somehow like:
: if (in_compat_syscall())
: return current->mm->mmap_compat_base;
: else
: return current->mm->mmap_base;

Quoted 6 lines

If adding an in_32bit_syscall() helper would help, then by all means please do so.

--Andy

Signature · 7 lines
             Dmitry

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#11 lore raw

PATCH v2 4/5 x86/mm: for MAP_32BIT check in_compat_syscall() instead TIF_ADDR32

In reply to Dmitry Safonov

At this momet, logic in arch_get_unmapped_area{,_topdown} for mmaps with MAP_32BIT flag checks TIF_ADDR32 which means:
o if 32-bit ELF changes mode to 64-bit on x86_64 and then tries to mmap() with MAP_32BIT it'll result in addr over 4Gb (as default is top-down allocation)
o if 64-bit ELF changes mode to 32-bit and tries mmap() with MAP_32BIT, it'll allocate only memory in 1GB space: [0x40000000, 0x80000000).

Fix it by handeling MAP_32BIT in 64-bit syscalls only. As a little bonus it'll make thread flag a little less used.

---
 arch/x86/kernel/sys_x86_64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/sys_x86_64.c +2 -2
index 1bf90cd1400c..e7d4bbbe6175 100644
@@ -100,7 +100,7 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
 static void find_start_end(unsigned long flags, unsigned long *begin,
 			   unsigned long *end)
 {
-	if (!test_thread_flag(TIF_ADDR32) && (flags & MAP_32BIT)) {
+	if (!in_compat_syscall() && (flags & MAP_32BIT)) {
 		/* This is usually used needed to map code in small
 		   model, so it needs to be in the first 31bit. Limit
 		   it to that.  This means we need to move the
@@ -213,7 +213,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 		return addr;
 
 	/* for MAP_32BIT mappings we force the legacy mmap base */
-	if (!test_thread_flag(TIF_ADDR32) && (flags & MAP_32BIT))
+	if (!in_compat_syscall() && (flags & MAP_32BIT))
 		goto bottomup;
 
 	/* requesting a specific address */
Signature · 7 lines
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#12 lore raw

Re: [PATCHv2 4/5] x86/mm: for MAP_32BIT check in_compat_syscall() instead TIF_ADDR32

Quoted 10 lines · Dmitry Safonov

At this momet, logic in arch_get_unmapped_area{,_topdown} for mmaps with MAP_32BIT flag checks TIF_ADDR32 which means:
o if 32-bit ELF changes mode to 64-bit on x86_64 and then tries to mmap() with MAP_32BIT it'll result in addr over 4Gb (as default is top-down allocation)
o if 64-bit ELF changes mode to 32-bit and tries mmap() with MAP_32BIT, it'll allocate only memory in 1GB space: [0x40000000, 0x80000000).

Fix it by handeling MAP_32BIT in 64-bit syscalls only. As a little bonus it'll make thread flag a little less used.

Seems like an improvement. Also, jeez, the mmap code is complicated.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#13 lore raw

Re: [PATCHv2 4/5] x86/mm: for MAP_32BIT check in_compat_syscall() instead TIF_ADDR32

Quoted 9 lines · Andy Lutomirski
Quoted 8 lines · Dmitry Safonov

At this momet, logic in arch_get_unmapped_area{,_topdown} for mmaps with MAP_32BIT flag checks TIF_ADDR32 which means:
o if 32-bit ELF changes mode to 64-bit on x86_64 and then tries to mmap() with MAP_32BIT it'll result in addr over 4Gb (as default is top-down allocation)
o if 64-bit ELF changes mode to 32-bit and tries mmap() with MAP_32BIT, it'll allocate only memory in 1GB space: [0x40000000, 0x80000000).

Quoted 4 lines
Quoted 2 lines

Fix it by handeling MAP_32BIT in 64-bit syscalls only. As a little bonus it'll make thread flag a little less used.

Seems like an improvement. Also, jeez, the mmap code is complicated.

Yes, but it's intentionally 4 of 5 by the reason, that it'll broke the second case:

Quoted 2 lines
Quoted 2 lines

o if 64-bit ELF changes mode to 32-bit and tries mmap() with MAP_32BIT, it'll allocate only memory in 1GB space: [0x40000000, 0x80000000).

if the first 3 patches has not applied earlier.
(because it'll allocate not in 1Gb space, but in 64-bit whole space, above 4Gb).
So, this should go only after fixing compat mmap in 64-bit binaries, which is done in prev 3 patches.

Signature · 7 lines
             Dmitry

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#14 lore raw

PATCH v2 5/5 selftests/x86: add test to check compat mmap() return addr

In reply to Dmitry Safonov

We can't just add segfault handler and use addr, returned by compat mmap() syscall, because the lower 4 bytes can be the same as already existed VMA. So, the test parses /proc/self/maps file, founds new VMAs those appeared after compatible sys_mmap() and checks if mmaped VMA is in that list.

On failure it prints:
[NOTE] Allocated mmap 0x6f36a000, sized 0x400000
[NOTE] New mapping appeared: 0x7f936f36a000
[FAIL] Found VMA [0x7f936f36a000, 0x7f936f76a000] in maps file, that was allocated with compat syscall

---
 tools/testing/selftests/x86/Makefile           |   2 +-
 tools/testing/selftests/x86/test_compat_mmap.c | 208 +++++++++++++++++++++++++
 2 files changed, 209 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/test_compat_mmap.c
tools/testing/selftests/x86/Makefile +1 -1
index 8c1cb423cfe6..9c3e746a6064 100644
@@ -10,7 +10,7 @@ TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_sysc
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			vdso_restorer
-TARGETS_C_64BIT_ONLY := fsgsbase
+TARGETS_C_64BIT_ONLY := fsgsbase test_compat_mmap
 
 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
 TARGETS_C_64BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_64BIT_ONLY)
tools/testing/selftests/x86/test_compat_mmap.c new file +208 -0

208 lines not on this page · raw message

Signature · 7 lines
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
maintainer
#15 lore raw
In reply to Dmitry Safonov

Kirill, can you take a careful look at this series? I bet you're going to run into this stuff too.

Quoted 50 lines · Dmitry Safonov

Changes since v1:
- Recalculate mmap_base instead of using max possible virtual address for compat/native syscall. That will make policy for allocation the same in 32-bit binaries and in 32-bit syscalls in 64-bit binaries. I need this because sys_mmap() in restored 32-bit process shouldn't hit the stack area.
- Fixed mmap() with MAP_32BIT flag in the same usecases
- used in_compat_syscall() helper rather TS_COMPAT check (Andy noticed)
- introduced find_top() helper as suggested by Andy to simplify code
- fixed test error-handeling: it checked the result of sys_mmap() with MMAP_FAILED, which is not correct, as it calls raw syscall - now checks return value to be aligned to PAGE_SIZE.

Description from v1 [2]:

A fix for bug in mmap() that I referenced in [1].
Also selftest for it.

I would like to mark the fix as for stable v4.9 kernel if it'll be accepted, as I try to support compatible 32-bit C/R after v4.9 and working compatible mmap() is really wanted there.

[1]: https://marc.info/?l=linux-kernel&m=148311451525315
[2]: https://marc.info/?l=linux-kernel&m=148415888707662

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>

Dmitry Safonov (5):
x86/mm: split arch_mmap_rnd() on compat/native versions x86/mm: introduce mmap_{,legacy}_base
x86/mm: fix native mmap() in compat bins and vice-versa x86/mm: for MAP_32BIT check in_compat_syscall() instead TIF_ADDR32 selftests/x86: add test to check compat mmap() return addr

 arch/x86/include/asm/elf.h                     |  14 +-
 arch/x86/include/asm/processor.h               |   2 +-
 arch/x86/kernel/sys_x86_64.c                   |  48 +++++-
 arch/x86/mm/hugetlbpage.c                      |   2 +-
 arch/x86/mm/mmap.c                             |  56 ++++---
 tools/testing/selftests/x86/Makefile           |   2 +-
 tools/testing/selftests/x86/test_compat_mmap.c | 208 +++++++++++++++++++++++++
 7 files changed, 301 insertions(+), 31 deletions(-)
 create mode 100644 tools/testing/selftests/x86/test_compat_mmap.c
Quoted 3 lines

--
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>