Hi Linus,
In sys_uselib(), if fget(fd) returns NULL, then fput(NULL) will
dereference NULL->f_count - therefore fput(file) should be inside
if(file).
Also, while I was in this file (fs/exec.c) I did the following changes:
1. removed initialisation 'formats = NULL' for the same reason
'file_systems=NULL' was removed from fs/super.c (BSS must be zeroed by
the ANSI C standard so arch/i386/kernel/head.S does it)
2. refined binfmt_lock into a read/write lock instead of plain spinlock
as the pattern of access to 'formats' suggests it naturally - i.e. only
register/unregister_binfmt() need to be exclusive, the rest can benefit
from being concurrent.
3. removed global kernel lock from sys_uselib() as it seems to be
re-entrant (uses sys_open/sys_close which do enough locking internally)
The patch is (tested) against 2.3.99-pre9-3.
Regards,
Tigran
--- linux/fs/exec.c Sun May 21 14:15:11 2000
+++ work/fs/exec.c Sun May 21 14:39:50 2000
@@ -45,8 +45,8 @@
#include <linux/kmod.h>
#endif
-static struct linux_binfmt *formats = (struct linux_binfmt *) NULL;
-static spinlock_t binfmt_lock = SPIN_LOCK_UNLOCKED;
+static struct linux_binfmt *formats;
+static rwlock_t binfmt_lock = RW_LOCK_UNLOCKED;
int register_binfmt(struct linux_binfmt * fmt)
{
@@ -56,17 +56,17 @@
return -EINVAL;
if (fmt->next)
return -EBUSY;
- spin_lock(&binfmt_lock);
+ write_lock(&binfmt_lock);
while (*tmp) {
if (fmt == *tmp) {
- spin_unlock(&binfmt_lock);
+ write_unlock(&binfmt_lock);
return -EBUSY;
}
tmp = &(*tmp)->next;
}
fmt->next = formats;
formats = fmt;
- spin_unlock(&binfmt_lock);
+ write_unlock(&binfmt_lock);
return 0;
}
@@ -74,16 +74,16 @@
{
struct linux_binfmt ** tmp = &formats;
- spin_lock(&binfmt_lock);
+ write_lock(&binfmt_lock);
while (*tmp) {
if (fmt == *tmp) {
*tmp = fmt->next;
- spin_unlock(&binfmt_lock);
+ write_unlock(&binfmt_lock);
return 0;
}
tmp = &(*tmp)->next;
}
- spin_unlock(&binfmt_lock);
+ write_unlock(&binfmt_lock);
return -EINVAL;
}
@@ -103,35 +103,34 @@
{
int fd, retval;
struct file * file;
- struct linux_binfmt * fmt;
- lock_kernel();
fd = sys_open(library, 0, 0);
- retval = fd;
if (fd < 0)
- goto out;
+ return fd;
file = fget(fd);
retval = -ENOEXEC;
- if (file && file->f_op && file->f_op->read) {
- spin_lock(&binfmt_lock);
- for (fmt = formats ; fmt ; fmt = fmt->next) {
- if (!fmt->load_shlib)
- continue;
- if (!try_inc_mod_count(fmt->module))
- continue;
- spin_unlock(&binfmt_lock);
- retval = fmt->load_shlib(file);
- spin_lock(&binfmt_lock);
- put_binfmt(fmt);
- if (retval != -ENOEXEC)
- break;
+ if (file) {
+ if(file->f_op && file->f_op->read) {
+ struct linux_binfmt * fmt;
+
+ read_lock(&binfmt_lock);
+ for (fmt = formats ; fmt ; fmt = fmt->next) {
+ if (!fmt->load_shlib)
+ continue;
+ if (!try_inc_mod_count(fmt->module))
+ continue;
+ read_unlock(&binfmt_lock);
+ retval = fmt->load_shlib(file);
+ read_lock(&binfmt_lock);
+ put_binfmt(fmt);
+ if (retval != -ENOEXEC)
+ break;
+ }
+ read_unlock(&binfmt_lock);
}
- spin_unlock(&binfmt_lock);
+ fput(file);
}
- fput(file);
sys_close(fd);
-out:
- unlock_kernel();
return retval;
}
@@ -747,14 +746,14 @@
}
#endif
for (try=0; try<2; try++) {
- spin_lock(&binfmt_lock);
+ read_lock(&binfmt_lock);
for (fmt = formats ; fmt ; fmt = fmt->next) {
int (*fn)(struct linux_binprm *, struct pt_regs *) = fmt->load_binary;
if (!fn)
continue;
if (!try_inc_mod_count(fmt->module))
continue;
- spin_unlock(&binfmt_lock);
+ read_unlock(&binfmt_lock);
retval = fn(bprm, regs);
if (retval >= 0) {
put_binfmt(fmt);
@@ -764,16 +763,16 @@
current->did_exec = 1;
return retval;
}
- spin_lock(&binfmt_lock);
+ read_lock(&binfmt_lock);
put_binfmt(fmt);
if (retval != -ENOEXEC)
break;
if (!bprm->file) {
- spin_unlock(&binfmt_lock);
+ read_unlock(&binfmt_lock);
return retval;
}
}
- spin_unlock(&binfmt_lock);
+ read_unlock(&binfmt_lock);
if (retval != -ENOEXEC) {
break;
#ifdef CONFIG_KMOD
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Tue May 23 2000 - 21:00:19 EST