ILP32 for ARM64

The only difference is that non-compat version forces O_LARGEFILE, and it should be the default behaviour for all architectures, as we don't support 32-bit off_t. The only exception is tile32, that continues with compat version of syscalls.

Signed-off-by Yury Norov <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Acked-by Chris Metcalf <[email protected]> [for tile]

---
 arch/tile/kernel/compat.c | 3 +++
 1 file changed, 3 insertions(+)

arch/tile/kernel/compat.c +0 -0

index 4912084..489ae19 100644

Quoted 20 lines · Yury Norov

The only difference is that non-compat version forces O_LARGEFILE, and it should be the default behaviour for all architectures, as we don't support 32-bit off_t. The only exception is tile32, that continues with compat version of syscalls.

Signed-off-by Yury Norov <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Acked-by Chris Metcalf <[email protected]> [for tile]

---
 arch/tile/kernel/compat.c | 3 +++
 1 file changed, 3 insertions(+)

arch/tile/kernel/compat.c +0 -0

index 4912084..489ae19 100644
@@ -94,6 +94,9 @@ COMPAT_SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned int, offset_high,
 #define compat_sys_readahead sys32_readahead
 #define sys_llseek compat_sys_llseek
 

arch/tile/kernel/compat.c COMPAT_SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned int, offset_high, · line 102

Quoted 6 lines

arch/tile/kernel/compat.c +3 -0

+#define sys_openat		compat_sys_openat
+#define sys_open_by_handle_at	compat_sys_open_by_handle_at
+
 /* Call the assembly trampolines where necessary. */
 #define compat_sys_rt_sigreturn _compat_sys_rt_sigreturn
 #define sys_clone _sys_clone

This is a no-op. Did you mean to add this? Without that the testsuite of tar fails on ILP32.

include/uapi/asm-generic/unistd.h +2 -3

index a26415b..4dcc38d 100644
@@ -178,7 +178,7 @@ __SYSCALL(__NR_fchownat, sys_fchownat)
 #define __NR_fchown 55
 __SYSCALL(__NR_fchown, sys_fchown)
 #define __NR_openat 56
-__SC_COMP(__NR_openat, sys_openat, compat_sys_openat)
+__SYSCALL(__NR_openat, sys_openat)
 #define __NR_close 57
 __SYSCALL(__NR_close, sys_close)
 #define __NR_vhangup 58

@@ -676,8 +676,7 @@ __SYSCALL(__NR_fanotify_mark, sys_fanotify_mark)
 #define __NR_name_to_handle_at         264
 __SYSCALL(__NR_name_to_handle_at, sys_name_to_handle_at)
 #define __NR_open_by_handle_at         265
-__SC_COMP(__NR_open_by_handle_at, sys_open_by_handle_at, \
-	  compat_sys_open_by_handle_at)
+__SYSCALL(__NR_open_by_handle_at, sys_open_by_handle_at)
 #define __NR_clock_adjtime 266
 __SC_COMP(__NR_clock_adjtime, sys_clock_adjtime, compat_sys_clock_adjtime)
 #define __NR_syncfs 267

Signature · 9 lines

2.9.0


Andreas.

-- 
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Quoted 22 lines · Andreas Schwab

Quoted 20 lines · Yury Norov

The only difference is that non-compat version forces O_LARGEFILE, and it should be the default behaviour for all architectures, as we don't support 32-bit off_t. The only exception is tile32, that continues with compat version of syscalls.

Signed-off-by Yury Norov <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Acked-by Chris Metcalf <[email protected]> [for tile]

---
 arch/tile/kernel/compat.c | 3 +++
 1 file changed, 3 insertions(+)

arch/tile/kernel/compat.c +0 -0

index 4912084..489ae19 100644
@@ -94,6 +94,9 @@ COMPAT_SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned int, offset_high,
 #define compat_sys_readahead sys32_readahead
 #define sys_llseek compat_sys_llseek
 

Quoted 9 lines

arch/tile/kernel/compat.c COMPAT_SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned int, offset_high, · line 102

Quoted 6 lines

arch/tile/kernel/compat.c +3 -0

+#define sys_openat		compat_sys_openat
+#define sys_open_by_handle_at	compat_sys_open_by_handle_at
+
 /* Call the assembly trampolines where necessary. */
 #define compat_sys_rt_sigreturn _compat_sys_rt_sigreturn
 #define sys_clone _sys_clone

This is a no-op. Did you mean to add this? Without that the testsuite of tar fails on ILP32.

The change you suggest below is what I originally sent at the end of December, but that chunk was lost after one of numerous rebases probably.

http://lkml.iu.edu/hypermail/linux/kernel/1512.3/00510.html

IIRC, the only difference between this couple of syscalls and their compat versions is that non-compat syscalls set O_LARGEFILE depending on (BITS_PER_LONG != 32). Next patch of this series introduces ARCH_32BIT_OFF_T config option to force O_LARGEFILE for 64-bit arches, and all new 32-bit arches, as ARCH_32BIT_OFF_T is enabled for existing 32-bit arches explicitly.

O_LARGEFILE is controlled like this:

        -#define force_o_largefile() (BITS_PER_LONG != 32)
        +#define force_o_largefile() (!IS_ENABLED(CONFIG_ARCH_32BIT_OFF_T))

So if I restore original patch, everything will work. But my question
is: if ARCH_32BIT_OFF_T disables O_LARGEFILE for tile32 anyway, what for we redirect native syscall handlers to compat ones? They are looking identical...

If my understanding is correct, we can drop the change in arch/tile/kernel/compat.c and join this patch with next one in this series.

Chris, Arnd?

Quoted 23 lines

include/uapi/asm-generic/unistd.h +2 -3

index a26415b..4dcc38d 100644
@@ -178,7 +178,7 @@ __SYSCALL(__NR_fchownat, sys_fchownat)
 #define __NR_fchown 55
 __SYSCALL(__NR_fchown, sys_fchown)
 #define __NR_openat 56
-__SC_COMP(__NR_openat, sys_openat, compat_sys_openat)
+__SYSCALL(__NR_openat, sys_openat)
 #define __NR_close 57
 __SYSCALL(__NR_close, sys_close)
 #define __NR_vhangup 58

@@ -676,8 +676,7 @@ __SYSCALL(__NR_fanotify_mark, sys_fanotify_mark)
 #define __NR_name_to_handle_at         264
 __SYSCALL(__NR_name_to_handle_at, sys_name_to_handle_at)
 #define __NR_open_by_handle_at         265
-__SC_COMP(__NR_open_by_handle_at, sys_open_by_handle_at, \
-	  compat_sys_open_by_handle_at)
+__SYSCALL(__NR_open_by_handle_at, sys_open_by_handle_at)
 #define __NR_clock_adjtime 266
 __SC_COMP(__NR_clock_adjtime, sys_clock_adjtime, compat_sys_clock_adjtime)
 #define __NR_syncfs 267

Quoted 45 lines · CEST Yury Norov

Quoted 22 lines · Andreas Schwab

Quoted 20 lines · Yury Norov

The only difference is that non-compat version forces O_LARGEFILE, and it should be the default behaviour for all architectures, as we don't support 32-bit off_t. The only exception is tile32, that continues with compat version of syscalls.

Signed-off-by Yury Norov <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Acked-by Chris Metcalf <[email protected]> [for tile]

---
 arch/tile/kernel/compat.c | 3 +++
 1 file changed, 3 insertions(+)

arch/tile/kernel/compat.c +0 -0

index 4912084..489ae19 100644
@@ -94,6 +94,9 @@ COMPAT_SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned int, offset_high,
 #define compat_sys_readahead sys32_readahead
 #define sys_llseek compat_sys_llseek
 

Quoted 9 lines

arch/tile/kernel/compat.c COMPAT_SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned int, offset_high, · line 102

Quoted 6 lines

arch/tile/kernel/compat.c +3 -0

+#define sys_openat		compat_sys_openat
+#define sys_open_by_handle_at	compat_sys_open_by_handle_at
+
 /* Call the assembly trampolines where necessary. */
 #define compat_sys_rt_sigreturn _compat_sys_rt_sigreturn
 #define sys_clone _sys_clone

This is a no-op. Did you mean to add this? Without that the testsuite of tar fails on ILP32.

The change you suggest below is what I originally sent at the end of December, but that chunk was lost after one of numerous rebases probably.

http://lkml.iu.edu/hypermail/linux/kernel/1512.3/00510.html

IIRC, the only difference between this couple of syscalls and their compat versions is that non-compat syscalls set O_LARGEFILE depending on (BITS_PER_LONG != 32). Next patch of this series introduces ARCH_32BIT_OFF_T config option to force O_LARGEFILE for 64-bit arches, and all new 32-bit arches, as ARCH_32BIT_OFF_T is enabled for existing 32-bit arches explicitly.

Quoted 8 lines

O_LARGEFILE is controlled like this:

        -#define force_o_largefile() (BITS_PER_LONG != 32)
        +#define force_o_largefile() (!IS_ENABLED(CONFIG_ARCH_32BIT_OFF_T))

So if I restore original patch, everything will work. But my question
is: if ARCH_32BIT_OFF_T disables O_LARGEFILE for tile32 anyway, what for we redirect native syscall handlers to compat ones? They are looking identical...

I've lost context here, but let me try to recall what we had discussed in the past:

- native 32-bit architectures are handled using CONFIG_ARCH_32BIT_OFF_T
- native 64-bit architectures keep setting the flag.
- compat 32-bit architectures need special cases, but the only one that exists is tile, which has to keep not setting O_LARGEFILE while new compat architectures (ilp32) should set it.

Quoted 4 lines

If my understanding is correct, we can drop the change in arch/tile/kernel/compat.c and join this patch with next one in this series.

Quoted 1 line

Chris, Arnd?

The change in arch/tile/kernel/compat.c can either come before the below change, or in the same patch, but not later.

Arnd

Quoted 25 lines

Quoted 24 lines

include/uapi/asm-generic/unistd.h +2 -3

index a26415b..4dcc38d 100644
@@ -178,7 +178,7 @@ __SYSCALL(__NR_fchownat, sys_fchownat)
 #define __NR_fchown 55
 __SYSCALL(__NR_fchown, sys_fchown)
 #define __NR_openat 56
-__SC_COMP(__NR_openat, sys_openat, compat_sys_openat)
+__SYSCALL(__NR_openat, sys_openat)
 #define __NR_close 57
 __SYSCALL(__NR_close, sys_close)
 #define __NR_vhangup 58

@@ -676,8 +676,7 @@ __SYSCALL(__NR_fanotify_mark, sys_fanotify_mark)
 #define __NR_name_to_handle_at         264
 __SYSCALL(__NR_name_to_handle_at, sys_name_to_handle_at)
 #define __NR_open_by_handle_at         265
-__SC_COMP(__NR_open_by_handle_at, sys_open_by_handle_at, \
-	  compat_sys_open_by_handle_at)
+__SYSCALL(__NR_open_by_handle_at, sys_open_by_handle_at)
 #define __NR_clock_adjtime 266
 __SC_COMP(__NR_clock_adjtime, sys_clock_adjtime, compat_sys_clock_adjtime)
 #define __NR_syncfs 267

From: Andrew Pinski <[email protected]>

In this patchset ILP32 ABI support is added. Additionally to AARCH32, which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.

From now, AARCH32_EL0 (former COMPAT) config option means the support of
AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches), and COMPAT indicates that one of them, or both, is enabled.

Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead

Reviewed-by David Daney <[email protected]>

Signed-off-by Andrew Pinski <[email protected]>

Signed-off-by Philipp Tomsich <[email protected]>

Signed-off-by Christoph Muellner <[email protected]>

Signed-off-by Bamvor Jian Zhang <[email protected]>

Signed-off-by Yury Norov <[email protected]>

arch/arm64/kernel/cpuinfo.c +0 -0

index c173d32..af200a8 100644
@@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
  		 */
  		seq_puts(m, "Features\t:");
  		if (compat) {

arch/arm64/kernel/cpuinfo.c +9 -0

+				for (j = 0; compat_hwcap_str[j]; j++)
+					if (compat_elf_hwcap & (1 << j))
+						seq_printf(m, " %s", compat_hwcap_str[j]);
+
+				for (j = 0; compat_hwcap2_str[j]; j++)
+					if (compat_elf_hwcap2 & (1 << j))
+						seq_printf(m, " %s", compat_hwcap2_str[j]);
+			}
+#endif /* CONFIG_AARCH32_EL0 */
  		} else {
  			for (j = 0; hwcap_str[j]; j++)
  				if (elf_hwcap & (1 << j))

Quoted 17 lines · Yury Norov

From: Andrew Pinski <[email protected]>

In this patchset ILP32 ABI support is added. Additionally to AARCH32, which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.

From now, AARCH32_EL0 (former COMPAT) config option means the support of
AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches), and COMPAT indicates that one of them, or both, is enabled.

Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead

Reviewed-by David Daney <[email protected]>

Signed-off-by Andrew Pinski <[email protected]>

Signed-off-by Philipp Tomsich <[email protected]>

Signed-off-by Christoph Muellner <[email protected]>

Signed-off-by Bamvor Jian Zhang <[email protected]>

Signed-off-by Yury Norov <[email protected]>

...

Quoted 8 lines

arch/arm64/kernel/cpuinfo.c +0 -0

index c173d32..af200a8 100644
@@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
  		 */
  		seq_puts(m, "Features\t:");
  		if (compat) {

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 137

Quoted 10 lines

arch/arm64/kernel/cpuinfo.c +1 -9

-#ifdef CONFIG_COMPAT
-			for (j = 0; compat_hwcap_str[j]; j++)
-				if (compat_elf_hwcap & (1 << j))
-					seq_printf(m, " %s", compat_hwcap_str[j]);
-
-			for (j = 0; compat_hwcap2_str[j]; j++)
-				if (compat_elf_hwcap2 & (1 << j))
-					seq_printf(m, " %s", compat_hwcap2_str[j]);
-#endif /* CONFIG_COMPAT */
+#ifdef CONFIG_AARCH32_EL0

I saw that compat_hwcap_str and compat_hwcap2_str is defined when "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0 in c show()?

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 138

Quoted 1 line

arch/arm64/kernel/cpuinfo.c +1 -0

+			if (personality(current->personality) == PER_LINUX32) {

And "compat" is "personality(current->personality) == PER_LINUX32;", it seems that there is no need to add this twice.

Quoted 40 lines · CEST Zhangjian (Bamvor)

Quoted 17 lines · Yury Norov

From: Andrew Pinski <[email protected]>

In this patchset ILP32 ABI support is added. Additionally to AARCH32, which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.

From now, AARCH32_EL0 (former COMPAT) config option means the support of
AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches), and COMPAT indicates that one of them, or both, is enabled.

Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead

Reviewed-by David Daney <[email protected]>

Signed-off-by Andrew Pinski <[email protected]>

Signed-off-by Philipp Tomsich <[email protected]>

Signed-off-by Christoph Muellner <[email protected]>

Signed-off-by Bamvor Jian Zhang <[email protected]>

Signed-off-by Yury Norov <[email protected]>

...

Quoted 8 lines

arch/arm64/kernel/cpuinfo.c +0 -0

index c173d32..af200a8 100644
@@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
  		 */
  		seq_puts(m, "Features\t:");
  		if (compat) {

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 137

Quoted 10 lines

arch/arm64/kernel/cpuinfo.c +1 -9

-#ifdef CONFIG_COMPAT
-			for (j = 0; compat_hwcap_str[j]; j++)
-				if (compat_elf_hwcap & (1 << j))
-					seq_printf(m, " %s", compat_hwcap_str[j]);
-
-			for (j = 0; compat_hwcap2_str[j]; j++)
-				if (compat_elf_hwcap2 & (1 << j))
-					seq_printf(m, " %s", compat_hwcap2_str[j]);
-#endif /* CONFIG_COMPAT */
+#ifdef CONFIG_AARCH32_EL0

I saw that compat_hwcap_str and compat_hwcap2_str is defined when "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0 in c show()?

Quoted 3 lines

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 138

Quoted 1 line

arch/arm64/kernel/cpuinfo.c +1 -0

+			if (personality(current->personality) == PER_LINUX32) {

And "compat" is "personality(current->personality) == PER_LINUX32;", it seems that there is no need to add this twice.

I think it would be best to remove the #ifdef here completely, the PER_LINUX32 concept is not strictly tied to the emulation of ARM binaries, it literally just changes the output of /proc/cpuinfo and 'uname',

and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have arm64 binaries running with PER_LINUX32.

Quoted 41 lines · Arnd Bergmann

Quoted 40 lines · CEST Zhangjian (Bamvor)

Quoted 17 lines · Yury Norov

From: Andrew Pinski <[email protected]>

In this patchset ILP32 ABI support is added. Additionally to AARCH32, which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.

From now, AARCH32_EL0 (former COMPAT) config option means the support of
AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches), and COMPAT indicates that one of them, or both, is enabled.

Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead

Reviewed-by David Daney <[email protected]>

Signed-off-by Andrew Pinski <[email protected]>

Signed-off-by Philipp Tomsich <[email protected]>

Signed-off-by Christoph Muellner <[email protected]>

Signed-off-by Bamvor Jian Zhang <[email protected]>

Signed-off-by Yury Norov <[email protected]>

...

Quoted 8 lines

arch/arm64/kernel/cpuinfo.c +0 -0

index c173d32..af200a8 100644
@@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
  		 */
  		seq_puts(m, "Features\t:");
  		if (compat) {

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 137

Quoted 10 lines

arch/arm64/kernel/cpuinfo.c +1 -9

-#ifdef CONFIG_COMPAT
-			for (j = 0; compat_hwcap_str[j]; j++)
-				if (compat_elf_hwcap & (1 << j))
-					seq_printf(m, " %s", compat_hwcap_str[j]);
-
-			for (j = 0; compat_hwcap2_str[j]; j++)
-				if (compat_elf_hwcap2 & (1 << j))
-					seq_printf(m, " %s", compat_hwcap2_str[j]);
-#endif /* CONFIG_COMPAT */
+#ifdef CONFIG_AARCH32_EL0

I saw that compat_hwcap_str and compat_hwcap2_str is defined when "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0 in c show()?

Quoted 8 lines

Quoted 3 lines

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 138

Quoted 1 line

arch/arm64/kernel/cpuinfo.c +1 -0

+			if (personality(current->personality) == PER_LINUX32) {

And "compat" is "personality(current->personality) == PER_LINUX32;", it seems that there is no need to add this twice.

I think it would be best to remove the #ifdef here completely, the PER_LINUX32 concept is not strictly tied to the emulation of ARM binaries, it literally just changes the output of /proc/cpuinfo and 'uname',

It's not strictly related to ARM binaries, however it is related to AArch32 CPU features being supported and detected by the kernel. Currently, with CONFIG_COMPAT disabled, we won't have access to a (meaningful) compat_elf_hwcap.

Quoted 3 lines

and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas at arm.com

(you were summoned on that discussion couple of times ;))

Quoted 51 lines · CEST Catalin Marinas

Quoted 41 lines · Arnd Bergmann

Quoted 40 lines · CEST Zhangjian (Bamvor)

Quoted 17 lines · Yury Norov

From: Andrew Pinski <[email protected]>

In this patchset ILP32 ABI support is added. Additionally to AARCH32, which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.

From now, AARCH32_EL0 (former COMPAT) config option means the support of
AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches), and COMPAT indicates that one of them, or both, is enabled.

Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead

Reviewed-by David Daney <[email protected]>

Signed-off-by Andrew Pinski <[email protected]>

Signed-off-by Philipp Tomsich <[email protected]>

Signed-off-by Christoph Muellner <[email protected]>

Signed-off-by Bamvor Jian Zhang <[email protected]>

Signed-off-by Yury Norov <[email protected]>

...

Quoted 8 lines

arch/arm64/kernel/cpuinfo.c +0 -0

index c173d32..af200a8 100644
@@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
  		 */
  		seq_puts(m, "Features\t:");
  		if (compat) {

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 137

Quoted 10 lines

arch/arm64/kernel/cpuinfo.c +1 -9

-#ifdef CONFIG_COMPAT
-			for (j = 0; compat_hwcap_str[j]; j++)
-				if (compat_elf_hwcap & (1 << j))
-					seq_printf(m, " %s", compat_hwcap_str[j]);
-
-			for (j = 0; compat_hwcap2_str[j]; j++)
-				if (compat_elf_hwcap2 & (1 << j))
-					seq_printf(m, " %s", compat_hwcap2_str[j]);
-#endif /* CONFIG_COMPAT */
+#ifdef CONFIG_AARCH32_EL0

I saw that compat_hwcap_str and compat_hwcap2_str is defined when "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0 in c show()?

Quoted 8 lines

Quoted 3 lines

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 138

Quoted 1 line

arch/arm64/kernel/cpuinfo.c +1 -0

+			if (personality(current->personality) == PER_LINUX32) {

And "compat" is "personality(current->personality) == PER_LINUX32;", it seems that there is no need to add this twice.

I think it would be best to remove the #ifdef here completely, the PER_LINUX32 concept is not strictly tied to the emulation of ARM binaries, it literally just changes the output of /proc/cpuinfo and 'uname',

Quoted 4 lines

It's not strictly related to ARM binaries, however it is related to AArch32 CPU features being supported and detected by the kernel. Currently, with CONFIG_COMPAT disabled, we won't have access to a (meaningful) compat_elf_hwcap.

Ah, makes sense. In that case, using CONFIG_AARCH32_EL0 sounds like the right thing to do here, though I guess we can just drop the "if (compat)" check, as we specifically want to print the supported features of the CPU, and they are still present even if a process with PER_LINUX reads them.

Quoted 10 lines

Quoted 3 lines

and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/[email protected]

Quoted 1 line

(you were summoned on that discussion couple of times ;))

Hmm, I thought I saw the thread and didn't have any good idea for the uname information, but didn't notice it was for /proc/cpuinfo.

What's wrong with always showing both the 32-bit and the 64-bit hwcap strings here (minus the duplicates, which hopefully have the same meaning here)?

Quoted 15 lines

Quoted 11 lines

Quoted 3 lines

and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/[email protected]

(you were summoned on that discussion couple of times ;))

Hmm, I thought I saw the thread and didn't have any good idea for the uname information, but didn't notice it was for /proc/cpuinfo.

Quoted 3 lines

What's wrong with always showing both the 32-bit and the 64-bit hwcap strings here (minus the duplicates, which hopefully have the same meaning here)?

As I said above, some of them have the same name (which may be a good thing at a first look) but we don't have an architecture guarantee that the feature is present in both AArch32 and AArch64 modes (e.g. AES may only be available in AArch64).

Quoted 19 lines · CEST Catalin Marinas

Quoted 18 lines

Quoted 11 lines

Quoted 3 lines

and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/[email protected]

(you were summoned on that discussion couple of times ;))

Hmm, I thought I saw the thread and didn't have any good idea for the uname information, but didn't notice it was for /proc/cpuinfo.

What's wrong with always showing both the 32-bit and the 64-bit hwcap strings here (minus the duplicates, which hopefully have the same meaning here)?

Quoted 4 lines

As I said above, some of them have the same name (which may be a good thing at a first look) but we don't have an architecture guarantee that the feature is present in both AArch32 and AArch64 modes (e.g. AES may only be available in AArch64).

Is this the case on actual implementations that exist today? If they are actually always both present, we might be able to get away with it.

Quoted 20 lines · Arnd Bergmann

Quoted 19 lines · CEST Catalin Marinas

Quoted 18 lines

Quoted 11 lines

Quoted 3 lines

and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas at arm.com

(you were summoned on that discussion couple of times ;))

Hmm, I thought I saw the thread and didn't have any good idea for the uname information, but didn't notice it was for /proc/cpuinfo.

What's wrong with always showing both the 32-bit and the 64-bit hwcap strings here (minus the duplicates, which hopefully have the same meaning here)?

Quoted 7 lines

Quoted 4 lines

As I said above, some of them have the same name (which may be a good thing at a first look) but we don't have an architecture guarantee that the feature is present in both AArch32 and AArch64 modes (e.g. AES may only be available in AArch64).

Is this the case on actual implementations that exist today? If they are actually always both present, we might be able to get away with it.

It may be fine on current implementations but what would we do when/if we actually find such discrepancy? It's not just ARM Ltd designing the chips, so as long as the architecture doesn't mandate it you may find strange implementations.

Imposing such restriction in the architecture doesn't make sense if the only reason is the /proc/cpuinfo file (and I can't think of any other reason why this should be enforced).

What I'm worried about is 32-bit apps running on an arm64 kernel and making use of the 64-bit /proc/cpuinfo without any guarantee that the AArch32 state has such features. In my patch proposal linked above I wanted to always force the compat /proc/cpuinfo for 32-bit tasks.

Signature · 2 lines

-- 
Catalin

Quoted 38 lines · Catalin Marinas

Quoted 20 lines · Arnd Bergmann

Quoted 19 lines · CEST Catalin Marinas

Quoted 18 lines

Quoted 11 lines

Quoted 3 lines

and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas at arm.com

(you were summoned on that discussion couple of times ;))

Hmm, I thought I saw the thread and didn't have any good idea for the uname information, but didn't notice it was for /proc/cpuinfo.

What's wrong with always showing both the 32-bit and the 64-bit hwcap strings here (minus the duplicates, which hopefully have the same meaning here)?

Quoted 7 lines

Quoted 4 lines

As I said above, some of them have the same name (which may be a good thing at a first look) but we don't have an architecture guarantee that the feature is present in both AArch32 and AArch64 modes (e.g. AES may only be available in AArch64).

Is this the case on actual implementations that exist today? If they are actually always both present, we might be able to get away with it.

It may be fine on current implementations but what would we do when/if we actually find such discrepancy? It's not just ARM Ltd designing the chips, so as long as the architecture doesn't mandate it you may find strange implementations.

Imposing such restriction in the architecture doesn't make sense if the only reason is the /proc/cpuinfo file (and I can't think of any other reason why this should be enforced).

Quoted 4 lines

What I'm worried about is 32-bit apps running on an arm64 kernel and making use of the 64-bit /proc/cpuinfo without any guarantee that the AArch32 state has such features. In my patch proposal linked above I wanted to always force the compat /proc/cpuinfo for 32-bit tasks.

The link doesn't work for me. Is there other link, or what's the maillist there?

So, what we decided finally? Is my understanding correct that we leave everything as is in ilp32 series, and it will be resolved separately?

Based on patch of Andrew Pinski.

This patch introduces is_a32_compat_task and is_a32_thread so it is easier to say this is a a32 specific thread or a generic compat thread/task. Corresponding functions are located in <asm/is_compat.h> to avoid mess in headers.

Some files include both <linux/compat.h> and <asm/compat.h>, and this is wrong because <linux/compat.h> has <asm/compat.h> already included. It was fixed too.

Signed-off-by Yury Norov <[email protected]>

Signed-off-by Philipp Tomsich <[email protected]>

Signed-off-by Christoph Muellner <[email protected]>

Signed-off-by Andrew Pinski <[email protected]>

Reviewed-by David Daney <[email protected]>

---
  arch/arm64/include/asm/compat.h      | 19 ++---------
  arch/arm64/include/asm/elf.h         | 10 +++---
  arch/arm64/include/asm/ftrace.h      |  2 +-
  arch/arm64/include/asm/is_compat.h   | 64 ++++++++++++++++++++++++++++++++++++
  arch/arm64/include/asm/memory.h      |  5 +--
  arch/arm64/include/asm/processor.h   |  5 +--
  arch/arm64/include/asm/syscall.h     |  2 +-
  arch/arm64/include/asm/thread_info.h |  2 +-
  arch/arm64/kernel/hw_breakpoint.c    | 10 +++---
  arch/arm64/kernel/perf_regs.c        |  2 +-
  arch/arm64/kernel/process.c          |  7 ++--
  arch/arm64/kernel/ptrace.c           | 11 +++----
  arch/arm64/kernel/signal.c           |  4 +--
  arch/arm64/kernel/traps.c            |  3 +-
  14 files changed, 98 insertions(+), 48 deletions(-)

create mode 100644 arch/arm64/include/asm/is_compat.h

arch/arm64/include/asm/compat.h +2 -17

index eb8432b..df2f72d 100644
@@ -24,6 +24,8 @@
  #include <linux/types.h>
  #include <linux/sched.h>
 
+#include <asm/is_compat.h>
+
  #define COMPAT_USER_HZ		100
  #ifdef __AARCH64EB__
  #define COMPAT_UTS_MACHINE	"armv8b\0\0"

@@ -298,23 +300,6 @@ struct compat_shmid64_ds {
  	compat_ulong_t __unused5;
  };
 
-static inline int is_compat_task(void)
-{
-	return test_thread_flag(TIF_32BIT);
-}
-
-static inline int is_compat_thread(struct thread_info *thread)
-{
-	return test_ti_thread_flag(thread, TIF_32BIT);
-}
-
-#else /* !CONFIG_COMPAT */
-
-static inline int is_compat_thread(struct thread_info *thread)
-{
-	return 0;
-}
-
  #endif /* CONFIG_COMPAT */
  #endif /* __KERNEL__ */
  #endif /* __ASM_COMPAT_H */

arch/arm64/include/asm/elf.h +5 -5

index 579b6e6..bef2d90 100644
@@ -16,6 +16,10 @@
  #ifndef __ASM_ELF_H
  #define __ASM_ELF_H
 
+#ifndef __ASSEMBLY__
+#include <linux/compat.h>
+#endif
+
  #include <asm/hwcap.h>
 
  /*

@@ -152,13 +156,9 @@ extern int arch_setup_additional_pages(struct linux_binprm *bprm,
  				       int uses_interp);
 
  /* 1GB of VA */
-#ifdef CONFIG_COMPAT
-#define STACK_RND_MASK			(test_thread_flag(TIF_32BIT) ? \
+#define STACK_RND_MASK			(is_compat_task() ? \
  						0x7ff >> (PAGE_SHIFT - 12) : \
  						0x3ffff >> (PAGE_SHIFT - 12))
-#else
-#define STACK_RND_MASK			(0x3ffff >> (PAGE_SHIFT - 12))
-#endif
 
  #ifdef __AARCH64EB__
  #define COMPAT_ELF_PLATFORM		("v8b")

arch/arm64/include/asm/ftrace.h +1 -1

index caa955f..0feb28a 100644
@@ -54,7 +54,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
  #define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS
  static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs)
  {
-	return is_compat_task();
+	return is_a32_compat_task();
  }
  #endif /* ifndef __ASSEMBLY__ */
 

arch/arm64/include/asm/is_compat.h new file +64 -0

new file mode 100644
index 0000000..8dba5ca
@@ -0,0 +1,64 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ASM_IS_COMPAT_H
+#define __ASM_IS_COMPAT_H
+#ifndef __ASSEMBLY__
+
+#include <linux/thread_bits.h>
+
+#ifdef CONFIG_AARCH32_EL0
+
+static inline int is_a32_compat_task(void)
+{
+	return test_thread_flag(TIF_32BIT);
+}
+
+static inline int is_a32_compat_thread(struct thread_info *thread)
+{
+	return test_ti_thread_flag(thread, TIF_32BIT);
+}
+
+#else
+
+static inline int is_a32_compat_task(void)
+
+{
+	return 0;
+}
+
+static inline int is_a32_compat_thread(struct thread_info *thread)
+{
+	return 0;
+}
+
+#endif /* CONFIG_AARCH32_EL0 */
+
+#ifdef CONFIG_COMPAT
+
+static inline int is_compat_task(void)
+{
+	return is_a32_compat_task();
+}
+
+#endif /* CONFIG_COMPAT */
+
+static inline int is_compat_thread(struct thread_info *thread)
+{
+	return is_a32_compat_thread(thread);
+}
+
+
+#endif /* !__ASSEMBLY__ */
+#endif /* __ASM_IS_COMPAT_H */

arch/arm64/include/asm/memory.h +1 -0

index 31b7322..4d734a1 100644
@@ -26,6 +26,7 @@
  #include <linux/types.h>
  #include <asm/bug.h>
  #include <asm/sizes.h>
+#include <asm/is_compat.h>
 
  /*
   * Allow for constants defined here to be used from assembly code

arch/arm64/include/asm/memory.h +0 -0

  				TASK_SIZE_32 : TASK_SIZE_64)
  #else
  #define TASK_SIZE		TASK_SIZE_64

arch/arm64/include/asm/processor.h +3 -2

index 5bbdbb4..1eccf47 100644
@@ -30,6 +30,7 @@
  #include <linux/string.h>
 
  #include <asm/alternative.h>
+#include <asm/is_compat.h>
  #include <asm/fpsimd.h>
  #include <asm/hw_breakpoint.h>
  #include <asm/lse.h>

@@ -41,7 +42,7 @@
  #define STACK_TOP_MAX		TASK_SIZE_64
  #ifdef CONFIG_COMPAT
  #define AARCH32_VECTORS_BASE	0xffff0000
-#define STACK_TOP		(test_thread_flag(TIF_32BIT) ? \
+#define STACK_TOP		(is_compat_task() ? \
  				AARCH32_VECTORS_BASE : STACK_TOP_MAX)
  #else
  #define STACK_TOP		STACK_TOP_MAX

@@ -94,7 +95,7 @@ struct thread_struct {
  #define task_user_tls(t)						\
  ({									\
  	unsigned long *__tls;						\
-	if (is_compat_thread(task_thread_info(t)))			\
+	if (is_a32_compat_thread(task_thread_info(t)))			\
  		__tls = &(t)->thread.tp2_value;				\
  	else								\
  		__tls = &(t)->thread.tp_value;				\

arch/arm64/include/asm/syscall.h +1 -1

index 709a574..ce09641 100644
@@ -113,7 +113,7 @@ static inline void syscall_set_arguments(struct task_struct *task,
   */
  static inline int syscall_get_arch(void)
  {
-	if (is_compat_task())
+	if (is_a32_compat_task())
  		return AUDIT_ARCH_ARM;
 
  	return AUDIT_ARCH_AARCH64;

arch/arm64/include/asm/thread_info.h +1 -1

index abd64bd..4daa559 100644
@@ -118,7 +118,7 @@ static inline struct thread_info *current_thread_info(void)
  #define TIF_FREEZE		19
  #define TIF_RESTORE_SIGMASK	20
  #define TIF_SINGLESTEP		21
-#define TIF_32BIT		22	/* 32bit process */
+#define TIF_32BIT		22	/* AARCH32 process */
 
  #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
  #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)

arch/arm64/kernel/hw_breakpoint.c +5 -5

index ce21aa8..d67f518 100644
@@ -164,7 +164,7 @@ enum hw_breakpoint_ops {
  	HW_BREAKPOINT_RESTORE
  };
 
-static int is_compat_bp(struct perf_event *bp)
+static int is_a32_compat_bp(struct perf_event *bp)
  {
  	struct task_struct *tsk = bp->hw.target;
 

@@ -175,7 +175,7 @@ static int is_compat_bp(struct perf_event *bp)
  	 * deprecated behaviour if we use unaligned watchpoints in
  	 * AArch64 state.
  	 */
-	return tsk && is_compat_thread(task_thread_info(tsk));
+	return tsk && is_a32_compat_thread(task_thread_info(tsk));
  }
 
  /**

@@ -435,7 +435,7 @@ static int arch_build_bp_info(struct perf_event *bp)
  	 * Watchpoints can be of length 1, 2, 4 or 8 bytes.
  	 */
  	if (info->ctrl.type == ARM_BREAKPOINT_EXECUTE) {
-		if (is_compat_bp(bp)) {
+		if (is_a32_compat_bp(bp)) {
  			if (info->ctrl.len != ARM_BREAKPOINT_LEN_2 &&
  			    info->ctrl.len != ARM_BREAKPOINT_LEN_4)
  				return -EINVAL;

@@ -492,7 +492,7 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
  	 * AArch32 tasks expect some simple alignment fixups, so emulate
  	 * that here.
  	 */
-	if (is_compat_bp(bp)) {
+	if (is_a32_compat_bp(bp)) {
  		if (info->ctrl.len == ARM_BREAKPOINT_LEN_8)
  			alignment_mask = 0x7;
  		else

@@ -679,7 +679,7 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
 
  		info = counter_arch_bp(wp);
  		/* AArch32 watchpoints are either 4 or 8 bytes aligned. */
-		if (is_compat_task()) {
+		if (is_a32_compat_task()) {
  			if (info->ctrl.len == ARM_BREAKPOINT_LEN_8)
  				alignment_mask = 0x7;
  			else

arch/arm64/kernel/perf_regs.c +1 -1

index 3f62b35..a79058f 100644
@@ -45,7 +45,7 @@ int perf_reg_validate(u64 mask)
 
  u64 perf_reg_abi(struct task_struct *task)
  {
-	if (is_compat_thread(task_thread_info(task)))
+	if (is_a32_compat_thread(task_thread_info(task)))
  		return PERF_SAMPLE_REGS_ABI_32;
  	else
  		return PERF_SAMPLE_REGS_ABI_64;

arch/arm64/kernel/process.c +3 -4

index 6cd2612..c008ec8 100644
@@ -47,7 +47,6 @@
  #include <trace/events/power.h>
 
  #include <asm/alternative.h>
-#include <asm/compat.h>
  #include <asm/cacheflush.h>
  #include <asm/fpsimd.h>
  #include <asm/mmu_context.h>

@@ -204,7 +203,7 @@ static void tls_thread_flush(void)
  {
  	asm ("msr tpidr_el0, xzr");
 
-	if (is_compat_task()) {
+	if (is_a32_compat_task()) {
  		current->thread.tp_value = 0;
 
  		/*

@@ -256,7 +255,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
  		asm("mrs %0, tpidr_el0" : "=r" (*task_user_tls(p)));
 
  		if (stack_start) {
-			if (is_compat_thread(task_thread_info(p)))
+			if (is_a32_compat_thread(task_thread_info(p)))
  				childregs->compat_sp = stack_start;
  			else
  				childregs->sp = stack_start;

@@ -293,7 +292,7 @@ static void tls_thread_switch(struct task_struct *next)
  	*task_user_tls(current) = tpidr;
 
  	tpidr = *task_user_tls(next);
-	tpidrro = is_compat_thread(task_thread_info(next)) ?
+	tpidrro = is_a32_compat_thread(task_thread_info(next)) ?
  		  next->thread.tp_value : 0;
 
  	asm(

arch/arm64/kernel/ptrace.c +5 -6

index aa79e81..38a09338 100644
@@ -38,7 +38,6 @@
  #include <linux/tracehook.h>
  #include <linux/elf.h>
 
-#include <asm/compat.h>
  #include <asm/debug-monitors.h>
  #include <asm/pgtable.h>
  #include <asm/syscall.h>

@@ -85,7 +84,7 @@ static void ptrace_hbptriggered(struct perf_event *bp,
  #ifdef CONFIG_AARCH32_EL0
  	int i;
 
-	if (!is_compat_task())
+	if (!is_a32_compat_task())
  		goto send_sig;
 
  	for (i = 0; i < ARM_MAX_BRP; ++i) {

@@ -1203,9 +1202,9 @@ const struct user_regset_view *task_user_regset_view(struct task_struct *task)
  	 * 32-bit children use an extended user_aarch32_ptrace_view to allow
  	 * access to the TLS register.
  	 */
-	if (is_compat_task())
+	if (is_a32_compat_task())
  		return &user_aarch32_view;
-	else if (is_compat_thread(task_thread_info(task)))
+	else if (is_a32_compat_thread(task_thread_info(task)))
  		return &user_aarch32_ptrace_view;
  #endif
  	return &user_aarch64_view;

@@ -1232,7 +1231,7 @@ static void tracehook_report_syscall(struct pt_regs *regs,
  	 * A scratch register (ip(r12) on AArch32, x7 on AArch64) is
  	 * used to denote syscall entry/exit:
  	 */
-	regno = (is_compat_task() ? 12 : 7);
+	regno = (is_a32_compat_task() ? 12 : 7);
  	saved_reg = regs->regs[regno];
  	regs->regs[regno] = dir;
 

@@ -1343,7 +1342,7 @@ int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task)
  	if (!test_tsk_thread_flag(task, TIF_SINGLESTEP))
  		regs->pstate &= ~DBG_SPSR_SS;
 
-	if (is_compat_thread(task_thread_info(task)))
+	if (is_a32_compat_thread(task_thread_info(task)))
  		return valid_compat_regs(regs);
  	else
  		return valid_native_regs(regs);

arch/arm64/kernel/signal.c +2 -2

index a8eafdb..be02f65 100644
@@ -276,7 +276,7 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
 
  static void setup_restart_syscall(struct pt_regs *regs)
  {
-	if (is_compat_task())
+	if (is_a32_compat_task())
  		compat_setup_restart_syscall(regs);
  	else
  		regs->regs[8] = __NR_restart_syscall;

@@ -295,7 +295,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
  	/*
  	 * Set up the stack frame
  	 */
-	if (is_compat_task()) {
+	if (is_a32_compat_task()) {
  		if (ksig->ka.sa.sa_flags & SA_SIGINFO)
  			ret = compat_setup_rt_frame(usig, ksig, oldset, regs);
  		else

arch/arm64/kernel/traps.c +2 -1

index 973faec..d77be17 100644
@@ -18,6 +18,7 @@
   */
 
  #include <linux/bug.h>
+#include <linux/compat.h>
  #include <linux/signal.h>
  #include <linux/personality.h>
  #include <linux/kallsyms.h>

@@ -411,7 +412,7 @@ asmlinkage long do_ni_syscall(struct pt_regs *regs)
  {
  #ifdef CONFIG_AARCH32_EL0
  	long ret;
-	if (is_compat_task()) {
+	if (is_a32_compat_task()) {
  		ret = compat_arm_syscall(regs);
  		if (ret != -ENOSYS)
  			return ret;

arch/arm64/include/asm/memory.h line 84

Quoted 9 lines

arch/arm64/include/asm/memory.h +2 -2

@@ -78,9 +79,9 @@
 
  #ifdef CONFIG_COMPAT
  #define TASK_SIZE_32		UL(0x100000000)
-#define TASK_SIZE		(test_thread_flag(TIF_32BIT) ? \
+#define TASK_SIZE		(is_compat_task() ?		\
  				TASK_SIZE_32 : TASK_SIZE_64)
-#define TASK_SIZE_OF(tsk)	(test_tsk_thread_flag(tsk, TIF_32BIT) ? \
+#define TASK_SIZE_OF(tsk)	(is_compat_thread(tsk) ? \

It should be
+#define TASK_SIZE_OF(tsk) (is_compat_thread(task_thread_info(tsk)) ? \
is_compat_thread takes "struct thread_info *thread" while TASK_SIZE_OF takes "struct task_struct *tsk"tsk.

Quoted 13 lines · Zhangjian (Bamvor)

arch/arm64/include/asm/memory.h line 84

Quoted 9 lines

arch/arm64/include/asm/memory.h +2 -2

@@ -78,9 +79,9 @@
 
  #ifdef CONFIG_COMPAT
  #define TASK_SIZE_32               UL(0x100000000)
-#define TASK_SIZE           (test_thread_flag(TIF_32BIT) ? \
+#define TASK_SIZE           (is_compat_task() ?             \
                             TASK_SIZE_32 : TASK_SIZE_64)
-#define TASK_SIZE_OF(tsk)   (test_tsk_thread_flag(tsk, TIF_32BIT) ? \
+#define TASK_SIZE_OF(tsk)   (is_compat_thread(tsk) ? \

It should be
+#define TASK_SIZE_OF(tsk) (is_compat_thread(task_thread_info(tsk)) ? \
is_compat_thread takes "struct thread_info *thread" while TASK_SIZE_OF takes "struct task_struct *tsk"tsk.

If that doesn't throw an error does that mean that TASK_SIZE_OF is unused?

Here new aarch32 ptrace syscall handler is introsuced to avoid run-time detection of the task type.

Signed-off-by Yury Norov<[email protected]>

---
  arch/arm64/include/asm/unistd32.h |  2 +-
  arch/arm64/kernel/ptrace.c        | 50 ++++++++++++++++++++++++++++++++++++++-
  arch/arm64/kernel/sys32.c         |  1 +
  include/linux/ptrace.h            |  6 +++++
  kernel/ptrace.c                   | 10 ++++----
  5 files changed, 62 insertions(+), 7 deletions(-)

arch/arm64/include/asm/unistd32.h +1 -1

index b7e8ef1..6da7cbd 100644
@@ -74,7 +74,7 @@ __SYSCALL(__NR_getuid, sys_getuid16)
  			/* 25 was sys_stime */
  __SYSCALL(25, sys_ni_syscall)
  #define __NR_ptrace 26
-__SYSCALL(__NR_ptrace, compat_sys_ptrace)
+__SYSCALL(__NR_ptrace, compat_sys_aarch32_ptrace)
  			/* 27 was sys_alarm */
  __SYSCALL(27, sys_ni_syscall)
  			/* 28 was sys_fstat */

arch/arm64/kernel/ptrace.c +49 -1

index 38a09338..a861105 100644
@@ -29,6 +29,7 @@
  #include<linux/user.h>
  #include<linux/seccomp.h>
  #include<linux/security.h>
+#include<linux/syscalls.h>
  #include<linux/init.h>
  #include<linux/signal.h>
  #include<linux/uaccess.h>

@@ -1114,7 +1115,7 @@ static int compat_ptrace_sethbpregs(struct task_struct *tsk, compat_long_t num,
  }
  #endif	/* CONFIG_HAVE_HW_BREAKPOINT */
 
-long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
+static long compat_a32_ptrace(struct task_struct *child, compat_long_t request,
  			compat_ulong_t caddr, compat_ulong_t cdata)
  {
  	unsigned long addr = caddr;

@@ -1191,8 +1192,55 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
 
  	return ret;
  }
+
+COMPAT_SYSCALL_DEFINE4(aarch32_ptrace, compat_long_t, request, compat_long_t, pid,
+		       compat_long_t, addr, compat_long_t, data)
+{
+	struct task_struct *child;
+	long ret;
+
+	if (request == PTRACE_TRACEME) {
+		ret = ptrace_traceme();
+		goto out;
+	}
+
+	child = ptrace_get_task_struct(pid);
+	if (IS_ERR(child)) {
+		ret = PTR_ERR(child);
+		goto out;
+	}
+
+	if (request == PTRACE_ATTACH || request == PTRACE_SEIZE) {
+		ret = ptrace_attach(child, request, addr, data);
+		goto out_put_task_struct;
+	}
+
+	ret = ptrace_check_attach(child, request == PTRACE_KILL ||
+				  request == PTRACE_INTERRUPT);
+	if (!ret) {
+		ret = compat_a32_ptrace(child, request, addr, data);
+		if (ret || request != PTRACE_DETACH)
+			ptrace_unfreeze_traced(child);
+	}
+
+ out_put_task_struct:
+	put_task_struct(child);
+ out:
+	return ret;
+}
+
  #endif /* CONFIG_AARCH32_EL0 */
 
+#ifdef CONFIG_COMPAT
+
+long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
+			compat_ulong_t caddr, compat_ulong_t cdata)
+{
+	return compat_ptrace_request(child, request, caddr, cdata);
+}
+
+#endif /* CONFIG_COMPAT */
+
  const struct user_regset_view *task_user_regset_view(struct task_struct *task)
  {
  #ifdef CONFIG_AARCH32_EL0

arch/arm64/kernel/sys32.c +1 -0

index a40b134..3752443 100644
@@ -38,6 +38,7 @@ asmlinkage long compat_sys_fadvise64_64_wrapper(void);
  asmlinkage long compat_sys_sync_file_range2_wrapper(void);
  asmlinkage long compat_sys_fallocate_wrapper(void);
  asmlinkage long compat_sys_mmap2_wrapper(void);
+asmlinkage long compat_sys_aarch32_ptrace(void);
 
  #undef __SYSCALL
  #define __SYSCALL(nr, sym)	[nr] = sym,

include/linux/ptrace.h +6 -0

index 504c98a..75887a0 100644
@@ -97,6 +97,12 @@ int generic_ptrace_peekdata(struct task_struct *tsk, unsigned long addr,
  			    unsigned long data);
  int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr,
  			    unsigned long data);
+int ptrace_traceme(void);
+struct task_struct *ptrace_get_task_struct(pid_t pid);
+int ptrace_attach(struct task_struct *task, long request,
+			 unsigned long addr, unsigned long flags);
+int ptrace_check_attach(struct task_struct *child, bool ignore_state);
+void ptrace_unfreeze_traced(struct task_struct *task);
 
  /**
   * ptrace_parent - return the task that is tracing the given task

kernel/ptrace.c +5 -5

index d49bfa1..cadf24c 100644
@@ -136,7 +136,7 @@ static bool ptrace_freeze_traced(struct task_struct *task)
  	return ret;
  }
 
-static void ptrace_unfreeze_traced(struct task_struct *task)
+void ptrace_unfreeze_traced(struct task_struct *task)
  {
  	if (task->state != __TASK_TRACED)
  		return;

@@ -168,7 +168,7 @@ static void ptrace_unfreeze_traced(struct task_struct *task)
   * RETURNS:
   * 0 on success, -ESRCH if %child is not ready.
   */
-static int ptrace_check_attach(struct task_struct *child, bool ignore_state)
+int ptrace_check_attach(struct task_struct *child, bool ignore_state)
  {
  	int ret = -ESRCH;
 

@@ -292,7 +292,7 @@ bool ptrace_may_access(struct task_struct *task, unsigned int mode)
  	return !err;
  }
 
-static int ptrace_attach(struct task_struct *task, long request,
+int ptrace_attach(struct task_struct *task, long request,
  			 unsigned long addr,
  			 unsigned long flags)
  {

@@ -406,7 +406,7 @@ out:
   * Performs checks and sets PT_PTRACED.
   * Should be used by all ptrace implementations for PTRACE_TRACEME.
   */
-static int ptrace_traceme(void)
+ int ptrace_traceme(void)
  {
  	int ret = -EPERM;
 

@@ -1056,7 +1056,7 @@ int ptrace_request(struct task_struct *child, long request,
  	return ret;
  }
 
-static struct task_struct *ptrace_get_task_struct(pid_t pid)
+struct task_struct *ptrace_get_task_struct(pid_t pid)
  {
  	struct task_struct *child;
 

atus: RO
Content-Length: 4732
Lines: 181

The function compat_ptrace_request(used by ilp32) don't handle {GET,SET}SIGMASK request, so it will be handled by ptrace_request. But it's wrong because the compat_sigset_t of ilp32 differs from the sigset_t of aarch64. The patch fixes it.

Signed-off-by Zhou Chengming <[email protected]>

---
 arch/arm64/include/asm/signal_ilp32.h |   22 ++++++++++++
 arch/arm64/kernel/ptrace.c            |   62

+++++++++++++++++++++++++++++++++

index 30eff23..ed52607 100644

arch/arm64/include/asm/signal_ilp32.h +22 -0

@@ -21,6 +21,28 @@
 int ilp32_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
 			  struct pt_regs *regs);
 
+static inline int put_sigset_t(compat_sigset_t __user *uset, sigset_t *set)
+{
+	compat_sigset_t cset;
+
+	cset.sig[0] = set->sig[0] & 0xffffffffull;
+	cset.sig[1] = set->sig[0] >> 32;
+
+	return copy_to_user(uset, &cset, sizeof(*uset));
+}
+
+static inline int get_sigset_t(sigset_t *set,
+			       const compat_sigset_t __user *uset)
+{
+	compat_sigset_t s32;
+
+	if (copy_from_user(&s32, uset, sizeof(*uset)))
+		return -EFAULT;
+
+	set->sig[0] = s32.sig[0] | (((long)s32.sig[1]) << 32);
+	return 0;
+}
+
 #else
 
 static inline int ilp32_setup_rt_frame(int usig, struct ksignal *ksig,

sigset_t *set,

arch/arm64/kernel/ptrace.c +1 -0

index a861105..8d4cad1 100644
@@ -44,6 +44,7 @@
 #include <asm/syscall.h>
 #include <asm/traps.h>
 #include <asm/system_misc.h>
+#include <asm/signal_ilp32.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>

@@ -1231,12 +1232,73 @@ COMPAT_SYSCALL_DEFINE4(aarch32_ptrace,

compat_long_t, request, compat_long_t, pi

#endif /* CONFIG_AARCH32_EL0 */

arch/arm64/kernel/ptrace.c +3 -0

+#ifdef CONFIG_ARM64_ILP32
+
+static int compat_ilp32_ptrace(struct task_struct *child, compat_long_t

request,

arch/arm64/kernel/ptrace.c +58 -0

+			compat_ulong_t addr, compat_ulong_t data)
+{
+	compat_ulong_t __user *datap = compat_ptr(data);
+	int ret;
+
+	switch (request) {
+	case PTRACE_GETSIGMASK:
+		if (addr != sizeof(compat_sigset_t)) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (put_sigset_t((compat_sigset_t __user *)datap, &child->blocked))
+			ret = -EFAULT;
+		else
+			ret = 0;
+		break;
+
+	case PTRACE_SETSIGMASK: {
+		sigset_t new_set;
+		if (addr != sizeof(compat_sigset_t)) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (get_sigset_t(&new_set, (compat_sigset_t __user *)datap)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		sigdelsetmask(&new_set, sigmask(SIGKILL)|sigmask(SIGSTOP));
+
+		/*
+		 * Every thread does recalc_sigpending() after resume, so
+		 * retarget_shared_pending() and recalc_sigpending() are not
+		 * called here.
+		 */
+		spin_lock_irq(&child->sighand->siglock);
+		child->blocked = new_set;
+		spin_unlock_irq(&child->sighand->siglock);
+
+		ret = 0;
+		break;
+	}
+
+	default:
+		ret = compat_ptrace_request(child, request, addr, data);
+	}
+
+	return ret;
+}
+
+#endif /* CONFIG_ARM64_ILP32 */
+
 #ifdef CONFIG_COMPAT
 
 long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
 			compat_ulong_t caddr, compat_ulong_t cdata)
 {
+#ifdef CONFIG_ARM64_ILP32
+	return compat_ilp32_ptrace(child, request, caddr, cdata);
+#else
 	return compat_ptrace_request(child, request, caddr, cdata);
+#endif
 }
 
 #endif /* CONFIG_COMPAT */

arch/arm64/kernel/signal_ilp32.c +1 -22

index 8ca64b9..3ef2749 100644
@@ -28,6 +28,7 @@
 #include <asm/fpsimd.h>
 #include <asm/signal32_common.h>
 #include <asm/signal_common.h>
+#include <asm/signal_ilp32.h>
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
 #include <asm/ucontext.h>

@@ -58,28 +59,6 @@ struct ilp32_rt_sigframe {
 	struct ilp32_sigframe sig;
 };
 
-static inline int put_sigset_t(compat_sigset_t __user *uset, sigset_t *set)
-{
-	compat_sigset_t cset;
-
-	cset.sig[0] = set->sig[0] & 0xffffffffull;
-	cset.sig[1] = set->sig[0] >> 32;
-
-	return copy_to_user(uset, &cset, sizeof(*uset));
-}
-
-static inline int get_sigset_t(sigset_t *set,
-                               const compat_sigset_t __user *uset)
-{
-	compat_sigset_t s32;
-
-	if (copy_from_user(&s32, uset, sizeof(*uset)))
-		return -EFAULT;
-
-	set->sig[0] = s32.sig[0] | (((long)s32.sig[1]) << 32);
-	return 0;
-}
-
 static int restore_ilp32_sigframe(struct pt_regs *regs,
                             struct

Hi Zhou,

Thank you for the patch. The idea is ok, but patch format got broken for some reason. Could you re-send it?

Yury.

Quoted 9 lines

atus: RO
Content-Length: 4732
Lines: 181

The function compat_ptrace_request(used by ilp32) don't handle {GET,SET}SIGMASK request, so it will be handled by ptrace_request. But it's wrong because the compat_sigset_t of ilp32 differs from the sigset_t of aarch64. The patch fixes it.

Quoted 5 lines

Signed-off-by Zhou Chengming<[email protected]>

---
  arch/arm64/include/asm/signal_ilp32.h |   22 ++++++++++++
  arch/arm64/kernel/ptrace.c            |   62

+++++++++++++++++++++++++++++++++

Here - unneeded line break

arch/arm64/include/asm/signal_ilp32.h

Quoted 5 lines

  arch/arm64/kernel/signal_ilp32.c      |   23 +------------
  3 files changed, 85 insertions(+), 22 deletions(-)

arch/arm64/include/asm/signal_ilp32.h +0 -0

and here

Quoted 32 lines

index 30eff23..ed52607 100644

arch/arm64/include/asm/signal_ilp32.h +22 -0

@@ -21,6 +21,28 @@
  int ilp32_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
  			  struct pt_regs *regs);
 
+static inline int put_sigset_t(compat_sigset_t __user *uset, sigset_t *set)
+{
+	compat_sigset_t cset;
+
+	cset.sig[0] = set->sig[0]&  0xffffffffull;
+	cset.sig[1] = set->sig[0]>>  32;
+
+	return copy_to_user(uset,&cset, sizeof(*uset));
+}
+
+static inline int get_sigset_t(sigset_t *set,
+			       const compat_sigset_t __user *uset)
+{
+	compat_sigset_t s32;
+
+	if (copy_from_user(&s32, uset, sizeof(*uset)))
+		return -EFAULT;
+
+	set->sig[0] = s32.sig[0] | (((long)s32.sig[1])<<  32);
+	return 0;
+}
+
  #else
 
  static inline int ilp32_setup_rt_frame(int usig, struct ksignal *ksig,

Quoted 1 line

sigset_t *set,

and here

Quoted 13 lines

arch/arm64/kernel/ptrace.c +1 -0

index a861105..8d4cad1 100644
@@ -44,6 +44,7 @@
  #include<asm/syscall.h>
  #include<asm/traps.h>
  #include<asm/system_misc.h>
+#include<asm/signal_ilp32.h>
 
  #define CREATE_TRACE_POINTS
  #include<trace/events/syscalls.h>

@@ -1231,12 +1232,73 @@ COMPAT_SYSCALL_DEFINE4(aarch32_ptrace,

Quoted 1 line

compat_long_t, request, compat_long_t, pi

and later on the patch

Quoted 109 lines

  #endif /* CONFIG_AARCH32_EL0 */

arch/arm64/kernel/ptrace.c +3 -0

+#ifdef CONFIG_ARM64_ILP32
+
+static int compat_ilp32_ptrace(struct task_struct *child, compat_long_t

request,

arch/arm64/kernel/ptrace.c +58 -0

+			compat_ulong_t addr, compat_ulong_t data)
+{
+	compat_ulong_t __user *datap = compat_ptr(data);
+	int ret;
+
+	switch (request) {
+	case PTRACE_GETSIGMASK:
+		if (addr != sizeof(compat_sigset_t)) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (put_sigset_t((compat_sigset_t __user *)datap,&child->blocked))
+			ret = -EFAULT;
+		else
+			ret = 0;
+		break;
+
+	case PTRACE_SETSIGMASK: {
+		sigset_t new_set;
+		if (addr != sizeof(compat_sigset_t)) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (get_sigset_t(&new_set, (compat_sigset_t __user *)datap)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		sigdelsetmask(&new_set, sigmask(SIGKILL)|sigmask(SIGSTOP));
+
+		/*
+		 * Every thread does recalc_sigpending() after resume, so
+		 * retarget_shared_pending() and recalc_sigpending() are not
+		 * called here.
+		 */
+		spin_lock_irq(&child->sighand->siglock);
+		child->blocked = new_set;
+		spin_unlock_irq(&child->sighand->siglock);
+
+		ret = 0;
+		break;
+	}
+
+	default:
+		ret = compat_ptrace_request(child, request, addr, data);
+	}
+
+	return ret;
+}
+
+#endif /* CONFIG_ARM64_ILP32 */
+
  #ifdef CONFIG_COMPAT
 
  long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
  			compat_ulong_t caddr, compat_ulong_t cdata)
  {
+#ifdef CONFIG_ARM64_ILP32
+	return compat_ilp32_ptrace(child, request, caddr, cdata);
+#else
  	return compat_ptrace_request(child, request, caddr, cdata);
+#endif
  }
 
  #endif /* CONFIG_COMPAT */

arch/arm64/kernel/signal_ilp32.c +1 -18

index 8ca64b9..3ef2749 100644
@@ -28,6 +28,7 @@
  #include<asm/fpsimd.h>
  #include<asm/signal32_common.h>
  #include<asm/signal_common.h>
+#include<asm/signal_ilp32.h>
  #include<asm/uaccess.h>
  #include<asm/unistd.h>
  #include<asm/ucontext.h>

@@ -58,28 +59,6 @@ struct ilp32_rt_sigframe {
  	struct ilp32_sigframe sig;
  };
 
-static inline int put_sigset_t(compat_sigset_t __user *uset, sigset_t *set)
-{
-	compat_sigset_t cset;
-
-	cset.sig[0] = set->sig[0]&  0xffffffffull;
-	cset.sig[1] = set->sig[0]>>  32;
-
-	return copy_to_user(uset,&cset, sizeof(*uset));
-}
-
-static inline int get_sigset_t(sigset_t *set,
-                               const compat_sigset_t __user *uset)
-{
-	compat_sigset_t s32;
-
-	if (copy_from_user(&s32, uset, sizeof(*uset)))
-		return -EFAULT;
-