[PATCH v2] fs: binfmt_elf_efpic: fix personality for ELF-FDPIC

[PATCH v2] fs: binfmt_elf_efpic: fix personality for ELF-FDPIC

[Greg Ungerer]

The elf-fdpic loader hard sets the process personality to either
PER_LINUX_FDPIC for true elf-fdpic binaries or to PER_LINUX for
normal ELF binaries (in this case they would be constant displacement
compiled with -pie for example). The problem with that is that it
will lose any other bits that may be in the ELF header personality
(such as the "bug emulation" bits).

On the ARM architecture the ADDR_LIMIT_32BIT flag is used to signify
a normal 32bit binary - as opposed to a legacy 26bit address binary.
This matters since start_thread() will set the ARM CPSR register as
required based on this flag. If the elf-fdpic loader loses this bit
the process will be mis-configured and crash out pretty quickly.

Modify elf-fdpic loader personality setting so that it preserves the
upper three bytes by using the SET_PERSONALITY macro to set it. This
macro in the generic case sets PER_LINUX and preserves the upper bytes.
Architectures can override this for their specific use case, and ARM
does exactly this.

The problem shows up quite easily running under qemu using the ARM
architecture, but not necessarily on all types of real ARM hardware.
If the underlying ARM processor does not support the legacy 26-bit
addressing mode then everything will work as expected.

Signed-off-by: Greg Ungerer <gerg@kernel.org>
---
 fs/binfmt_elf_fdpic.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

v2: fixes the elf-fdpic binary case as well

I have done more extensive testing, and in particular on the true
elf-fdpic case. It was broken for that too (as expected).

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 43b2a2851ba3..206812ce544a 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -345,10 +345,9 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 	/* there's now no turning back... the old userspace image is dead,
 	 * defunct, deceased, etc.
 	 */
+	SET_PERSONALITY(exec_params.hdr);
 	if (elf_check_fdpic(&exec_params.hdr))
-		set_personality(PER_LINUX_FDPIC);
-	else
-		set_personality(PER_LINUX);
+		current->personality |= PER_LINUX_FDPIC;
 	if (elf_read_implies_exec(&exec_params.hdr, executable_stack))
 		current->personality |= READ_IMPLIES_EXEC;
 
-- 
2.25.1

Re: [PATCH v2] fs: binfmt_elf_efpic: fix personality for ELF-FDPIC

[Andrew Morton]

On Thu,  7 Sep 2023 11:18:08 +1000 Greg Ungerer <gerg@kernel.org> wrote:

> The elf-fdpic loader hard sets the process personality to either
> PER_LINUX_FDPIC for true elf-fdpic binaries or to PER_LINUX for
> normal ELF binaries (in this case they would be constant displacement
> compiled with -pie for example). The problem with that is that it
> will lose any other bits that may be in the ELF header personality
> (such as the "bug emulation" bits).
> 
> On the ARM architecture the ADDR_LIMIT_32BIT flag is used to signify
> a normal 32bit binary - as opposed to a legacy 26bit address binary.
> This matters since start_thread() will set the ARM CPSR register as
> required based on this flag. If the elf-fdpic loader loses this bit
> the process will be mis-configured and crash out pretty quickly.
> 
> Modify elf-fdpic loader personality setting so that it preserves the
> upper three bytes by using the SET_PERSONALITY macro to set it. This
> macro in the generic case sets PER_LINUX and preserves the upper bytes.
> Architectures can override this for their specific use case, and ARM
> does exactly this.
> 
> The problem shows up quite easily running under qemu using the ARM
> architecture, but not necessarily on all types of real ARM hardware.
> If the underlying ARM processor does not support the legacy 26-bit
> addressing mode then everything will work as expected.

I'm thinking

Fixes: 1bde925d23547 ("fs/binfmt_elf_fdpic.c: provide NOMMU loader for regular ELF binaries")
Cc: <stable@vger.kernel.org>

?

Re: [PATCH v2] fs: binfmt_elf_efpic: fix personality for ELF-FDPIC

[Greg Ungerer]

Hi Andrew,

On 18/9/23 05:26, Andrew Morton wrote:
> On Thu,  7 Sep 2023 11:18:08 +1000 Greg Ungerer <gerg@kernel.org> wrote:
> 
>> The elf-fdpic loader hard sets the process personality to either
>> PER_LINUX_FDPIC for true elf-fdpic binaries or to PER_LINUX for
>> normal ELF binaries (in this case they would be constant displacement
>> compiled with -pie for example). The problem with that is that it
>> will lose any other bits that may be in the ELF header personality
>> (such as the "bug emulation" bits).
>>
>> On the ARM architecture the ADDR_LIMIT_32BIT flag is used to signify
>> a normal 32bit binary - as opposed to a legacy 26bit address binary.
>> This matters since start_thread() will set the ARM CPSR register as
>> required based on this flag. If the elf-fdpic loader loses this bit
>> the process will be mis-configured and crash out pretty quickly.
>>
>> Modify elf-fdpic loader personality setting so that it preserves the
>> upper three bytes by using the SET_PERSONALITY macro to set it. This
>> macro in the generic case sets PER_LINUX and preserves the upper bytes.
>> Architectures can override this for their specific use case, and ARM
>> does exactly this.
>>
>> The problem shows up quite easily running under qemu using the ARM
>> architecture, but not necessarily on all types of real ARM hardware.
>> If the underlying ARM processor does not support the legacy 26-bit
>> addressing mode then everything will work as expected.
> 
> I'm thinking
> 
> Fixes: 1bde925d23547 ("fs/binfmt_elf_fdpic.c: provide NOMMU loader for regular ELF binaries")
> Cc: <stable@vger.kernel.org>

Yes, that seems reasonable. It will apply easily, and legitimately fix this
specific issue going back to the original change.

Regards
Greg