[PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support

Eric W. Biederman posted 1 patch 1 year ago
There is a newer version of this series
arch/alpha/include/asm/elf.h       |  6 +-----
arch/alpha/include/asm/pgtable.h   |  2 +-
arch/alpha/include/asm/processor.h |  8 ++------
arch/alpha/kernel/osf_sys.c        | 11 ++---------
4 files changed, 6 insertions(+), 21 deletions(-)
[PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Eric W. Biederman 1 year ago

Richard Henderson <richard.henderson@linaro.org> writes[1]:

> There was a Spec benchmark (I forget which) which was memory bound and ran
> twice as fast with 32-bit pointers.
>
> I copied the idea from DEC to the ELF abi, but never did all the other work
> to allow the toolchain to take advantage.
>
> Amusingly, a later Spec changed the benchmark data sets to not fit into a
> 32-bit address space, specifically because of this.
>
> I expect one could delete the ELF bit and personality and no one would
> notice. Not even the 10 remaining Alpha users.

In [2] it was pointed out that parts of setarch weren't working
properly on alpha because it has it's own SET_PERSONALITY
implementation.  In the discussion that followed Richard Henderson
pointed out that the 32bit pointer support for alpha was never
completed.

Fix this by removing alpha's 32bit pointer support.

As a bit of paranoia refuse to execute any alpha binaries that hafe
the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
somewhere has binaries that trying to use alpha's 32bit pointer
support.

[1] https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
[2] https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/alpha/include/asm/elf.h       |  6 +-----
 arch/alpha/include/asm/pgtable.h   |  2 +-
 arch/alpha/include/asm/processor.h |  8 ++------
 arch/alpha/kernel/osf_sys.c        | 11 ++---------
 4 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
index 4d7c46f50382..50c82187e60e 100644
--- a/arch/alpha/include/asm/elf.h
+++ b/arch/alpha/include/asm/elf.h
@@ -74,7 +74,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
 /*
  * This is used to ensure we don't load something for the wrong architecture.
  */
-#define elf_check_arch(x) ((x)->e_machine == EM_ALPHA)
+#define elf_check_arch(x) (((x)->e_machine == EM_ALPHA) && !((x)->e_flags & EF_ALPHA_32BIT))
 
 /*
  * These are used to set parameters in the core dumps.
@@ -137,10 +137,6 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
 	: amask (AMASK_CIX) ? "ev6" : "ev67");	\
 })
 
-#define SET_PERSONALITY(EX)					\
-	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
-	   ? PER_LINUX_32BIT : PER_LINUX)
-
 extern int alpha_l1i_cacheshape;
 extern int alpha_l1d_cacheshape;
 extern int alpha_l2_cacheshape;
diff --git a/arch/alpha/include/asm/pgtable.h b/arch/alpha/include/asm/pgtable.h
index 635f0a5f5bbd..02e8817a8921 100644
--- a/arch/alpha/include/asm/pgtable.h
+++ b/arch/alpha/include/asm/pgtable.h
@@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
 
 extern void paging_init(void);
 
-/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
+/* We have our own get_unmapped_area */
 #define HAVE_ARCH_UNMAPPED_AREA
 
 #endif /* _ALPHA_PGTABLE_H */
diff --git a/arch/alpha/include/asm/processor.h b/arch/alpha/include/asm/processor.h
index 55bb1c09fd39..5dce5518a211 100644
--- a/arch/alpha/include/asm/processor.h
+++ b/arch/alpha/include/asm/processor.h
@@ -8,23 +8,19 @@
 #ifndef __ASM_ALPHA_PROCESSOR_H
 #define __ASM_ALPHA_PROCESSOR_H
 
-#include <linux/personality.h>	/* for ADDR_LIMIT_32BIT */
-
 /*
  * We have a 42-bit user address space: 4TB user VM...
  */
 #define TASK_SIZE (0x40000000000UL)
 
-#define STACK_TOP \
-  (current->personality & ADDR_LIMIT_32BIT ? 0x80000000 : 0x00120000000UL)
+#define STACK_TOP (0x00120000000UL)
 
 #define STACK_TOP_MAX	0x00120000000UL
 
 /* This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
  */
-#define TASK_UNMAPPED_BASE \
-  ((current->personality & ADDR_LIMIT_32BIT) ? 0x40000000 : TASK_SIZE / 2)
+#define TASK_UNMAPPED_BASE (TASK_SIZE / 2)
 
 /* This is dead.  Everything has been moved to thread_info.  */
 struct thread_struct { };
diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index 86185021f75a..a08e8edef1a4 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
 	return ret;
 }
 
-/* Get an address range which is currently unmapped.  Similar to the
-   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
+/* Get an address range which is currently unmapped. */
 
 static unsigned long
 arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
@@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 		       unsigned long len, unsigned long pgoff,
 		       unsigned long flags, vm_flags_t vm_flags)
 {
-	unsigned long limit;
-
-	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
-	if (current->personality & ADDR_LIMIT_32BIT)
-		limit = 0x80000000;
-	else
-		limit = TASK_SIZE;
+	unsigned long limit = TASK_SIZE;
 
 	if (len > limit)
 		return -ENOMEM;
-- 
2.41.0
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by John Paul Adrian Glaubitz 1 year ago
Hi Eric,

On Fri, 2025-01-10 at 18:16 -0600, Eric W. Biederman wrote:
> Richard Henderson <richard.henderson@linaro.org> writes[1]:
> 
> > There was a Spec benchmark (I forget which) which was memory bound and ran
> > twice as fast with 32-bit pointers.
> > 
> > I copied the idea from DEC to the ELF abi, but never did all the other work
> > to allow the toolchain to take advantage.
> > 
> > Amusingly, a later Spec changed the benchmark data sets to not fit into a
> > 32-bit address space, specifically because of this.
> > 
> > I expect one could delete the ELF bit and personality and no one would
> > notice. Not even the 10 remaining Alpha users.
> 
> In [2] it was pointed out that parts of setarch weren't working
> properly on alpha because it has it's own SET_PERSONALITY
> implementation.  In the discussion that followed Richard Henderson
> pointed out that the 32bit pointer support for alpha was never
> completed.
> 
> Fix this by removing alpha's 32bit pointer support.
> 
> As a bit of paranoia refuse to execute any alpha binaries that hafe
> the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
> somewhere has binaries that trying to use alpha's 32bit pointer
> support.
> 
> [1] https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
> [2] https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  arch/alpha/include/asm/elf.h       |  6 +-----
>  arch/alpha/include/asm/pgtable.h   |  2 +-
>  arch/alpha/include/asm/processor.h |  8 ++------
>  arch/alpha/kernel/osf_sys.c        | 11 ++---------
>  4 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
> index 4d7c46f50382..50c82187e60e 100644
> --- a/arch/alpha/include/asm/elf.h
> +++ b/arch/alpha/include/asm/elf.h
> @@ -74,7 +74,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
>  /*
>   * This is used to ensure we don't load something for the wrong architecture.
>   */
> -#define elf_check_arch(x) ((x)->e_machine == EM_ALPHA)
> +#define elf_check_arch(x) (((x)->e_machine == EM_ALPHA) && !((x)->e_flags & EF_ALPHA_32BIT))
>  
>  /*
>   * These are used to set parameters in the core dumps.
> @@ -137,10 +137,6 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
>  	: amask (AMASK_CIX) ? "ev6" : "ev67");	\
>  })
>  
> -#define SET_PERSONALITY(EX)					\
> -	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
> -	   ? PER_LINUX_32BIT : PER_LINUX)
> -
>  extern int alpha_l1i_cacheshape;
>  extern int alpha_l1d_cacheshape;
>  extern int alpha_l2_cacheshape;
> diff --git a/arch/alpha/include/asm/pgtable.h b/arch/alpha/include/asm/pgtable.h
> index 635f0a5f5bbd..02e8817a8921 100644
> --- a/arch/alpha/include/asm/pgtable.h
> +++ b/arch/alpha/include/asm/pgtable.h
> @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  
>  extern void paging_init(void);
>  
> -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> +/* We have our own get_unmapped_area */
>  #define HAVE_ARCH_UNMAPPED_AREA
>  
>  #endif /* _ALPHA_PGTABLE_H */
> diff --git a/arch/alpha/include/asm/processor.h b/arch/alpha/include/asm/processor.h
> index 55bb1c09fd39..5dce5518a211 100644
> --- a/arch/alpha/include/asm/processor.h
> +++ b/arch/alpha/include/asm/processor.h
> @@ -8,23 +8,19 @@
>  #ifndef __ASM_ALPHA_PROCESSOR_H
>  #define __ASM_ALPHA_PROCESSOR_H
>  
> -#include <linux/personality.h>	/* for ADDR_LIMIT_32BIT */
> -
>  /*
>   * We have a 42-bit user address space: 4TB user VM...
>   */
>  #define TASK_SIZE (0x40000000000UL)
>  
> -#define STACK_TOP \
> -  (current->personality & ADDR_LIMIT_32BIT ? 0x80000000 : 0x00120000000UL)
> +#define STACK_TOP (0x00120000000UL)
>  
>  #define STACK_TOP_MAX	0x00120000000UL
>  
>  /* This decides where the kernel will search for a free chunk of vm
>   * space during mmap's.
>   */
> -#define TASK_UNMAPPED_BASE \
> -  ((current->personality & ADDR_LIMIT_32BIT) ? 0x40000000 : TASK_SIZE / 2)
> +#define TASK_UNMAPPED_BASE (TASK_SIZE / 2)
>  
>  /* This is dead.  Everything has been moved to thread_info.  */
>  struct thread_struct { };
> diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> index 86185021f75a..a08e8edef1a4 100644
> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
>  	return ret;
>  }
>  
> -/* Get an address range which is currently unmapped.  Similar to the
> -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> +/* Get an address range which is currently unmapped. */
>  
>  static unsigned long
>  arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>  		       unsigned long len, unsigned long pgoff,
>  		       unsigned long flags, vm_flags_t vm_flags)
>  {
> -	unsigned long limit;
> -
> -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> -	if (current->personality & ADDR_LIMIT_32BIT)
> -		limit = 0x80000000;
> -	else
> -		limit = TASK_SIZE;
> +	unsigned long limit = TASK_SIZE;
>  
>  	if (len > limit)
>  		return -ENOMEM;

Compile- and boot-tested on QEMU alpha, works as expected:

glaubitz@debian-alpha:~/util-linux$ ./tests/ts/misc/setarch 
         misc: setarch                        ...
                : options                     ...[  110.194279] pid=558, couldn't seal address 0, ret=-12.
 OK
                : uname26                     ... OK
                : uname26-version             ... OK
                : show                        ... OK
           ... OK (all 4 sub-tests PASSED)
glaubitz@debian-alpha:~/util-linux$

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Ivan Kokshaysky 1 year ago
On Fri, Jan 10, 2025 at 06:16:28PM -0600, Eric W. Biederman wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes[1]:
> 
> > There was a Spec benchmark (I forget which) which was memory bound and ran
> > twice as fast with 32-bit pointers.
> >
> > I copied the idea from DEC to the ELF abi, but never did all the other work
> > to allow the toolchain to take advantage.
> >
> > Amusingly, a later Spec changed the benchmark data sets to not fit into a
> > 32-bit address space, specifically because of this.
> >
> > I expect one could delete the ELF bit and personality and no one would
> > notice. Not even the 10 remaining Alpha users.
> 
> In [2] it was pointed out that parts of setarch weren't working
> properly on alpha because it has it's own SET_PERSONALITY
> implementation.  In the discussion that followed Richard Henderson
> pointed out that the 32bit pointer support for alpha was never
> completed.
> 
> Fix this by removing alpha's 32bit pointer support.
> 
> As a bit of paranoia refuse to execute any alpha binaries that hafe
> the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
> somewhere has binaries that trying to use alpha's 32bit pointer
> support.

In general I agree, but then someone ought to remove the "--taso" option
from GNU ld, which produces such binaries.

Ivan.

> [1] https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
> [2] https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  arch/alpha/include/asm/elf.h       |  6 +-----
>  arch/alpha/include/asm/pgtable.h   |  2 +-
>  arch/alpha/include/asm/processor.h |  8 ++------
>  arch/alpha/kernel/osf_sys.c        | 11 ++---------
>  4 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
> index 4d7c46f50382..50c82187e60e 100644
> --- a/arch/alpha/include/asm/elf.h
> +++ b/arch/alpha/include/asm/elf.h
> @@ -74,7 +74,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
>  /*
>   * This is used to ensure we don't load something for the wrong architecture.
>   */
> -#define elf_check_arch(x) ((x)->e_machine == EM_ALPHA)
> +#define elf_check_arch(x) (((x)->e_machine == EM_ALPHA) && !((x)->e_flags & EF_ALPHA_32BIT))
>  
>  /*
>   * These are used to set parameters in the core dumps.
> @@ -137,10 +137,6 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
>  	: amask (AMASK_CIX) ? "ev6" : "ev67");	\
>  })
>  
> -#define SET_PERSONALITY(EX)					\
> -	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
> -	   ? PER_LINUX_32BIT : PER_LINUX)
> -
>  extern int alpha_l1i_cacheshape;
>  extern int alpha_l1d_cacheshape;
>  extern int alpha_l2_cacheshape;
> diff --git a/arch/alpha/include/asm/pgtable.h b/arch/alpha/include/asm/pgtable.h
> index 635f0a5f5bbd..02e8817a8921 100644
> --- a/arch/alpha/include/asm/pgtable.h
> +++ b/arch/alpha/include/asm/pgtable.h
> @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  
>  extern void paging_init(void);
>  
> -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> +/* We have our own get_unmapped_area */
>  #define HAVE_ARCH_UNMAPPED_AREA
>  
>  #endif /* _ALPHA_PGTABLE_H */
> diff --git a/arch/alpha/include/asm/processor.h b/arch/alpha/include/asm/processor.h
> index 55bb1c09fd39..5dce5518a211 100644
> --- a/arch/alpha/include/asm/processor.h
> +++ b/arch/alpha/include/asm/processor.h
> @@ -8,23 +8,19 @@
>  #ifndef __ASM_ALPHA_PROCESSOR_H
>  #define __ASM_ALPHA_PROCESSOR_H
>  
> -#include <linux/personality.h>	/* for ADDR_LIMIT_32BIT */
> -
>  /*
>   * We have a 42-bit user address space: 4TB user VM...
>   */
>  #define TASK_SIZE (0x40000000000UL)
>  
> -#define STACK_TOP \
> -  (current->personality & ADDR_LIMIT_32BIT ? 0x80000000 : 0x00120000000UL)
> +#define STACK_TOP (0x00120000000UL)
>  
>  #define STACK_TOP_MAX	0x00120000000UL
>  
>  /* This decides where the kernel will search for a free chunk of vm
>   * space during mmap's.
>   */
> -#define TASK_UNMAPPED_BASE \
> -  ((current->personality & ADDR_LIMIT_32BIT) ? 0x40000000 : TASK_SIZE / 2)
> +#define TASK_UNMAPPED_BASE (TASK_SIZE / 2)
>  
>  /* This is dead.  Everything has been moved to thread_info.  */
>  struct thread_struct { };
> diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> index 86185021f75a..a08e8edef1a4 100644
> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
>  	return ret;
>  }
>  
> -/* Get an address range which is currently unmapped.  Similar to the
> -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> +/* Get an address range which is currently unmapped. */
>  
>  static unsigned long
>  arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>  		       unsigned long len, unsigned long pgoff,
>  		       unsigned long flags, vm_flags_t vm_flags)
>  {
> -	unsigned long limit;
> -
> -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> -	if (current->personality & ADDR_LIMIT_32BIT)
> -		limit = 0x80000000;
> -	else
> -		limit = TASK_SIZE;
> +	unsigned long limit = TASK_SIZE;
>  
>  	if (len > limit)
>  		return -ENOMEM;
> -- 
> 2.41.0
> 
>
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Eric W. Biederman 1 year ago
Ivan Kokshaysky <ink@unseen.parts> writes:

> On Fri, Jan 10, 2025 at 06:16:28PM -0600, Eric W. Biederman wrote:
>> 
>> Richard Henderson <richard.henderson@linaro.org> writes[1]:
>> 
>> > There was a Spec benchmark (I forget which) which was memory bound and ran
>> > twice as fast with 32-bit pointers.
>> >
>> > I copied the idea from DEC to the ELF abi, but never did all the other work
>> > to allow the toolchain to take advantage.
>> >
>> > Amusingly, a later Spec changed the benchmark data sets to not fit into a
>> > 32-bit address space, specifically because of this.
>> >
>> > I expect one could delete the ELF bit and personality and no one would
>> > notice. Not even the 10 remaining Alpha users.
>> 
>> In [2] it was pointed out that parts of setarch weren't working
>> properly on alpha because it has it's own SET_PERSONALITY
>> implementation.  In the discussion that followed Richard Henderson
>> pointed out that the 32bit pointer support for alpha was never
>> completed.
>> 
>> Fix this by removing alpha's 32bit pointer support.
>> 
>> As a bit of paranoia refuse to execute any alpha binaries that hafe
>> the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
>> somewhere has binaries that trying to use alpha's 32bit pointer
>> support.
>
> In general I agree, but then someone ought to remove the "--taso" option
> from GNU ld, which produces such binaries.

Please feel free to write such a patch.  I don't know the GNU ld process
well enough to write that patch.

It would be good to remove dead code and confusing code from GNU ld,
as well as from the linux kernel.

Eric
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Arnd Bergmann 1 year ago
On Sat, Jan 11, 2025, at 01:16, Eric W. Biederman wrote:
> Richard Henderson <richard.henderson@linaro.org> writes[1]:
>
>> There was a Spec benchmark (I forget which) which was memory bound and ran
>> twice as fast with 32-bit pointers.
>>
>> I copied the idea from DEC to the ELF abi, but never did all the other work
>> to allow the toolchain to take advantage.
>>
>> Amusingly, a later Spec changed the benchmark data sets to not fit into a
>> 32-bit address space, specifically because of this.
>>
>> I expect one could delete the ELF bit and personality and no one would
>> notice. Not even the 10 remaining Alpha users.
>
> In [2] it was pointed out that parts of setarch weren't working
> properly on alpha because it has it's own SET_PERSONALITY
> implementation.  In the discussion that followed Richard Henderson
> pointed out that the 32bit pointer support for alpha was never
> completed.
>
> Fix this by removing alpha's 32bit pointer support.
>
> As a bit of paranoia refuse to execute any alpha binaries that hafe
> the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
> somewhere has binaries that trying to use alpha's 32bit pointer
> support.
>
> [1] 
> https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
> [2] 
> https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by John Paul Adrian Glaubitz 1 year ago
Hi Eric,

On Fri, 2025-01-10 at 18:16 -0600, Eric W. Biederman wrote:
> Richard Henderson <richard.henderson@linaro.org> writes[1]:
> 
> > There was a Spec benchmark (I forget which) which was memory bound and ran
> > twice as fast with 32-bit pointers.
> > 
> > I copied the idea from DEC to the ELF abi, but never did all the other work
> > to allow the toolchain to take advantage.
> > 
> > Amusingly, a later Spec changed the benchmark data sets to not fit into a
> > 32-bit address space, specifically because of this.
> > 
> > I expect one could delete the ELF bit and personality and no one would
> > notice. Not even the 10 remaining Alpha users.
> 
> In [2] it was pointed out that parts of setarch weren't working
> properly on alpha because it has it's own SET_PERSONALITY
> implementation.  In the discussion that followed Richard Henderson
> pointed out that the 32bit pointer support for alpha was never
> completed.
> 
> Fix this by removing alpha's 32bit pointer support.
> 
> As a bit of paranoia refuse to execute any alpha binaries that hafe
                                                                 ^^^^ hafe->have

> the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
> somewhere has binaries that trying to use alpha's 32bit pointer
                            ^^^ are

> support.
> 
> [1] https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
> [2] https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  arch/alpha/include/asm/elf.h       |  6 +-----
>  arch/alpha/include/asm/pgtable.h   |  2 +-
>  arch/alpha/include/asm/processor.h |  8 ++------
>  arch/alpha/kernel/osf_sys.c        | 11 ++---------
>  4 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
> index 4d7c46f50382..50c82187e60e 100644
> --- a/arch/alpha/include/asm/elf.h
> +++ b/arch/alpha/include/asm/elf.h
> @@ -74,7 +74,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
>  /*
>   * This is used to ensure we don't load something for the wrong architecture.
>   */
> -#define elf_check_arch(x) ((x)->e_machine == EM_ALPHA)
> +#define elf_check_arch(x) (((x)->e_machine == EM_ALPHA) && !((x)->e_flags & EF_ALPHA_32BIT))
>  
>  /*
>   * These are used to set parameters in the core dumps.
> @@ -137,10 +137,6 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
>  	: amask (AMASK_CIX) ? "ev6" : "ev67");	\
>  })
>  
> -#define SET_PERSONALITY(EX)					\
> -	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
> -	   ? PER_LINUX_32BIT : PER_LINUX)
> -
>  extern int alpha_l1i_cacheshape;
>  extern int alpha_l1d_cacheshape;
>  extern int alpha_l2_cacheshape;
> diff --git a/arch/alpha/include/asm/pgtable.h b/arch/alpha/include/asm/pgtable.h
> index 635f0a5f5bbd..02e8817a8921 100644
> --- a/arch/alpha/include/asm/pgtable.h
> +++ b/arch/alpha/include/asm/pgtable.h
> @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  
>  extern void paging_init(void);
>  
> -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> +/* We have our own get_unmapped_area */
>  #define HAVE_ARCH_UNMAPPED_AREA
>  
>  #endif /* _ALPHA_PGTABLE_H */
> diff --git a/arch/alpha/include/asm/processor.h b/arch/alpha/include/asm/processor.h
> index 55bb1c09fd39..5dce5518a211 100644
> --- a/arch/alpha/include/asm/processor.h
> +++ b/arch/alpha/include/asm/processor.h
> @@ -8,23 +8,19 @@
>  #ifndef __ASM_ALPHA_PROCESSOR_H
>  #define __ASM_ALPHA_PROCESSOR_H
>  
> -#include <linux/personality.h>	/* for ADDR_LIMIT_32BIT */
> -
>  /*
>   * We have a 42-bit user address space: 4TB user VM...
>   */
>  #define TASK_SIZE (0x40000000000UL)
>  
> -#define STACK_TOP \
> -  (current->personality & ADDR_LIMIT_32BIT ? 0x80000000 : 0x00120000000UL)
> +#define STACK_TOP (0x00120000000UL)
>  
>  #define STACK_TOP_MAX	0x00120000000UL
>  
>  /* This decides where the kernel will search for a free chunk of vm
>   * space during mmap's.
>   */
> -#define TASK_UNMAPPED_BASE \
> -  ((current->personality & ADDR_LIMIT_32BIT) ? 0x40000000 : TASK_SIZE / 2)
> +#define TASK_UNMAPPED_BASE (TASK_SIZE / 2)
>  
>  /* This is dead.  Everything has been moved to thread_info.  */
>  struct thread_struct { };
> diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> index 86185021f75a..a08e8edef1a4 100644
> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
>  	return ret;
>  }
>  
> -/* Get an address range which is currently unmapped.  Similar to the
> -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> +/* Get an address range which is currently unmapped. */
>  
>  static unsigned long
>  arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>  		       unsigned long len, unsigned long pgoff,
>  		       unsigned long flags, vm_flags_t vm_flags)
>  {
> -	unsigned long limit;
> -
> -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> -	if (current->personality & ADDR_LIMIT_32BIT)
> -		limit = 0x80000000;
> -	else
> -		limit = TASK_SIZE;
> +	unsigned long limit = TASK_SIZE;
>  
>  	if (len > limit)
>  		return -ENOMEM;

Looks good to me besides the two spelling mistakes above in the comment.

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Will also test and report back shortly.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Maciej W. Rozycki 1 year ago
On Sat, 11 Jan 2025, John Paul Adrian Glaubitz wrote:

> > the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
> > somewhere has binaries that trying to use alpha's 32bit pointer
>                             ^^^ are

 If nitpicking, I'd say just "try".

  Maciej
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by John Paul Adrian Glaubitz 1 year ago
On Sun, 2025-01-12 at 14:40 +0000, Maciej W. Rozycki wrote:
> On Sat, 11 Jan 2025, John Paul Adrian Glaubitz wrote:
> 
> > > the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
> > > somewhere has binaries that trying to use alpha's 32bit pointer
> >                             ^^^ are
> 
>  If nitpicking, I'd say just "try".

No objection. I was just hinting at obvious grammar mistakes. ;-)

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
[PATCH v2] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Eric W. Biederman 1 year ago

Richard Henderson <richard.henderson@linaro.org> writes[1]:

> There was a Spec benchmark (I forget which) which was memory bound and ran
> twice as fast with 32-bit pointers.
>
> I copied the idea from DEC to the ELF abi, but never did all the other work
> to allow the toolchain to take advantage.
>
> Amusingly, a later Spec changed the benchmark data sets to not fit into a
> 32-bit address space, specifically because of this.
>
> I expect one could delete the ELF bit and personality and no one would
> notice. Not even the 10 remaining Alpha users.

In [2] it was pointed out that parts of setarch weren't working
properly on alpha because it has it's own SET_PERSONALITY
implementation.  In the discussion that followed Richard Henderson
pointed out that the 32bit pointer support for alpha was never
completed.

Fix this by removing alpha's 32bit pointer support.

As a bit of paranoia refuse to execute any alpha binaries that have
the EF_ALPHA_32BIT flag set.  Just in case someone somewhere has
binaries that try to use alpha's 32bit pointer support.

[1] https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
[2] https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
v1: https://lkml.kernel.org/r/87jzb2tdb7.fsf_-_@email.froward.int.ebiederm.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---

Kees can you pick this one up?

 arch/alpha/include/asm/elf.h       |  6 +-----
 arch/alpha/include/asm/pgtable.h   |  2 +-
 arch/alpha/include/asm/processor.h |  8 ++------
 arch/alpha/kernel/osf_sys.c        | 11 ++---------
 4 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
index 4d7c46f50382..50c82187e60e 100644
--- a/arch/alpha/include/asm/elf.h
+++ b/arch/alpha/include/asm/elf.h
@@ -74,7 +74,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
 /*
  * This is used to ensure we don't load something for the wrong architecture.
  */
-#define elf_check_arch(x) ((x)->e_machine == EM_ALPHA)
+#define elf_check_arch(x) (((x)->e_machine == EM_ALPHA) && !((x)->e_flags & EF_ALPHA_32BIT))
 
 /*
  * These are used to set parameters in the core dumps.
@@ -137,10 +137,6 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
 	: amask (AMASK_CIX) ? "ev6" : "ev67");	\
 })
 
-#define SET_PERSONALITY(EX)					\
-	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
-	   ? PER_LINUX_32BIT : PER_LINUX)
-
 extern int alpha_l1i_cacheshape;
 extern int alpha_l1d_cacheshape;
 extern int alpha_l2_cacheshape;
diff --git a/arch/alpha/include/asm/pgtable.h b/arch/alpha/include/asm/pgtable.h
index 635f0a5f5bbd..02e8817a8921 100644
--- a/arch/alpha/include/asm/pgtable.h
+++ b/arch/alpha/include/asm/pgtable.h
@@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
 
 extern void paging_init(void);
 
-/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
+/* We have our own get_unmapped_area */
 #define HAVE_ARCH_UNMAPPED_AREA
 
 #endif /* _ALPHA_PGTABLE_H */
diff --git a/arch/alpha/include/asm/processor.h b/arch/alpha/include/asm/processor.h
index 55bb1c09fd39..5dce5518a211 100644
--- a/arch/alpha/include/asm/processor.h
+++ b/arch/alpha/include/asm/processor.h
@@ -8,23 +8,19 @@
 #ifndef __ASM_ALPHA_PROCESSOR_H
 #define __ASM_ALPHA_PROCESSOR_H
 
-#include <linux/personality.h>	/* for ADDR_LIMIT_32BIT */
-
 /*
  * We have a 42-bit user address space: 4TB user VM...
  */
 #define TASK_SIZE (0x40000000000UL)
 
-#define STACK_TOP \
-  (current->personality & ADDR_LIMIT_32BIT ? 0x80000000 : 0x00120000000UL)
+#define STACK_TOP (0x00120000000UL)
 
 #define STACK_TOP_MAX	0x00120000000UL
 
 /* This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
  */
-#define TASK_UNMAPPED_BASE \
-  ((current->personality & ADDR_LIMIT_32BIT) ? 0x40000000 : TASK_SIZE / 2)
+#define TASK_UNMAPPED_BASE (TASK_SIZE / 2)
 
 /* This is dead.  Everything has been moved to thread_info.  */
 struct thread_struct { };
diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index 86185021f75a..a08e8edef1a4 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
 	return ret;
 }
 
-/* Get an address range which is currently unmapped.  Similar to the
-   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
+/* Get an address range which is currently unmapped. */
 
 static unsigned long
 arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
@@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 		       unsigned long len, unsigned long pgoff,
 		       unsigned long flags, vm_flags_t vm_flags)
 {
-	unsigned long limit;
-
-	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
-	if (current->personality & ADDR_LIMIT_32BIT)
-		limit = 0x80000000;
-	else
-		limit = TASK_SIZE;
+	unsigned long limit = TASK_SIZE;
 
 	if (len > limit)
 		return -ENOMEM;
-- 
2.41.0
Re: [PATCH v2] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Kees Cook 1 year ago
On Sun, 12 Jan 2025 23:39:01 -0600, Eric W. Biederman wrote:
> Richard Henderson <richard.henderson@linaro.org> writes[1]:
> 
> > There was a Spec benchmark (I forget which) which was memory bound and ran
> > twice as fast with 32-bit pointers.
> >
> > I copied the idea from DEC to the ELF abi, but never did all the other work
> > to allow the toolchain to take advantage.
> >
> > Amusingly, a later Spec changed the benchmark data sets to not fit into a
> > 32-bit address space, specifically because of this.
> >
> > I expect one could delete the ELF bit and personality and no one would
> > notice. Not even the 10 remaining Alpha users.
> 
> [...]

Applied to for-next/topic/execve/core, thanks!

[1/1] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
      https://git.kernel.org/kees/c/b029628be267

Take care,

-- 
Kees Cook
Re: [PATCH v2] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by John Paul Adrian Glaubitz 1 year ago
Hi,

On Sun, 2025-01-12 at 23:39 -0600, Eric W. Biederman wrote:
> Richard Henderson <richard.henderson@linaro.org> writes[1]:
> 
> > There was a Spec benchmark (I forget which) which was memory bound and ran
> > twice as fast with 32-bit pointers.
> > 
> > I copied the idea from DEC to the ELF abi, but never did all the other work
> > to allow the toolchain to take advantage.
> > 
> > Amusingly, a later Spec changed the benchmark data sets to not fit into a
> > 32-bit address space, specifically because of this.
> > 
> > I expect one could delete the ELF bit and personality and no one would
> > notice. Not even the 10 remaining Alpha users.
> 
> In [2] it was pointed out that parts of setarch weren't working
> properly on alpha because it has it's own SET_PERSONALITY
> implementation.  In the discussion that followed Richard Henderson
> pointed out that the 32bit pointer support for alpha was never
> completed.
> 
> Fix this by removing alpha's 32bit pointer support.
> 
> As a bit of paranoia refuse to execute any alpha binaries that have
> the EF_ALPHA_32BIT flag set.  Just in case someone somewhere has
> binaries that try to use alpha's 32bit pointer support.
> 
> [1] https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
> [2] https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
> v1: https://lkml.kernel.org/r/87jzb2tdb7.fsf_-_@email.froward.int.ebiederm.org
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> 
> Kees can you pick this one up?
> 
>  arch/alpha/include/asm/elf.h       |  6 +-----
>  arch/alpha/include/asm/pgtable.h   |  2 +-
>  arch/alpha/include/asm/processor.h |  8 ++------
>  arch/alpha/kernel/osf_sys.c        | 11 ++---------
>  4 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
> index 4d7c46f50382..50c82187e60e 100644
> --- a/arch/alpha/include/asm/elf.h
> +++ b/arch/alpha/include/asm/elf.h
> @@ -74,7 +74,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
>  /*
>   * This is used to ensure we don't load something for the wrong architecture.
>   */
> -#define elf_check_arch(x) ((x)->e_machine == EM_ALPHA)
> +#define elf_check_arch(x) (((x)->e_machine == EM_ALPHA) && !((x)->e_flags & EF_ALPHA_32BIT))
>  
>  /*
>   * These are used to set parameters in the core dumps.
> @@ -137,10 +137,6 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
>  	: amask (AMASK_CIX) ? "ev6" : "ev67");	\
>  })
>  
> -#define SET_PERSONALITY(EX)					\
> -	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
> -	   ? PER_LINUX_32BIT : PER_LINUX)
> -
>  extern int alpha_l1i_cacheshape;
>  extern int alpha_l1d_cacheshape;
>  extern int alpha_l2_cacheshape;
> diff --git a/arch/alpha/include/asm/pgtable.h b/arch/alpha/include/asm/pgtable.h
> index 635f0a5f5bbd..02e8817a8921 100644
> --- a/arch/alpha/include/asm/pgtable.h
> +++ b/arch/alpha/include/asm/pgtable.h
> @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  
>  extern void paging_init(void);
>  
> -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> +/* We have our own get_unmapped_area */
>  #define HAVE_ARCH_UNMAPPED_AREA
>  
>  #endif /* _ALPHA_PGTABLE_H */
> diff --git a/arch/alpha/include/asm/processor.h b/arch/alpha/include/asm/processor.h
> index 55bb1c09fd39..5dce5518a211 100644
> --- a/arch/alpha/include/asm/processor.h
> +++ b/arch/alpha/include/asm/processor.h
> @@ -8,23 +8,19 @@
>  #ifndef __ASM_ALPHA_PROCESSOR_H
>  #define __ASM_ALPHA_PROCESSOR_H
>  
> -#include <linux/personality.h>	/* for ADDR_LIMIT_32BIT */
> -
>  /*
>   * We have a 42-bit user address space: 4TB user VM...
>   */
>  #define TASK_SIZE (0x40000000000UL)
>  
> -#define STACK_TOP \
> -  (current->personality & ADDR_LIMIT_32BIT ? 0x80000000 : 0x00120000000UL)
> +#define STACK_TOP (0x00120000000UL)
>  
>  #define STACK_TOP_MAX	0x00120000000UL
>  
>  /* This decides where the kernel will search for a free chunk of vm
>   * space during mmap's.
>   */
> -#define TASK_UNMAPPED_BASE \
> -  ((current->personality & ADDR_LIMIT_32BIT) ? 0x40000000 : TASK_SIZE / 2)
> +#define TASK_UNMAPPED_BASE (TASK_SIZE / 2)
>  
>  /* This is dead.  Everything has been moved to thread_info.  */
>  struct thread_struct { };
> diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> index 86185021f75a..a08e8edef1a4 100644
> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
>  	return ret;
>  }
>  
> -/* Get an address range which is currently unmapped.  Similar to the
> -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> +/* Get an address range which is currently unmapped. */
>  
>  static unsigned long
>  arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>  		       unsigned long len, unsigned long pgoff,
>  		       unsigned long flags, vm_flags_t vm_flags)
>  {
> -	unsigned long limit;
> -
> -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> -	if (current->personality & ADDR_LIMIT_32BIT)
> -		limit = 0x80000000;
> -	else
> -		limit = TASK_SIZE;
> +	unsigned long limit = TASK_SIZE;
>  
>  	if (len > limit)
>  		return -ENOMEM;

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Can someone pick up this patch for v6.14?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Re: [PATCH v2] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Ivan Kokshaysky 1 year ago
On Sun, Jan 12, 2025 at 11:39:01PM -0600, Eric W. Biederman wrote:
...
> --- a/arch/alpha/include/asm/pgtable.h
> +++ b/arch/alpha/include/asm/pgtable.h
> @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  
>  extern void paging_init(void);
>  
> -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> +/* We have our own get_unmapped_area */
>  #define HAVE_ARCH_UNMAPPED_AREA

Just remove the definition. As the comment suggests, the only reason
it exists is ADDR_LIMIT_32BIT, which is gone.

> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
>  	return ret;
>  }
>  
> -/* Get an address range which is currently unmapped.  Similar to the
> -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> +/* Get an address range which is currently unmapped. */
>  
>  static unsigned long
>  arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>  		       unsigned long len, unsigned long pgoff,
>  		       unsigned long flags, vm_flags_t vm_flags)
>  {
> -	unsigned long limit;
> -
> -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> -	if (current->personality & ADDR_LIMIT_32BIT)
> -		limit = 0x80000000;
> -	else
> -		limit = TASK_SIZE;
> +	unsigned long limit = TASK_SIZE;
>  
>  	if (len > limit)
>  		return -ENOMEM;

Likewise, just remove these functions. The generic_get_unmapped_area()
works fine, tested on up1500.

Ivan.
Re: [PATCH v2] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by John Paul Adrian Glaubitz 1 year ago
Hi Eric,

On Sat, 2025-01-18 at 11:35 +0100, Ivan Kokshaysky wrote:
> On Sun, Jan 12, 2025 at 11:39:01PM -0600, Eric W. Biederman wrote:
> ...
> > --- a/arch/alpha/include/asm/pgtable.h
> > +++ b/arch/alpha/include/asm/pgtable.h
> > @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
> >  
> >  extern void paging_init(void);
> >  
> > -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> > +/* We have our own get_unmapped_area */
> >  #define HAVE_ARCH_UNMAPPED_AREA
> 
> Just remove the definition. As the comment suggests, the only reason
> it exists is ADDR_LIMIT_32BIT, which is gone.
> 
> > --- a/arch/alpha/kernel/osf_sys.c
> > +++ b/arch/alpha/kernel/osf_sys.c
> > @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
> >  	return ret;
> >  }
> >  
> > -/* Get an address range which is currently unmapped.  Similar to the
> > -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> > +/* Get an address range which is currently unmapped. */
> >  
> >  static unsigned long
> >  arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> > @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> >  		       unsigned long len, unsigned long pgoff,
> >  		       unsigned long flags, vm_flags_t vm_flags)
> >  {
> > -	unsigned long limit;
> > -
> > -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> > -	if (current->personality & ADDR_LIMIT_32BIT)
> > -		limit = 0x80000000;
> > -	else
> > -		limit = TASK_SIZE;
> > +	unsigned long limit = TASK_SIZE;
> >  
> >  	if (len > limit)
> >  		return -ENOMEM;
> 
> Likewise, just remove these functions. The generic_get_unmapped_area()
> works fine, tested on up1500.

Can you send a follow-up integrating those changes? It would be good if
SET_PERSONALITY() could be fixed on alpha for v6.14.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Re: [PATCH v2] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Ivan Kokshaysky 1 year ago
On Sun, Jan 26, 2025 at 06:15:43PM +0100, John Paul Adrian Glaubitz wrote:
> Hi Eric,
> 
> On Sat, 2025-01-18 at 11:35 +0100, Ivan Kokshaysky wrote:
> > On Sun, Jan 12, 2025 at 11:39:01PM -0600, Eric W. Biederman wrote:
> > ...
> > > --- a/arch/alpha/include/asm/pgtable.h
> > > +++ b/arch/alpha/include/asm/pgtable.h
> > > @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
> > >  
> > >  extern void paging_init(void);
> > >  
> > > -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> > > +/* We have our own get_unmapped_area */
> > >  #define HAVE_ARCH_UNMAPPED_AREA
> > 
> > Just remove the definition. As the comment suggests, the only reason
> > it exists is ADDR_LIMIT_32BIT, which is gone.
> > 
> > > --- a/arch/alpha/kernel/osf_sys.c
> > > +++ b/arch/alpha/kernel/osf_sys.c
> > > @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
> > >  	return ret;
> > >  }
> > >  
> > > -/* Get an address range which is currently unmapped.  Similar to the
> > > -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> > > +/* Get an address range which is currently unmapped. */
> > >  
> > >  static unsigned long
> > >  arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> > > @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> > >  		       unsigned long len, unsigned long pgoff,
> > >  		       unsigned long flags, vm_flags_t vm_flags)
> > >  {
> > > -	unsigned long limit;
> > > -
> > > -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> > > -	if (current->personality & ADDR_LIMIT_32BIT)
> > > -		limit = 0x80000000;
> > > -	else
> > > -		limit = TASK_SIZE;
> > > +	unsigned long limit = TASK_SIZE;
> > >  
> > >  	if (len > limit)
> > >  		return -ENOMEM;
> > 
> > Likewise, just remove these functions. The generic_get_unmapped_area()
> > works fine, tested on up1500.
> 
> Can you send a follow-up integrating those changes? It would be good if
> SET_PERSONALITY() could be fixed on alpha for v6.14.

Oh, the changes I proposed are mere cleanup suggestions.
The original patch would do just fine.

Ivan.
Re: [PATCH] alpha/elf: Fix misc/setarch test of util-linux by removing 32bit support
Posted by Richard Henderson 1 year ago
On 1/10/25 16:16, Eric W. Biederman wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes[1]:
> 
>> There was a Spec benchmark (I forget which) which was memory bound and ran
>> twice as fast with 32-bit pointers.
>>
>> I copied the idea from DEC to the ELF abi, but never did all the other work
>> to allow the toolchain to take advantage.
>>
>> Amusingly, a later Spec changed the benchmark data sets to not fit into a
>> 32-bit address space, specifically because of this.
>>
>> I expect one could delete the ELF bit and personality and no one would
>> notice. Not even the 10 remaining Alpha users.
> 
> In [2] it was pointed out that parts of setarch weren't working
> properly on alpha because it has it's own SET_PERSONALITY
> implementation.  In the discussion that followed Richard Henderson
> pointed out that the 32bit pointer support for alpha was never
> completed.
> 
> Fix this by removing alpha's 32bit pointer support.
> 
> As a bit of paranoia refuse to execute any alpha binaries that hafe
> the EF_ALPHA_32BIT flag set.  Just to fail explicitly in case someone
> somewhere has binaries that trying to use alpha's 32bit pointer
> support.
> 
> [1] https://lkml.kernel.org/r/CAFXwXrkgu=4Qn-v1PjnOR4SG0oUb9LSa0g6QXpBq4ttm52pJOQ@mail.gmail.com
> [2] https://lkml.kernel.org/r/20250103140148.370368-1-glaubitz@physik.fu-berlin.de
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Thanks for cleaning this up.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


> ---
>   arch/alpha/include/asm/elf.h       |  6 +-----
>   arch/alpha/include/asm/pgtable.h   |  2 +-
>   arch/alpha/include/asm/processor.h |  8 ++------
>   arch/alpha/kernel/osf_sys.c        | 11 ++---------
>   4 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
> index 4d7c46f50382..50c82187e60e 100644
> --- a/arch/alpha/include/asm/elf.h
> +++ b/arch/alpha/include/asm/elf.h
> @@ -74,7 +74,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
>   /*
>    * This is used to ensure we don't load something for the wrong architecture.
>    */
> -#define elf_check_arch(x) ((x)->e_machine == EM_ALPHA)
> +#define elf_check_arch(x) (((x)->e_machine == EM_ALPHA) && !((x)->e_flags & EF_ALPHA_32BIT))
>   
>   /*
>    * These are used to set parameters in the core dumps.
> @@ -137,10 +137,6 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
>   	: amask (AMASK_CIX) ? "ev6" : "ev67");	\
>   })
>   
> -#define SET_PERSONALITY(EX)					\
> -	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
> -	   ? PER_LINUX_32BIT : PER_LINUX)
> -
>   extern int alpha_l1i_cacheshape;
>   extern int alpha_l1d_cacheshape;
>   extern int alpha_l2_cacheshape;
> diff --git a/arch/alpha/include/asm/pgtable.h b/arch/alpha/include/asm/pgtable.h
> index 635f0a5f5bbd..02e8817a8921 100644
> --- a/arch/alpha/include/asm/pgtable.h
> +++ b/arch/alpha/include/asm/pgtable.h
> @@ -360,7 +360,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>   
>   extern void paging_init(void);
>   
> -/* We have our own get_unmapped_area to cope with ADDR_LIMIT_32BIT.  */
> +/* We have our own get_unmapped_area */
>   #define HAVE_ARCH_UNMAPPED_AREA
>   
>   #endif /* _ALPHA_PGTABLE_H */
> diff --git a/arch/alpha/include/asm/processor.h b/arch/alpha/include/asm/processor.h
> index 55bb1c09fd39..5dce5518a211 100644
> --- a/arch/alpha/include/asm/processor.h
> +++ b/arch/alpha/include/asm/processor.h
> @@ -8,23 +8,19 @@
>   #ifndef __ASM_ALPHA_PROCESSOR_H
>   #define __ASM_ALPHA_PROCESSOR_H
>   
> -#include <linux/personality.h>	/* for ADDR_LIMIT_32BIT */
> -
>   /*
>    * We have a 42-bit user address space: 4TB user VM...
>    */
>   #define TASK_SIZE (0x40000000000UL)
>   
> -#define STACK_TOP \
> -  (current->personality & ADDR_LIMIT_32BIT ? 0x80000000 : 0x00120000000UL)
> +#define STACK_TOP (0x00120000000UL)
>   
>   #define STACK_TOP_MAX	0x00120000000UL
>   
>   /* This decides where the kernel will search for a free chunk of vm
>    * space during mmap's.
>    */
> -#define TASK_UNMAPPED_BASE \
> -  ((current->personality & ADDR_LIMIT_32BIT) ? 0x40000000 : TASK_SIZE / 2)
> +#define TASK_UNMAPPED_BASE (TASK_SIZE / 2)
>   
>   /* This is dead.  Everything has been moved to thread_info.  */
>   struct thread_struct { };
> diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> index 86185021f75a..a08e8edef1a4 100644
> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1210,8 +1210,7 @@ SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p)
>   	return ret;
>   }
>   
> -/* Get an address range which is currently unmapped.  Similar to the
> -   generic version except that we know how to honor ADDR_LIMIT_32BIT.  */
> +/* Get an address range which is currently unmapped. */
>   
>   static unsigned long
>   arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> @@ -1230,13 +1229,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>   		       unsigned long len, unsigned long pgoff,
>   		       unsigned long flags, vm_flags_t vm_flags)
>   {
> -	unsigned long limit;
> -
> -	/* "32 bit" actually means 31 bit, since pointers sign extend.  */
> -	if (current->personality & ADDR_LIMIT_32BIT)
> -		limit = 0x80000000;
> -	else
> -		limit = TASK_SIZE;
> +	unsigned long limit = TASK_SIZE;
>   
>   	if (len > limit)
>   		return -ENOMEM;