[PATCH v2 0/2] cleanup {COMPAT_,}SET_PERSONALITY

[PATCH v2 0/2] cleanup {COMPAT_,}SET_PERSONALITY

[Yury Norov]

This patchset is the result of discussion:
https://lkml.org/lkml/2017/7/31/454

First patch introduces MMCF flags for mm_context_t ->flags to separate it from
TIF ones. And second patch moves personality-related setup code from
SET_PERSONALITY() to the helper.

Yury Norov (2):
  arm64: introduce separated bits for mm_context_t flags
  arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

 arch/arm64/include/asm/elf.h         | 7 +++++--
 arch/arm64/include/asm/mmu.h         | 2 ++
 arch/arm64/include/asm/thread_info.h | 3 +++
 arch/arm64/kernel/probes/uprobes.c   | 2 +-
 arch/arm64/kernel/process.c          | 8 ++++++++
 5 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.11.0

[PATCH v2 0/2] cleanup {COMPAT_,}SET_PERSONALITY

[Yury Norov]

This patchset is the result of discussion:
https://lkml.org/lkml/2017/7/31/454

First patch introduces MMCF flags for mm_context_t ->flags to separate it from
TIF ones. And second patch moves personality-related setup code from
SET_PERSONALITY() to the helper.

Yury Norov (2):
  arm64: introduce separated bits for mm_context_t flags
  arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

 arch/arm64/include/asm/elf.h         | 7 +++++--
 arch/arm64/include/asm/mmu.h         | 2 ++
 arch/arm64/include/asm/thread_info.h | 3 +++
 arch/arm64/kernel/probes/uprobes.c   | 2 +-
 arch/arm64/kernel/process.c          | 8 ++++++++
 5 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.11.0

[PATCH 1/2] arm64: introduce separated bits for mm_context_t flags

[Yury Norov]

Currently mm->context.flags field uses thread_info flags which is not
the best idea for many reasons. For example, mm_context_t doesn't need
most of thread_info flags. And it would be difficult to add new mm-related
flag if needed because it may easily interfere with TIF ones.

To deal with it, the new MMCF_AARCH32 flag is introduced for
mm_context_t->flags, where MMCF prefix stands for mm_context_t flags.
Also, mm_context_t flag doesn't require atomicity and ordering of the
access, so using set/clear_bit() is replaced with simple masks.

RFC: https://lkml.org/lkml/2017/7/31/454
v1:
 - changed the MMCF_AARCH32 bit number from 0x1 to 0x0 and added comment
v2:
 - using set/clear_bit() is replaced with simple masks.

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 arch/arm64/include/asm/elf.h       | 4 ++--
 arch/arm64/include/asm/mmu.h       | 2 ++
 arch/arm64/kernel/probes/uprobes.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 3288c2b36731..517b9ff0456c 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -139,7 +139,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
 
 #define SET_PERSONALITY(ex)						\
 ({									\
-	clear_bit(TIF_32BIT, &current->mm->context.flags);		\
+	current->mm->context.flags = 0;					\
 	clear_thread_flag(TIF_32BIT);					\
 	current->personality &= ~READ_IMPLIES_EXEC;			\
 })
@@ -195,7 +195,7 @@ typedef compat_elf_greg_t		compat_elf_gregset_t[COMPAT_ELF_NGREG];
  */
 #define COMPAT_SET_PERSONALITY(ex)					\
 ({									\
-	set_bit(TIF_32BIT, &current->mm->context.flags);		\
+	current->mm->context.flags = MMCF_AARCH32;			\
 	set_thread_flag(TIF_32BIT);					\
  })
 #define COMPAT_ARCH_DLINFO
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 5468c834b072..e4c474c6501e 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -16,6 +16,8 @@
 #ifndef __ASM_MMU_H
 #define __ASM_MMU_H
 
+#define MMCF_AARCH32	0x1	/* MM hosts AArch32 executables */
+
 typedef struct {
 	atomic64_t	id;
 	void		*vdso;
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index 26c998534dca..636ca0119c0e 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -40,7 +40,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
 	probe_opcode_t insn;
 
 	/* TODO: Currently we do not support AARCH32 instruction probing */
-	if (test_bit(TIF_32BIT, &mm->context.flags))
+	if (mm->context.flags & MMCF_AARCH32)
 		return -ENOTSUPP;
 	else if (!IS_ALIGNED(addr, AARCH64_INSN_SIZE))
 		return -EINVAL;
-- 
2.11.0

[PATCH 1/2] arm64: introduce separated bits for mm_context_t flags

[Yury Norov]

Currently mm->context.flags field uses thread_info flags which is not
the best idea for many reasons. For example, mm_context_t doesn't need
most of thread_info flags. And it would be difficult to add new mm-related
flag if needed because it may easily interfere with TIF ones.

To deal with it, the new MMCF_AARCH32 flag is introduced for
mm_context_t->flags, where MMCF prefix stands for mm_context_t flags.
Also, mm_context_t flag doesn't require atomicity and ordering of the
access, so using set/clear_bit() is replaced with simple masks.

RFC: https://lkml.org/lkml/2017/7/31/454
v1:
 - changed the MMCF_AARCH32 bit number from 0x1 to 0x0 and added comment
v2:
 - using set/clear_bit() is replaced with simple masks.

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 arch/arm64/include/asm/elf.h       | 4 ++--
 arch/arm64/include/asm/mmu.h       | 2 ++
 arch/arm64/kernel/probes/uprobes.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 3288c2b36731..517b9ff0456c 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -139,7 +139,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
 
 #define SET_PERSONALITY(ex)						\
 ({									\
-	clear_bit(TIF_32BIT, &current->mm->context.flags);		\
+	current->mm->context.flags = 0;					\
 	clear_thread_flag(TIF_32BIT);					\
 	current->personality &= ~READ_IMPLIES_EXEC;			\
 })
@@ -195,7 +195,7 @@ typedef compat_elf_greg_t		compat_elf_gregset_t[COMPAT_ELF_NGREG];
  */
 #define COMPAT_SET_PERSONALITY(ex)					\
 ({									\
-	set_bit(TIF_32BIT, &current->mm->context.flags);		\
+	current->mm->context.flags = MMCF_AARCH32;			\
 	set_thread_flag(TIF_32BIT);					\
  })
 #define COMPAT_ARCH_DLINFO
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 5468c834b072..e4c474c6501e 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -16,6 +16,8 @@
 #ifndef __ASM_MMU_H
 #define __ASM_MMU_H
 
+#define MMCF_AARCH32	0x1	/* MM hosts AArch32 executables */
+
 typedef struct {
 	atomic64_t	id;
 	void		*vdso;
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index 26c998534dca..636ca0119c0e 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -40,7 +40,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
 	probe_opcode_t insn;
 
 	/* TODO: Currently we do not support AARCH32 instruction probing */
-	if (test_bit(TIF_32BIT, &mm->context.flags))
+	if (mm->context.flags & MMCF_AARCH32)
 		return -ENOTSUPP;
 	else if (!IS_ALIGNED(addr, AARCH64_INSN_SIZE))
 		return -EINVAL;
-- 
2.11.0

[PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

[Yury Norov]

There is some work that should be done after setting the personality.
Currently it's done in the macro, which is not the best idea.

In this patch new arch_setup_new_exec() routine is introduced, and all
setup code is moved there, as suggested by Catalin:
https://lkml.org/lkml/2017/8/4/494

v2:
 - don't move clearing READ_IMPLIES_EXEC flag from SET_PERSONALITY()

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
CC: Pratyush Anand <panand@redhat.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/elf.h         | 7 +++++--
 arch/arm64/include/asm/thread_info.h | 3 +++
 arch/arm64/kernel/process.c          | 8 ++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 517b9ff0456c..9c4434b8a9a8 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -137,9 +137,13 @@ typedef struct user_fpsimd_state elf_fpregset_t;
  */
 #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
 
+/*
+ * Don't modify this macro unless you add new personality.
+ * All personality-related setup should be done at proper place.
+ * If not sure, consider the arch_setup_new_exec() function.
+ */
 #define SET_PERSONALITY(ex)						\
 ({									\
-	current->mm->context.flags = 0;					\
 	clear_thread_flag(TIF_32BIT);					\
 	current->personality &= ~READ_IMPLIES_EXEC;			\
 })
@@ -195,7 +199,6 @@ typedef compat_elf_greg_t		compat_elf_gregset_t[COMPAT_ELF_NGREG];
  */
 #define COMPAT_SET_PERSONALITY(ex)					\
 ({									\
-	current->mm->context.flags = MMCF_AARCH32;			\
 	set_thread_flag(TIF_32BIT);					\
  })
 #define COMPAT_ARCH_DLINFO
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93cf865..c823d2f12b4c 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -68,6 +68,9 @@ struct thread_info {
 #define thread_saved_fp(tsk)	\
 	((unsigned long)(tsk->thread.cpu_context.fp))
 
+void arch_setup_new_exec(void);
+#define arch_setup_new_exec     arch_setup_new_exec
+
 #endif
 
 /*
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 659ae8094ed5..e99012c4803a 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -417,3 +417,11 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
 	else
 		return randomize_page(mm->brk, SZ_1G);
 }
+
+/*
+ * Called immediately after a successful exec.
+ */
+void arch_setup_new_exec(void)
+{
+	current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
+}
-- 
2.11.0

[PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

[Yury Norov]

There is some work that should be done after setting the personality.
Currently it's done in the macro, which is not the best idea.

In this patch new arch_setup_new_exec() routine is introduced, and all
setup code is moved there, as suggested by Catalin:
https://lkml.org/lkml/2017/8/4/494

v2:
 - don't move clearing READ_IMPLIES_EXEC flag from SET_PERSONALITY()

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
CC: Pratyush Anand <panand@redhat.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/elf.h         | 7 +++++--
 arch/arm64/include/asm/thread_info.h | 3 +++
 arch/arm64/kernel/process.c          | 8 ++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 517b9ff0456c..9c4434b8a9a8 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -137,9 +137,13 @@ typedef struct user_fpsimd_state elf_fpregset_t;
  */
 #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
 
+/*
+ * Don't modify this macro unless you add new personality.
+ * All personality-related setup should be done at proper place.
+ * If not sure, consider the arch_setup_new_exec() function.
+ */
 #define SET_PERSONALITY(ex)						\
 ({									\
-	current->mm->context.flags = 0;					\
 	clear_thread_flag(TIF_32BIT);					\
 	current->personality &= ~READ_IMPLIES_EXEC;			\
 })
@@ -195,7 +199,6 @@ typedef compat_elf_greg_t		compat_elf_gregset_t[COMPAT_ELF_NGREG];
  */
 #define COMPAT_SET_PERSONALITY(ex)					\
 ({									\
-	current->mm->context.flags = MMCF_AARCH32;			\
 	set_thread_flag(TIF_32BIT);					\
  })
 #define COMPAT_ARCH_DLINFO
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93cf865..c823d2f12b4c 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -68,6 +68,9 @@ struct thread_info {
 #define thread_saved_fp(tsk)	\
 	((unsigned long)(tsk->thread.cpu_context.fp))
 
+void arch_setup_new_exec(void);
+#define arch_setup_new_exec     arch_setup_new_exec
+
 #endif
 
 /*
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 659ae8094ed5..e99012c4803a 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -417,3 +417,11 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
 	else
 		return randomize_page(mm->brk, SZ_1G);
 }
+
+/*
+ * Called immediately after a successful exec.
+ */
+void arch_setup_new_exec(void)
+{
+	current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
+}
-- 
2.11.0

Re: [PATCH v2 0/2] cleanup {COMPAT_,}SET_PERSONALITY

[Catalin Marinas]

On Sun, Aug 20, 2017 at 01:20:46PM +0300, Yury Norov wrote:
> This patchset is the result of discussion:
> https://lkml.org/lkml/2017/7/31/454
> 
> First patch introduces MMCF flags for mm_context_t ->flags to separate it from
> TIF ones. And second patch moves personality-related setup code from
> SET_PERSONALITY() to the helper.
> 
> Yury Norov (2):
>   arm64: introduce separated bits for mm_context_t flags
>   arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

Patches queued for 4.14. Thanks.

-- 
Catalin

[PATCH v2 0/2] cleanup {COMPAT_,}SET_PERSONALITY

[Catalin Marinas]

On Sun, Aug 20, 2017 at 01:20:46PM +0300, Yury Norov wrote:
> This patchset is the result of discussion:
> https://lkml.org/lkml/2017/7/31/454
> 
> First patch introduces MMCF flags for mm_context_t ->flags to separate it from
> TIF ones. And second patch moves personality-related setup code from
> SET_PERSONALITY() to the helper.
> 
> Yury Norov (2):
>   arm64: introduce separated bits for mm_context_t flags
>   arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

Patches queued for 4.14. Thanks.

-- 
Catalin

Re: [PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

[Catalin Marinas]

On Sat, Aug 05, 2017 at 05:40:22PM +0300, Yury Norov wrote:
> Originally {COMPAT_,}SET_PERSONALITY() only sets the 32-bit flag in thread_info
> structure. But there is some work that should be done after setting the personality.
> Currently it's done in the macro, which is not the best idea.
> 
> In this patch new arch_setup_new_exec() routine is introduced, and all setup code
> is moved there, as suggested by Catalin:
> https://lkml.org/lkml/2017/8/4/494
> 
> Note: mm->context.flags doesn't require the atomic strong ordered acceess to the
> field, so use __set_bit() there;

As I replied to patch 1, we don't even need __set_bit() but just '|='
for setting and '&' for testing.

> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index de11ed1484e3..615953243961 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -137,11 +137,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>   */
>  #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
>  
> +/*
> + * Don't modify this macro unless you add new personality.
> + * All personality-related setup should be done at proper place.
> + * If not sure, consider the arch_setup_new_exec() function.
> + */
>  #define SET_PERSONALITY(ex)						\
>  ({									\
> -	clear_bit(MMCF_AARCH32, &current->mm->context.flags);		\
>  	clear_thread_flag(TIF_32BIT);					\
> -	current->personality &= ~READ_IMPLIES_EXEC;			\
>  })

What I meant is that we keep the personality setting in SET_PERSONALITY,
together with the existing TIF bits but just move the mm->context.flags
setting out.

> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 46c3b93cf865..c823d2f12b4c 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -68,6 +68,9 @@ struct thread_info {
>  #define thread_saved_fp(tsk)	\
>  	((unsigned long)(tsk->thread.cpu_context.fp))
>  
> +void arch_setup_new_exec(void);
> +#define arch_setup_new_exec     arch_setup_new_exec

I'm fine with out of line implementation, it probably helps with any
header conflicts (and it's not a fast path anyway).

> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 659ae8094ed5..ebca9e4f62c7 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -417,3 +417,20 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
>  	else
>  		return randomize_page(mm->brk, SZ_1G);
>  }
> +
> +/*
> + * Called immediately after a successful exec.
> + */
> +void arch_setup_new_exec(void)
> +{
> +	current->mm->context.flags = 0;
> +
> +	/*
> +	 * Unlike the native one, the compat version of exec() inherits
> +	 * READ_IMPLIES_EXEC since this is the behaviour on arch/arm/.
> +	 */
> +	if (is_compat_task())
> +		__set_bit(MMCF_AARCH32, &current->mm->context.flags);
> +	else
> +		current->personality &= ~READ_IMPLIES_EXEC;
> +}

As I said above, just context.flags |= MMCF_AARCH32.

Thanks.

-- 
Catalin

[PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

[Catalin Marinas]

On Sat, Aug 05, 2017 at 05:40:22PM +0300, Yury Norov wrote:
> Originally {COMPAT_,}SET_PERSONALITY() only sets the 32-bit flag in thread_info
> structure. But there is some work that should be done after setting the personality.
> Currently it's done in the macro, which is not the best idea.
> 
> In this patch new arch_setup_new_exec() routine is introduced, and all setup code
> is moved there, as suggested by Catalin:
> https://lkml.org/lkml/2017/8/4/494
> 
> Note: mm->context.flags doesn't require the atomic strong ordered acceess to the
> field, so use __set_bit() there;

As I replied to patch 1, we don't even need __set_bit() but just '|='
for setting and '&' for testing.

> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index de11ed1484e3..615953243961 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -137,11 +137,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>   */
>  #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
>  
> +/*
> + * Don't modify this macro unless you add new personality.
> + * All personality-related setup should be done at proper place.
> + * If not sure, consider the arch_setup_new_exec() function.
> + */
>  #define SET_PERSONALITY(ex)						\
>  ({									\
> -	clear_bit(MMCF_AARCH32, &current->mm->context.flags);		\
>  	clear_thread_flag(TIF_32BIT);					\
> -	current->personality &= ~READ_IMPLIES_EXEC;			\
>  })

What I meant is that we keep the personality setting in SET_PERSONALITY,
together with the existing TIF bits but just move the mm->context.flags
setting out.

> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 46c3b93cf865..c823d2f12b4c 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -68,6 +68,9 @@ struct thread_info {
>  #define thread_saved_fp(tsk)	\
>  	((unsigned long)(tsk->thread.cpu_context.fp))
>  
> +void arch_setup_new_exec(void);
> +#define arch_setup_new_exec     arch_setup_new_exec

I'm fine with out of line implementation, it probably helps with any
header conflicts (and it's not a fast path anyway).

> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 659ae8094ed5..ebca9e4f62c7 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -417,3 +417,20 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
>  	else
>  		return randomize_page(mm->brk, SZ_1G);
>  }
> +
> +/*
> + * Called immediately after a successful exec.
> + */
> +void arch_setup_new_exec(void)
> +{
> +	current->mm->context.flags = 0;
> +
> +	/*
> +	 * Unlike the native one, the compat version of exec() inherits
> +	 * READ_IMPLIES_EXEC since this is the behaviour on arch/arm/.
> +	 */
> +	if (is_compat_task())
> +		__set_bit(MMCF_AARCH32, &current->mm->context.flags);
> +	else
> +		current->personality &= ~READ_IMPLIES_EXEC;
> +}

As I said above, just context.flags |= MMCF_AARCH32.

Thanks.

-- 
Catalin

[PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

[Yury Norov]

Originally {COMPAT_,}SET_PERSONALITY() only sets the 32-bit flag in thread_info
structure. But there is some work that should be done after setting the personality.
Currently it's done in the macro, which is not the best idea.

In this patch new arch_setup_new_exec() routine is introduced, and all setup code
is moved there, as suggested by Catalin:
https://lkml.org/lkml/2017/8/4/494

Note: mm->context.flags doesn't require the atomic strong ordered acceess to the
field, so use __set_bit() there;

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
CC: Pratyush Anand <panand@redhat.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/elf.h         | 15 +++++++++------
 arch/arm64/include/asm/thread_info.h |  3 +++
 arch/arm64/kernel/process.c          | 17 +++++++++++++++++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index de11ed1484e3..615953243961 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -137,11 +137,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
  */
 #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
 
+/*
+ * Don't modify this macro unless you add new personality.
+ * All personality-related setup should be done at proper place.
+ * If not sure, consider the arch_setup_new_exec() function.
+ */
 #define SET_PERSONALITY(ex)						\
 ({									\
-	clear_bit(MMCF_AARCH32, &current->mm->context.flags);		\
 	clear_thread_flag(TIF_32BIT);					\
-	current->personality &= ~READ_IMPLIES_EXEC;			\
 })
 
 /* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT entries changes */
@@ -188,14 +191,14 @@ typedef compat_elf_greg_t		compat_elf_gregset_t[COMPAT_ELF_NGREG];
 					 ((x)->e_flags & EF_ARM_EABI_MASK))
 
 #define compat_start_thread		compat_start_thread
+
 /*
- * Unlike the native SET_PERSONALITY macro, the compat version inherits
- * READ_IMPLIES_EXEC across a fork() since this is the behaviour on
- * arch/arm/.
+ * Don't modify this macro unless you add new personality.
+ * All personality-related setup should be done at proper place.
+ * If not sure, consider the arch_setup_new_exec() function.
  */
 #define COMPAT_SET_PERSONALITY(ex)					\
 ({									\
-	set_bit(MMCF_AARCH32, &current->mm->context.flags);		\
 	set_thread_flag(TIF_32BIT);					\
  })
 #define COMPAT_ARCH_DLINFO
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93cf865..c823d2f12b4c 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -68,6 +68,9 @@ struct thread_info {
 #define thread_saved_fp(tsk)	\
 	((unsigned long)(tsk->thread.cpu_context.fp))
 
+void arch_setup_new_exec(void);
+#define arch_setup_new_exec     arch_setup_new_exec
+
 #endif
 
 /*
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 659ae8094ed5..ebca9e4f62c7 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -417,3 +417,20 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
 	else
 		return randomize_page(mm->brk, SZ_1G);
 }
+
+/*
+ * Called immediately after a successful exec.
+ */
+void arch_setup_new_exec(void)
+{
+	current->mm->context.flags = 0;
+
+	/*
+	 * Unlike the native one, the compat version of exec() inherits
+	 * READ_IMPLIES_EXEC since this is the behaviour on arch/arm/.
+	 */
+	if (is_compat_task())
+		__set_bit(MMCF_AARCH32, &current->mm->context.flags);
+	else
+		current->personality &= ~READ_IMPLIES_EXEC;
+}
-- 
2.11.0

[PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro

[Yury Norov]

Originally {COMPAT_,}SET_PERSONALITY() only sets the 32-bit flag in thread_info
structure. But there is some work that should be done after setting the personality.
Currently it's done in the macro, which is not the best idea.

In this patch new arch_setup_new_exec() routine is introduced, and all setup code
is moved there, as suggested by Catalin:
https://lkml.org/lkml/2017/8/4/494

Note: mm->context.flags doesn't require the atomic strong ordered acceess to the
field, so use __set_bit() there;

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
CC: Pratyush Anand <panand@redhat.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/elf.h         | 15 +++++++++------
 arch/arm64/include/asm/thread_info.h |  3 +++
 arch/arm64/kernel/process.c          | 17 +++++++++++++++++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index de11ed1484e3..615953243961 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -137,11 +137,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
  */
 #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
 
+/*
+ * Don't modify this macro unless you add new personality.
+ * All personality-related setup should be done at proper place.
+ * If not sure, consider the arch_setup_new_exec() function.
+ */
 #define SET_PERSONALITY(ex)						\
 ({									\
-	clear_bit(MMCF_AARCH32, &current->mm->context.flags);		\
 	clear_thread_flag(TIF_32BIT);					\
-	current->personality &= ~READ_IMPLIES_EXEC;			\
 })
 
 /* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT entries changes */
@@ -188,14 +191,14 @@ typedef compat_elf_greg_t		compat_elf_gregset_t[COMPAT_ELF_NGREG];
 					 ((x)->e_flags & EF_ARM_EABI_MASK))
 
 #define compat_start_thread		compat_start_thread
+
 /*
- * Unlike the native SET_PERSONALITY macro, the compat version inherits
- * READ_IMPLIES_EXEC across a fork() since this is the behaviour on
- * arch/arm/.
+ * Don't modify this macro unless you add new personality.
+ * All personality-related setup should be done at proper place.
+ * If not sure, consider the arch_setup_new_exec() function.
  */
 #define COMPAT_SET_PERSONALITY(ex)					\
 ({									\
-	set_bit(MMCF_AARCH32, &current->mm->context.flags);		\
 	set_thread_flag(TIF_32BIT);					\
  })
 #define COMPAT_ARCH_DLINFO
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93cf865..c823d2f12b4c 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -68,6 +68,9 @@ struct thread_info {
 #define thread_saved_fp(tsk)	\
 	((unsigned long)(tsk->thread.cpu_context.fp))
 
+void arch_setup_new_exec(void);
+#define arch_setup_new_exec     arch_setup_new_exec
+
 #endif
 
 /*
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 659ae8094ed5..ebca9e4f62c7 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -417,3 +417,20 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
 	else
 		return randomize_page(mm->brk, SZ_1G);
 }
+
+/*
+ * Called immediately after a successful exec.
+ */
+void arch_setup_new_exec(void)
+{
+	current->mm->context.flags = 0;
+
+	/*
+	 * Unlike the native one, the compat version of exec() inherits
+	 * READ_IMPLIES_EXEC since this is the behaviour on arch/arm/.
+	 */
+	if (is_compat_task())
+		__set_bit(MMCF_AARCH32, &current->mm->context.flags);
+	else
+		current->personality &= ~READ_IMPLIES_EXEC;
+}
-- 
2.11.0