lkml.org 
[lkml]   [2020]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH v2 6/8] 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.

    Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
    ---
    fs/binfmt_script.c | 80 ++++++++++++++++++++++------------------------
    1 file changed, 38 insertions(+), 42 deletions(-)

    diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
    index 8d718d8fd0fe..85e0ef86eb11 100644
    --- a/fs/binfmt_script.c
    +++ b/fs/binfmt_script.c
    @@ -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;
    --
    2.25.0
    \
     
     \ /
      Last update: 2020-05-19 02:38    [W:4.525 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site