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    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site