On Fri, Oct 12, 2012 at 10:50 PM, halfdog <me@xxxxxxxxxxx> wrote: > Kees Cook wrote: >> More importantly, I also wonder if interp handling to just be >> changed to be an allocation that needs to be cleaned up, as done with >> argv? > > You mean like an allocation on the stack of the new process' growing > stack? This would be cleaned automatically if something goes wrong > during exec. Either in userspace like argv, or just a straight kmalloc. Looking at the code, the problem is that binfmt_script and binfmt_misc are "rewrite" hacks (they change the bprm instead of actually starting a process), and the module loading is a hack in that it retries all the loaders a second time. These two hacks together aren't very compatible if the error path of the rewrite hacks expects to see the bprm go away instead of getting retried. I'm concerned that the proposed patch is really just a band-aid on top of a broken design. To make this safe, either the bprm needs to be explicitly copied for each recursion attempt (so the post-module-load retry starts with a clean bprm), or everything about the bprm needs to stay off the stack (to allow rewrite modifications to be stable). I think the latter approach is best since it means we don't have to execute the rewrite logic twice. It just means we must take a closer look at the lifetime of that structure and make sure we're cleaning it up correctly. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
