exec/binfmt_script: Don't modify bprm->buf and then return -ENOEXEC
authorEric W. Biederman <ebiederm@xmission.com>
Mon, 18 May 2020 15:11:10 +0000 (10:11 -0500)
committerEric W. Biederman <ebiederm@xmission.com>
Thu, 21 May 2020 15:16:57 +0000 (10:16 -0500)
commitccbb18b67323b5c70b5f07177381d14cd002621f
treeca6d13ecb172de8e4ac8d68290ce0350c04e8afc
parent8b72ca9004ed35104deb80b07990da5503bc5252
exec/binfmt_script: Don't modify bprm->buf and then return -ENOEXEC

The return code -ENOEXEC serves to tell search_binary_handler that it
should continue searching for the binfmt to handle a given file.  This
makes return -ENOEXEC with a bprm->buf that is needed to continue the
search problematic.

The current binfmt_script manages to escape problems as it closes and
clears bprm->file before return -ENOEXEC with bprm->buf modified.
This prevents search_binary_handler from looping as it explicitly
handles a NULL bprm->file.

I plan on moving all of the bprm->file managment into fs/exec.c and out
of the binary handlers so this will become a problem.

Move closing bprm->file and the test for BINPRM_PATH_INACCESSIBLE
down below the last return of -ENOEXEC.

Introduce i_sep and i_end to track the end of the first argument and
the end of the parameters respectively.  Using those, constification
of all char * pointers, and the helpers next_terminator and
next_non_spacetab guarantee the parameter parsing will not modify
bprm->buf.

Only modify bprm->buf to terminate the strings i_arg and i_name with
'\0' for passing to copy_strings_kernel.

When replacing loops with next_non_spacetab and next_terminator care
has been take that the logic of the parsing code (short of replacing
characters by '\0') remains the same.

Link: https://lkml.kernel.org/r/874ksczru6.fsf_-_@x220.int.ebiederm.org
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
fs/binfmt_script.c