Linus Torvalds wrote:> > On Tue, 2 Feb 2010, Michal Simek wrote:>> Would it be possible to cc me or send that patches to linux-next? I am doing>> every day tests and report results on my site. I would be able to catch up>> bugs earlier.> > 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.

ok. I just wanted to be sure.

> > 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.> > 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())> > In fact, it looks like PF_RANDOMIZE never gets set with the new code, but > I didn't check if it might not happen somewhere else.> > But while I doubt that clearing PF_RANDOMIZE will break anything, the > movement also affects other thigns. Lookie here:> > if (elf_read_implies_exec(loc->elf_ex, executable_stack))> current->personality |= READ_IMPLIES_EXEC;> > also happens before setup_new_exec(), and then setup_new_exec() does> > current->personality &= ~bprm->per_clear;> > where that per_clear mask may be PER_CLEAR_ON_SETID. Which contains > READ_IMPLIES_EXEC.> > So we now always clear READ_IMPLIES_EXEC for setuid applications.> > Anyway, I'm not sure this is it, but that's two examples of something that > did change unintentionally. > > Michael, mind trying this (UNTESTED!) patch?

Just Michal. :-) No worries about.

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()".

yes, your patch works. I tested it on QEMU and on real hw and I can't see any visible problem. I will do more test tomorrow.