On Tue, May 12, 2020 at 01:42:53PM -0500, Eric W. Biederman wrote: > Kees Cook <keescook@xxxxxxxxxxxx> writes: > > Should binfmt_misc do the install, or can the consuming binfmt do it? > > i.e. when binfmt_elf sees bprm->execfd, does it perform the install > > instead? > > I am still thinking about this one, but here is where I am at. At a > practical level passing the file descriptor of the script to interpreter > seems like something we should encourage in the long term. It removes > races and it is cheaper because then the interpreter does not have to > turn around and open the script itself. Yeah, this does sounds pretty good, though I have concerns about doing it for a process that isn't expecting it. I've seen a lot of bad code make assumptions about initial fd numbers. :( > Strictly speaking binfmt_misc should not need to close the file > descriptor in binfmt_misc because we have already unshared the files > struct and reset_files_struct should handle restoring it. If I get what you mean, I agree. The error case is fine. > Calling fd_install in binfmt_misc still seems wrong, as that exposes > the new file descriptor to user space with the old creds. I haven't dug into the details here -- is there a real risk here? The old creds are what opened the file originally for the exec. Are you thinking about executable-but-not-readable files? > It is possible although unlikely for userspace to find the file > descriptor without consulting AT_EXECFD so just to be conservative I > think we should install the file descriptor in begin_new_exec even if > the next interpreter does not support AT_EXECFD. I think universally installing the fd needs to be a distinct patch -- it's going to have a lot of consequences, IMO. We can certainly deal with them, but I don't think it should be part of this clean-up series. > I am still working on how to handle recursive binfmts but I suspect it > is just a matter of having an array of struct files in struct > linux_binprm. If install is left if binfmt_misc, then the recursive problem goes away, yes? -- Kees Cook
