lkml.org 
[lkml]   [2018]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Subject[PATCH 2/4] exec: Assign unshared files after there is no way back
    From
    Date
    The patch makes unshared files_struct to be assigned
    after exec or coredump are known to be successeful.
    Since previous patch converted all load_binary methods
    to use linux_binprm::new_files instead of current->files,
    now we are able to assign unshared_files after load_binary
    call.

    This change makes task->files more predictable and since
    that we may analyse the only thread's files pointer
    to determ either the task contains a fd or not, and not
    be afraid of race with failing exec. Next patch will
    use this feature to skip thread group iteration in __do_SAK(),
    and also this predictability may be useful for something else.

    Also note, that without the patch we skip failing exec,
    when we are doing the iterations in __do_SAK(), and
    these tasks remain alive. The patch also fixes this rare issue.

    Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
    ---
    fs/binfmt_misc.c | 2 --
    fs/coredump.c | 9 +++++----
    fs/exec.c | 15 ++++++++-------
    fs/file.c | 2 +-
    include/linux/fdtable.h | 4 ++--
    kernel/fork.c | 19 +++++++------------
    6 files changed, 23 insertions(+), 28 deletions(-)

    diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
    index 13c1fae45717..7568456895c7 100644
    --- a/fs/binfmt_misc.c
    +++ b/fs/binfmt_misc.c
    @@ -242,8 +242,6 @@ static int load_misc_binary(struct linux_binprm *bprm)
    dput(fmt->dentry);
    return retval;
    error:
    - if (fd_binary > 0)
    - sys_close(fd_binary);
    bprm->interp_flags = 0;
    bprm->interp_data = 0;
    goto ret;
    diff --git a/fs/coredump.c b/fs/coredump.c
    index 1e2c87acac9b..fe5a4504d975 100644
    --- a/fs/coredump.c
    +++ b/fs/coredump.c
    @@ -546,7 +546,7 @@ void do_coredump(const siginfo_t *siginfo)
    struct cred *cred;
    int retval = 0;
    int ispipe;
    - struct files_struct *displaced;
    + struct files_struct *files;
    /* require nonrelative corefile path and be extra careful */
    bool need_suid_safe = false;
    bool core_dumped = false;
    @@ -747,11 +747,12 @@ void do_coredump(const siginfo_t *siginfo)
    }

    /* get us an unshared descriptor table; almost always a no-op */
    - retval = unshare_files(&displaced);
    + files = unshare_files();
    + retval = PTR_ERR_OR_ZERO(files);
    if (retval)
    goto close_fail;
    - if (displaced)
    - put_files_struct(displaced);
    + if (files != current->files)
    + assign_files_struct(files);
    if (!dump_interrupted()) {
    file_start_write(cprm.file);
    core_dumped = binfmt->core_dump(&cprm);
    diff --git a/fs/exec.c b/fs/exec.c
    index 4c3b924ae103..ced4dc09444a 100644
    --- a/fs/exec.c
    +++ b/fs/exec.c
    @@ -1699,7 +1699,7 @@ static int do_execveat_common(int fd, struct filename *filename,
    char *pathbuf = NULL;
    struct linux_binprm *bprm;
    struct file *file;
    - struct files_struct *displaced;
    + struct files_struct *files;
    int retval;

    if (IS_ERR(filename))
    @@ -1721,7 +1721,8 @@ static int do_execveat_common(int fd, struct filename *filename,
    * further execve() calls fail. */
    current->flags &= ~PF_NPROC_EXCEEDED;

    - retval = unshare_files(&displaced);
    + files = unshare_files();
    + retval = PTR_ERR_OR_ZERO(files);
    if (retval)
    goto out_ret;

    @@ -1729,7 +1730,7 @@ static int do_execveat_common(int fd, struct filename *filename,
    bprm = kzalloc(sizeof(*bprm), GFP_KERNEL);
    if (!bprm)
    goto out_files;
    - bprm->new_files = current->files;
    + bprm->new_files = files;

    retval = prepare_bprm_creds(bprm);
    if (retval)
    @@ -1813,8 +1814,8 @@ static int do_execveat_common(int fd, struct filename *filename,
    free_bprm(bprm);
    kfree(pathbuf);
    putname(filename);
    - if (displaced)
    - put_files_struct(displaced);
    + if (files != current->files)
    + assign_files_struct(files);
    return retval;

    out:
    @@ -1832,8 +1833,8 @@ static int do_execveat_common(int fd, struct filename *filename,
    kfree(pathbuf);

    out_files:
    - if (displaced)
    - reset_files_struct(displaced);
    + if (files != current->files)
    + put_files_struct(files);
    out_ret:
    putname(filename);
    return retval;
    diff --git a/fs/file.c b/fs/file.c
    index e578e5537c32..7ed519c65d0b 100644
    --- a/fs/file.c
    +++ b/fs/file.c
    @@ -427,7 +427,7 @@ void put_files_struct(struct files_struct *files)
    }
    }

    -void reset_files_struct(struct files_struct *files)
    +void assign_files_struct(struct files_struct *files)
    {
    struct task_struct *tsk = current;
    struct files_struct *old;
    diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
    index 1c65817673db..00db7eadf895 100644
    --- a/include/linux/fdtable.h
    +++ b/include/linux/fdtable.h
    @@ -104,8 +104,8 @@ struct task_struct;

    struct files_struct *get_files_struct(struct task_struct *);
    void put_files_struct(struct files_struct *fs);
    -void reset_files_struct(struct files_struct *);
    -int unshare_files(struct files_struct **);
    +void assign_files_struct(struct files_struct *);
    +struct files_struct *unshare_files(void);
    struct files_struct *dup_fd(struct files_struct *, int *) __latent_entropy;
    void do_close_on_exec(struct files_struct *);
    int iterate_fd(struct files_struct *, unsigned,
    diff --git a/kernel/fork.c b/kernel/fork.c
    index 2295fc69717f..7690734eb354 100644
    --- a/kernel/fork.c
    +++ b/kernel/fork.c
    @@ -2434,22 +2434,17 @@ SYSCALL_DEFINE1(unshare, unsigned long, unshare_flags)
    * the exec layer of the kernel.
    */

    -int unshare_files(struct files_struct **displaced)
    +struct files_struct *unshare_files(void)
    {
    struct task_struct *task = current;
    - struct files_struct *copy = NULL;
    + struct files_struct *files = task->files;
    int error;

    - error = unshare_fd(CLONE_FILES, &copy);
    - if (error || !copy) {
    - *displaced = NULL;
    - return error;
    - }
    - *displaced = task->files;
    - task_lock(task);
    - task->files = copy;
    - task_unlock(task);
    - return 0;
    + error = unshare_fd(CLONE_FILES, &files);
    + if (error)
    + return ERR_PTR(error);
    +
    + return files;
    }

    int sysctl_max_threads(struct ctl_table *table, int write,
    \
     
     \ /
      Last update: 2018-01-14 23:23    [W:3.180 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site