failed exec() leaves caller with incorrect personality

[Michael Davidson <md@google.com>]

This problem was initially seen on 2.6.26 x86_64 and has been verified
to still exist in 2.6.32.1.

When attempting to exec() a dynamic linked ELF binary, load_elf_binary()
calls the architecture specific SET_PERSONALITY() before attempting
to open the ELF interpreter for that binary. If the open fails or if the sanity
checks fail then the exec fails before the point of no return and returns
an error to the caller. However, depending on exactly what SET_PERSONALITY()
has done, it may already have changed some aspects of the current
personality and not restored them.

The specific issue that we saw on x86_64 involved a 32 bit binary attempting
to exec a 64 bit binary for which the ELF interpreter was missing. In this case
SET_PERSONALITY() maps to set_personality_64bit() in x86/kernel/process_64.c
which immediately clears the TIF_IA32 flag meaning that after the exec fails
we are still running in a 32 bit process but TIF_IA32 is no longer set.

It seems that this could be fixed by having the x86 SET_PERSONALITY()
use the TIF_ABI_PENDING flag to signal to flush_thread() that TIF_IA32
needed to be cleared, but to be strictly correct it would still be necessary
to find a way of restoring the state of READ_IMPLES_EXEC correctly in
current->personality if the exec fails before the point of no return.

Also it isn't clear to me exactly what some other architecture might want
to do in SET_PERSONALITY() or how they should go about undoing it
in the event that the exec() fails early enough to be able to return an error.

The comments in binfmt_elf.c say, for example:

 /*
  * The early SET_PERSONALITY here is so that the lookup
  * for the interpreter happens in the namespace of the
  * to-be-execed image.  SET_PERSONALITY can select an
  * alternate root.

although it doesn't appear that anyone currently does that.

For x86 I am inclined to just modify SET_PERSONALITY() so that
the clearing of TIF_IA32 doesn't happen until we get to flush_thread()
but wondered if anyone had any thoughts on this.

md