Re: Split 'flush_old_exec' into two functions - 221af7f87b97431e3ee21ce4b0e77d5411cf1549

From: H. Peter Anvin
Date: Tue Feb 02 2010 - 16:45:52 EST


On 02/02/2010 07:50 AM, Linus Torvalds wrote:
>
> Normally, that would happen, but this patch got applied early _literally_
> because I wanted it to hit -rc6 rather than wait any longer. So it had
> only a day or two of discussion, and probably just a few hours from the
> final version.
>
> That said, I think I may have found the cause.
>
> Peter: look at setup_new_exec(), and realize that it got moved _down_ to
> after all the personality setting. So far, so good, that was the
> intention, but look at what it does:
>
> current->flags &= ~PF_RANDOMIZE;
>
> and look at how fs/binfmt_elf.c does it not just after the personality
> setting, but also after
>
> if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space)
> current->flags |= PF_RANDOMIZE;
>
> so it looks like you may have moved it down too much.
>

Yes, not entirely surprising.

> I think you did that because you wanted to do that
>
> arch_pick_mmap_layout(current->mm);
>
> in setup_new_exec(). Which makes total sense, but it all means that the
> whole preparatory patch did way more than my initial one (which put
> setup_new_exec() right after flush_old_exec())

Yes, that was the intention, and I did specify that I had folded in my
(previously posted as a separate patch) changes; the intent was to avoid
a bisect hole. I didn't describe it well because of the rush, though.

> Michael, mind trying this (UNTESTED!) patch? It makes conceptual sense,
> and moves some more of the flushing of the old process state up to
> "flush_old_exec()" rather than doing it late in "setup_new_exec()".
>
> (I suspect we should also move the signal/fd flushing there, but I doubt
> it matters)

Quite.

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/