All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: fix current_thread_info()->addr_limit setup
Date: Fri, 13 May 2016 10:24:20 +0100	[thread overview]
Message-ID: <20160513092419.GA13689@arm.com> (raw)
In-Reply-To: <1463052414-29033-1-git-send-email-ynorov@caviumnetworks.com>

On Thu, May 12, 2016 at 02:26:54PM +0300, Yury Norov wrote:
> At elf loading in flush_old_exec() in fs/exec.c, generic code sets
> current_thread_info()->addr_limit to one that corresponds aarch64 value,
> and ignores compat mode there as corresponding status setup happens
> later on in load_elf_binary() by SET_PERSONALITY() macro. As result,
> compat task has wrong addr_limit, and it may cause various bugs.
> 
> This patch fixes it. It also fixes USER_DS macro to return different
> values depending on compat at runtime.
> 
> It was discovered during ilp32 development. See details here:
> https://lkml.org/lkml/2016/5/11/975

It looks like that thread is ongoing, with discussion around a generic
fix and a related issue with our TLS handling:

  https://lkml.org/lkml/2016/5/12/513
  https://lkml.org/lkml/2016/5/12/515

so I'm going to hold off on this until there's some agreement on the
right way forward. Whatever we end up doing, we should probably cc
stable too.

> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> ---
>  arch/arm64/include/asm/elf.h     | 11 +++++++++--
>  arch/arm64/include/asm/uaccess.h |  2 +-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index 24ed037..fda75ce 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -138,7 +138,10 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>   */
>  #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
>  
> -#define SET_PERSONALITY(ex)		clear_thread_flag(TIF_32BIT);
> +#define SET_PERSONALITY(ex) do {					\
> +	clear_thread_flag(TIF_32BIT);					\
> +	set_fs(TASK_SIZE_64);						\

You could just use USER_DS for both of the SET_PERSONALITY macros.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Yury Norov <ynorov@caviumnetworks.com>
Cc: arnd@arndb.de, catalin.marinas@arm.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: fix current_thread_info()->addr_limit setup
Date: Fri, 13 May 2016 10:24:20 +0100	[thread overview]
Message-ID: <20160513092419.GA13689@arm.com> (raw)
In-Reply-To: <1463052414-29033-1-git-send-email-ynorov@caviumnetworks.com>

On Thu, May 12, 2016 at 02:26:54PM +0300, Yury Norov wrote:
> At elf loading in flush_old_exec() in fs/exec.c, generic code sets
> current_thread_info()->addr_limit to one that corresponds aarch64 value,
> and ignores compat mode there as corresponding status setup happens
> later on in load_elf_binary() by SET_PERSONALITY() macro. As result,
> compat task has wrong addr_limit, and it may cause various bugs.
> 
> This patch fixes it. It also fixes USER_DS macro to return different
> values depending on compat at runtime.
> 
> It was discovered during ilp32 development. See details here:
> https://lkml.org/lkml/2016/5/11/975

It looks like that thread is ongoing, with discussion around a generic
fix and a related issue with our TLS handling:

  https://lkml.org/lkml/2016/5/12/513
  https://lkml.org/lkml/2016/5/12/515

so I'm going to hold off on this until there's some agreement on the
right way forward. Whatever we end up doing, we should probably cc
stable too.

> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> ---
>  arch/arm64/include/asm/elf.h     | 11 +++++++++--
>  arch/arm64/include/asm/uaccess.h |  2 +-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index 24ed037..fda75ce 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -138,7 +138,10 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>   */
>  #define ELF_PLAT_INIT(_r, load_addr)	(_r)->regs[0] = 0
>  
> -#define SET_PERSONALITY(ex)		clear_thread_flag(TIF_32BIT);
> +#define SET_PERSONALITY(ex) do {					\
> +	clear_thread_flag(TIF_32BIT);					\
> +	set_fs(TASK_SIZE_64);						\

You could just use USER_DS for both of the SET_PERSONALITY macros.

Will

  reply	other threads:[~2016-05-13  9:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-12 11:26 [PATCH] arm64: fix current_thread_info()->addr_limit setup Yury Norov
2016-05-12 11:26 ` Yury Norov
2016-05-13  9:24 ` Will Deacon [this message]
2016-05-13  9:24   ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160513092419.GA13689@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.