Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Dmitry Safonov <dsafonov@virtuozzo.com>]

On 03/28/2017 03:51 PM, Thomas Gleixner wrote:
> On Tue, 28 Mar 2017, Dmitry Safonov wrote:
>> On 03/22/2017 01:21 AM, Thomas Gleixner wrote:
>>> On Tue, 21 Mar 2017, Dmitry Safonov wrote:
>>>> v3:
>>>> - clear x32 syscall flag during x32 -> x86-64 exec() (thanks, HPA).
>>>
>>> For correctness sake, this wants to be cleared in the IA32 path as
>>> well. It's not causing any harm, but ....
>>>
>>> I'll amend the patch.
>>
>> So, just a gentle reminder about this problem.
>> Should I resend v4 with clearing x32 bit in ia32 path?
>> Or should I resend with this fixup:
>> https://lkml.org/lkml/2017/3/22/343
>>
>> The fixup doesn't look as simple as clearing x32 syscall bit, but I may
>> be wrong.
>
> Something like the below should set it correctly for all possible
> scenarios.

Ok, I'll check the ifdeffery, define __NR_{x32_,ia32_}execve,
test it and resend v4 today or tomorrow.
Thanks.

>
> Thanks,
>
> 	tglx
>
> 8<------------------
>
>  arch/x86/kernel/process_64.c |   63 ++++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 21 deletions(-)
>
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -494,6 +494,8 @@ void set_personality_64bit(void)
>  	clear_thread_flag(TIF_IA32);
>  	clear_thread_flag(TIF_ADDR32);
>  	clear_thread_flag(TIF_X32);
> +	/* Pretend that this comes from a 64bit execve */
> +	task_pt_regs(current)->orig_ax = __NR_execve;
>
>  	/* Ensure the corresponding mm is not marked. */
>  	if (current->mm)
> @@ -506,32 +508,51 @@ void set_personality_64bit(void)
>  	current->personality &= ~READ_IMPLIES_EXEC;
>  }
>
> -void set_personality_ia32(bool x32)
> +static void __set_personality_x32(void)
> +{
> +#ifdef CONFIG_X86_X32
> +	clear_thread_flag(TIF_IA32);
> +	set_thread_flag(TIF_X32);
> +	if (current->mm)
> +		current->mm->context.ia32_compat = TIF_X32;
> +	current->personality &= ~READ_IMPLIES_EXEC;
> +	/*
> +	 * in_compat_syscall() uses the presence of the x32
> +	 * syscall bit flag to determine compat status.
> +	 * The x86 mmap() code relies on the syscall bitness
> +	 * so set x32 syscall bit right here to make
> +	 * in_compat_syscall() work during exec().
> +	 *
> +	 * Pretend to come from a x32 execve.
> +	 */
> +	task_pt_regs(current)->orig_ax = __NR_x32_execve | __X32_SYSCALL_BIT;
> +	current->thread.status &= ~TS_COMPAT;
> +#endif
> +}
> +
> +static void __set_personality_ia32(void)
>  {
> -	/* inherit personality from parent */
> +#ifdef CONFIG_COMPAT_32
> +	set_thread_flag(TIF_IA32);
> +	clear_thread_flag(TIF_X32);
> +	if (current->mm)
> +		current->mm->context.ia32_compat = TIF_IA32;
> +	current->personality |= force_personality32;
> +	/* Prepare the first "return" to user space */
> +	task_pt_regs(current)->orig_ax = __NR_ia32_execve;
> +	current->thread.status |= TS_COMPAT;
> +#endif
> +}
>
> +void set_personality_ia32(bool x32)
> +{
>  	/* Make sure to be in 32bit mode */
>  	set_thread_flag(TIF_ADDR32);
>
> -	/* Mark the associated mm as containing 32-bit tasks. */
> -	if (x32) {
> -		clear_thread_flag(TIF_IA32);
> -		set_thread_flag(TIF_X32);
> -		if (current->mm)
> -			current->mm->context.ia32_compat = TIF_X32;
> -		current->personality &= ~READ_IMPLIES_EXEC;
> -		/* in_compat_syscall() uses the presence of the x32
> -		   syscall bit flag to determine compat status */
> -		current->thread.status &= ~TS_COMPAT;
> -	} else {
> -		set_thread_flag(TIF_IA32);
> -		clear_thread_flag(TIF_X32);
> -		if (current->mm)
> -			current->mm->context.ia32_compat = TIF_IA32;
> -		current->personality |= force_personality32;
> -		/* Prepare the first "return" to user space */
> -		current->thread.status |= TS_COMPAT;
> -	}
> +	if (x32)
> +		__set_personality_x32();
> +	else
> +		__set_personality_ia32();
>  }
>  EXPORT_SYMBOL_GPL(set_personality_ia32);
>
>


-- 
              Dmitry