lkml.org 
[lkml]   [2003]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] 2.4: Fix steal_locks race
    Here is an accumulate patch of Herbert's and my changes.

    I could see no reason for ftmp in flush_old_exec and load_elf_binary, so I
    removed that. Please correct me if that is wrong.

    Cheers,
    Andreas.


    On Fri, 8 Aug 2003, Herbert Xu wrote:

    > Hi:
    >
    > The steal_locks() call in binfmt_elf.c is buggy. It steals locks from
    > a files entry whose reference was dropped much earlier. This allows it
    > to steal other process's locks.
    >
    > The following patch calls steal_locks() earlier so that this does not
    > happen.Index: linux-2.4.22-rc2.orig/fs/exec.c
    ===================================================================
    --- linux-2.4.22-rc2.orig.orig/fs/exec.c 2003-08-09 04:44:07.000000000 +0200
    +++ linux-2.4.22-rc2.orig/fs/exec.c 2003-08-09 04:47:53.000000000 +0200
    @@ -582,8 +582,6 @@ int flush_old_exec(struct linux_binprm *
    retval = unshare_files();
    if(retval)
    goto flush_failed;
    - steal_locks(files, current->files);
    - put_files_struct(files);

    /*
    * Release all of the old mmap stuff
    @@ -592,6 +590,8 @@ int flush_old_exec(struct linux_binprm *
    if (retval) goto mmap_failed;

    /* This is the point of no return */
    + steal_locks(files);
    + put_files_struct(files);
    release_old_signals(oldsig);

    current->sas_ss_sp = current->sas_ss_size = 0;
    @@ -629,6 +629,8 @@ int flush_old_exec(struct linux_binprm *
    return 0;

    mmap_failed:
    + put_files_struct(current->files);
    + current->files = files;
    flush_failed:
    spin_lock_irq(&current->sigmask_lock);
    if (current->sig != oldsig) {
    Index: linux-2.4.22-rc2.orig/include/linux/fs.h
    ===================================================================
    --- linux-2.4.22-rc2.orig.orig/include/linux/fs.h 2003-08-09 04:44:07.000000000 +0200
    +++ linux-2.4.22-rc2.orig/include/linux/fs.h 2003-08-09 04:46:05.000000000 +0200
    @@ -674,7 +674,7 @@ extern int __get_lease(struct inode *ino
    extern time_t lease_get_mtime(struct inode *);
    extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
    extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
    -extern void steal_locks(fl_owner_t from, fl_owner_t to);
    +extern void steal_locks(fl_owner_t from);

    struct fasync_struct {
    int magic;
    Index: linux-2.4.22-rc2.orig/fs/binfmt_elf.c
    ===================================================================
    --- linux-2.4.22-rc2.orig.orig/fs/binfmt_elf.c 2003-08-09 04:44:07.000000000 +0200
    +++ linux-2.4.22-rc2.orig/fs/binfmt_elf.c 2003-08-09 04:48:27.000000000 +0200
    @@ -444,7 +444,7 @@ static int load_elf_binary(struct linux_
    struct elfhdr interp_elf_ex;
    struct exec interp_ex;
    char passed_fileno[6];
    - struct files_struct *files, *ftmp;
    + struct files_struct *files;

    /* Get the exec-header */
    elf_ex = *((struct elfhdr *) bprm->buf);
    @@ -480,7 +480,6 @@ static int load_elf_binary(struct linux_
    files = current->files; /* Refcounted so ok */
    if(unshare_files() < 0)
    goto out_free_ph;
    - steal_locks(files, current->files);

    /* exec will make our files private anyway, but for the a.out
    loader stuff we need to do it earlier */
    @@ -603,7 +602,9 @@ static int load_elf_binary(struct linux_
    goto out_free_dentry;

    /* Discard our unneeded old files struct */
    + steal_locks(files);
    put_files_struct(files);
    + files = NULL;

    /* OK, This is the point of no return */
    current->mm->start_data = 0;
    @@ -714,18 +715,16 @@ static int load_elf_binary(struct linux_
    elf_entry = load_elf_interp(&interp_elf_ex,
    interpreter,
    &interp_load_addr);
    -
    - allow_write_access(interpreter);
    - fput(interpreter);
    - kfree(elf_interpreter);
    -
    if (BAD_ADDR(elf_entry)) {
    printk(KERN_ERR "Unable to load interpreter\n");
    - kfree(elf_phdata);
    send_sig(SIGSEGV, current, 0);
    retval = -ENOEXEC; /* Nobody gets to see this, but.. */
    - goto out;
    + goto out_free_dentry;
    }
    +
    + allow_write_access(interpreter);
    + fput(interpreter);
    + kfree(elf_interpreter);
    }

    kfree(elf_phdata);
    @@ -811,10 +810,10 @@ out_free_interp:
    out_free_file:
    sys_close(elf_exec_fileno);
    out_free_fh:
    - ftmp = current->files;
    - current->files = files;
    - steal_locks(ftmp, current->files);
    - put_files_struct(ftmp);
    + if (files) {
    + put_files_struct(current->files);
    + current->files = files;
    + }
    out_free_ph:
    kfree(elf_phdata);
    goto out;
    Index: linux-2.4.22-rc2.orig/fs/locks.c
    ===================================================================
    --- linux-2.4.22-rc2.orig.orig/fs/locks.c 2003-08-09 04:44:07.000000000 +0200
    +++ linux-2.4.22-rc2.orig/fs/locks.c 2003-08-09 04:46:05.000000000 +0200
    @@ -1937,11 +1937,11 @@ done:
    return length;
    }

    -void steal_locks(fl_owner_t from, fl_owner_t to)
    +void steal_locks(fl_owner_t from)
    {
    struct list_head *tmp;

    - if (from == to)
    + if (from == current->files)
    return;

    lock_kernel();
    @@ -1949,7 +1949,7 @@ void steal_locks(fl_owner_t from, fl_own
    struct file_lock *fl = list_entry(tmp, struct file_lock,
    fl_link);
    if (fl->fl_owner == from)
    - fl->fl_owner = to;
    + fl->fl_owner = current->files;
    }
    unlock_kernel();
    }
    \
     
     \ /
      Last update: 2005-03-22 13:47    [W:0.027 / U:0.880 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site