Re: [PATCH] alpha: Fix personality flag propagation across an exec

Hi Arnd,

On Thu, 2025-01-09 at 09:56 +0100, Arnd Bergmann wrote:
> On Thu, Jan 9, 2025, at 09:46, John Paul Adrian Glaubitz wrote:
> > On Thu, 2025-01-09 at 09:43 +0100, Arnd Bergmann wrote:
> > > On Thu, Jan 9, 2025, at 09:01, Arnd Bergmann wrote:
> > > > On Fri, Jan 3, 2025, at 15:01, John Paul Adrian Glaubitz wrote:
> > > > 
> > > > > 
> > > > >  #define SET_PERSONALITY(EX)					\
> > > > > -	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
> > > > > -	   ? PER_LINUX_32BIT : PER_LINUX)
> > > > > +	set_personality((((EX).e_flags & EF_ALPHA_32BIT)	\
> > > > > +	   ? PER_LINUX_32BIT : PER_LINUX) | (current->personality & (~PER_MASK)))
> > > > 
> > > > This looks wrong to me: since ADDR_LIMIT_32BIT is not part of
> > > > PER_MASK, executing a regular binary from a taso binary no longer
> > > > reverts back to the entire 64-bit address space.
> > > > 
> > > > It seems that the behavior on most other architectures changed in 2012
> > > > commit 16f3e95b3209 ("cross-arch: don't corrupt personality flags upon
> > > > exec()").
> > > > 
> > 
> > So, if I understand this correctly, we should just use PER_MASK on alpha
> > for 64-bit executables and allow the bits to be cleared for 32-bit binaries?
> 
> I think ideally the EF_ALPHA_32BIT handling should use TIF_32BIT
> as we do on other architectures, at that point the custom SET_PERSONALITY()
> can be removed in favor of the asm-generic version.

I have thought about that as well but I wasn't sure whether the extra
mangling on alpha was necessary.

> Alternatively this could do something like the arm32 version (note
> that on arm, PER_LINUX_32BIT/ADDR_LIMIT_32BIT means "allow using
> the entire 32-bit address space rather than limiting to 26 bits for
> compatibility", while on alpha it means "use only 31 instead of
> 42 bits for addressing", but the logic can be the same):
> 
>         unsigned int personality = current->personality & ~PER_MASK;
>         /*
>          * APCS-26 is only valid for OABI executables
>          */
>         if ((eflags & EF_ARM_EABI_MASK) == EF_ARM_EABI_UNKNOWN &&
>             (eflags & EF_ARM_APCS_26))
>                 personality &= ~ADDR_LIMIT_32BIT;
>         else
>                 personality |= ADDR_LIMIT_32BIT;
>         set_personality(personality);

So, this would be the 100% correct for alpha then which would not loose
any functionality even for 32-bit binaries?

> In any case, I think we should fix alpha, mips and loongarch at
> the same time, to make sure it doesn't take another decade to
> fix the rest.

If you're willing to fix all three at once, I would be happy to help
with the testing on all three architectures as I have machines for all
of these.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913