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>
This commit is contained in:
parent
8b72ca9004
commit
ccbb18b673
|
@ -16,14 +16,14 @@
|
|||
#include <linux/fs.h>
|
||||
|
||||
static inline bool spacetab(char c) { return c == ' ' || c == '\t'; }
|
||||
static inline char *next_non_spacetab(char *first, const char *last)
|
||||
static inline const char *next_non_spacetab(const char *first, const char *last)
|
||||
{
|
||||
for (; first <= last; first++)
|
||||
if (!spacetab(*first))
|
||||
return first;
|
||||
return NULL;
|
||||
}
|
||||
static inline char *next_terminator(char *first, const char *last)
|
||||
static inline const char *next_terminator(const char *first, const char *last)
|
||||
{
|
||||
for (; first <= last; first++)
|
||||
if (spacetab(*first) || !*first)
|
||||
|
@ -33,8 +33,7 @@ static inline char *next_terminator(char *first, const char *last)
|
|||
|
||||
static int load_script(struct linux_binprm *bprm)
|
||||
{
|
||||
const char *i_arg, *i_name;
|
||||
char *cp, *buf_end;
|
||||
const char *i_name, *i_sep, *i_arg, *i_end, *buf_end;
|
||||
struct file *file;
|
||||
int retval;
|
||||
|
||||
|
@ -42,20 +41,6 @@ static int load_script(struct linux_binprm *bprm)
|
|||
if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!'))
|
||||
return -ENOEXEC;
|
||||
|
||||
/*
|
||||
* If the script filename will be inaccessible after exec, typically
|
||||
* because it is a "/dev/fd/<fd>/.." path against an O_CLOEXEC fd, give
|
||||
* up now (on the assumption that the interpreter will want to load
|
||||
* this file).
|
||||
*/
|
||||
if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
|
||||
return -ENOENT;
|
||||
|
||||
/* Release since we are not mapping a binary into memory. */
|
||||
allow_write_access(bprm->file);
|
||||
fput(bprm->file);
|
||||
bprm->file = NULL;
|
||||
|
||||
/*
|
||||
* This section handles parsing the #! line into separate
|
||||
* interpreter path and argument strings. We must be careful
|
||||
|
@ -71,39 +56,48 @@ static int load_script(struct linux_binprm *bprm)
|
|||
* parse them on its own.
|
||||
*/
|
||||
buf_end = bprm->buf + sizeof(bprm->buf) - 1;
|
||||
cp = strnchr(bprm->buf, sizeof(bprm->buf), '\n');
|
||||
if (!cp) {
|
||||
cp = next_non_spacetab(bprm->buf + 2, buf_end);
|
||||
if (!cp)
|
||||
i_end = strnchr(bprm->buf, sizeof(bprm->buf), '\n');
|
||||
if (!i_end) {
|
||||
i_end = next_non_spacetab(bprm->buf + 2, buf_end);
|
||||
if (!i_end)
|
||||
return -ENOEXEC; /* Entire buf is spaces/tabs */
|
||||
/*
|
||||
* If there is no later space/tab/NUL we must assume the
|
||||
* interpreter path is truncated.
|
||||
*/
|
||||
if (!next_terminator(cp, buf_end))
|
||||
if (!next_terminator(i_end, buf_end))
|
||||
return -ENOEXEC;
|
||||
cp = buf_end;
|
||||
i_end = buf_end;
|
||||
}
|
||||
/* NUL-terminate the buffer and any trailing spaces/tabs. */
|
||||
*cp = '\0';
|
||||
while (cp > bprm->buf) {
|
||||
cp--;
|
||||
if ((*cp == ' ') || (*cp == '\t'))
|
||||
*cp = '\0';
|
||||
else
|
||||
break;
|
||||
}
|
||||
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
|
||||
if (*cp == '\0')
|
||||
/* Trim any trailing spaces/tabs from i_end */
|
||||
while (spacetab(i_end[-1]))
|
||||
i_end--;
|
||||
|
||||
/* Skip over leading spaces/tabs */
|
||||
i_name = next_non_spacetab(bprm->buf+2, i_end);
|
||||
if (!i_name || (i_name == i_end))
|
||||
return -ENOEXEC; /* No interpreter name found */
|
||||
i_name = cp;
|
||||
|
||||
/* Is there an optional argument? */
|
||||
i_arg = NULL;
|
||||
for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++)
|
||||
/* nothing */ ;
|
||||
while ((*cp == ' ') || (*cp == '\t'))
|
||||
*cp++ = '\0';
|
||||
if (*cp)
|
||||
i_arg = cp;
|
||||
i_sep = next_terminator(i_name, i_end);
|
||||
if (i_sep && (*i_sep != '\0'))
|
||||
i_arg = next_non_spacetab(i_sep, i_end);
|
||||
|
||||
/*
|
||||
* If the script filename will be inaccessible after exec, typically
|
||||
* because it is a "/dev/fd/<fd>/.." path against an O_CLOEXEC fd, give
|
||||
* up now (on the assumption that the interpreter will want to load
|
||||
* this file).
|
||||
*/
|
||||
if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
|
||||
return -ENOENT;
|
||||
|
||||
/* Release since we are not mapping a binary into memory. */
|
||||
allow_write_access(bprm->file);
|
||||
fput(bprm->file);
|
||||
bprm->file = NULL;
|
||||
|
||||
/*
|
||||
* OK, we've parsed out the interpreter name and
|
||||
* (optional) argument.
|
||||
|
@ -121,7 +115,9 @@ static int load_script(struct linux_binprm *bprm)
|
|||
if (retval < 0)
|
||||
return retval;
|
||||
bprm->argc++;
|
||||
*((char *)i_end) = '\0';
|
||||
if (i_arg) {
|
||||
*((char *)i_sep) = '\0';
|
||||
retval = copy_strings_kernel(1, &i_arg, bprm);
|
||||
if (retval < 0)
|
||||
return retval;
|
||||
|
|
Loading…
Reference in New Issue