SET_PERSONALITY and TASK_SIZE

SET_PERSONALITY and TASK_SIZE

[Heiko Carstens]

Hi Andrew,

while debugging I noticed the following comment in fs/binfmt_elf.c:

			/*
			 * 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.
			 *
			 * However, SET_PERSONALITY is NOT allowed to switch
			 * this task into the new images's memory mapping
			 * policy - that is, TASK_SIZE must still evaluate to
			 * that which is appropriate to the execing application.
			 * This is because exit_mmap() needs to have TASK_SIZE
			 * evaluate to the size of the old image.
			 *
			 * So if (say) a 64-bit application is execing a 32-bit
			 * application it is the architecture's responsibility
			 * to defer changing the value of TASK_SIZE until the
			 * switch really is going to happen - do this in
			 * flush_thread().	- akpm
			 */

At least s390 isn't doing the deferred TASK_SIZE switch. Also it seems like
MIPS, PARISC and IA64 don't do it either. However from a quick a view I
couldn't see that exit_mmap depends on TASK_SIZE. So is this still necessary?

And the bug I was looking for is this one: in SET_PERSONALITY we do this:

	if (current->personality != PER_LINUX32)
		set_personality(PER_LINUX);

However we should use the PER_MASK if we want to check for PER_LINUX32,
since there are more bits in the personality flags. In case any of the
'extra' bits is set we may incorrectly set personality to PER_LINUX even
when we want PER_LINUX32.

Looks like more architectures should do something like:

	if (personality(current->personality) != PER_LINUX32)
		...

Re: SET_PERSONALITY and TASK_SIZE

[Andrew Morton]

> On Sun, 18 Jan 2009 12:18:31 +0100 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> Hi Andrew,
> 
> while debugging I noticed the following comment in fs/binfmt_elf.c:
> 
> 			/*
> 			 * 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.
> 			 *
> 			 * However, SET_PERSONALITY is NOT allowed to switch
> 			 * this task into the new images's memory mapping
> 			 * policy - that is, TASK_SIZE must still evaluate to
> 			 * that which is appropriate to the execing application.
> 			 * This is because exit_mmap() needs to have TASK_SIZE
> 			 * evaluate to the size of the old image.
> 			 *
> 			 * So if (say) a 64-bit application is execing a 32-bit
> 			 * application it is the architecture's responsibility
> 			 * to defer changing the value of TASK_SIZE until the
> 			 * switch really is going to happen - do this in
> 			 * flush_thread().	- akpm
> 			 */

Cripes, that must have been a different akpm.

> At least s390 isn't doing the deferred TASK_SIZE switch. Also it seems like
> MIPS, PARISC and IA64 don't do it either. However from a quick a view I
> couldn't see that exit_mmap depends on TASK_SIZE. So is this still necessary?
> 
> And the bug I was looking for is this one: in SET_PERSONALITY we do this:
> 
> 	if (current->personality != PER_LINUX32)
> 		set_personality(PER_LINUX);
> 
> However we should use the PER_MASK if we want to check for PER_LINUX32,
> since there are more bits in the personality flags. In case any of the
> 'extra' bits is set we may incorrectly set personality to PER_LINUX even
> when we want PER_LINUX32.
> 
> Looks like more architectures should do something like:
> 
> 	if (personality(current->personality) != PER_LINUX32)
> 		...

I'm stuck in planes and hotels for the rest of the week and don't have a
git tree to dig through, sorry.  I'd prefer to hold off until next week
before diving into that one.

Re: SET_PERSONALITY and TASK_SIZE

[Benjamin Herrenschmidt]

> However we should use the PER_MASK if we want to check for PER_LINUX32,
> since there are more bits in the personality flags. In case any of the
> 'extra' bits is set we may incorrectly set personality to PER_LINUX even
> when we want PER_LINUX32.
> 
> Looks like more architectures should do something like:
> 
> 	if (personality(current->personality) != PER_LINUX32)

If you do gitk mm/memory.c, and look at the first 2 or 3 commits from
Hugh near the botto,, I -think- they may explain why you no longer see
the use of TASK_SIZE in there, ie, we may have fixed that in 2.6.12...

So from that point of view, it's quite possible that we no longer need
to defer the personality switch, which would allow to simplify things
quite a bit on ppc64, unless there is some -other- reason here...

Cheers,
Ben.