Looks like more your area of expertise..
(inner circle? me? good god.)
----- Forwarded message from Amnon Shiloh <[EMAIL PROTECTED]> -----
Envelope-to: [EMAIL PROTECTED]
Delivery-date: Fri, 22 Sep 2000 07:50:31 +0100
Delivered-To: [EMAIL PROTECTED]
From: Amnon Shiloh <[EMAIL PROTECTED]>
Subject: A Linux bug
To: [EMAIL PROTECTED]
Date: Fri, 22 Sep 2000 09:50:37 +0300 (IDT)
X-Mailer: ELM [version 2.5 PL0pre8]
Dear Matthew,
I write to you as the Linux-maintainer:
I am aware that the following bug is not exactly to do with "FILE LOCKING",
but this is the closest it gets within the MAINTAINERS file and I don't
know how else to convey this bug to the inner-circles of the Linux community
and get them to fix it.
Linux-version: 2.4.0-test8
Description of the bug:
-----------------------
According to the security tests of "fs/exec.c", lines 676 and 678,
both "current->files->count" and "current->fs->files" must reflect how many
processes share the same files and important-directories respectively.
Those fields, however, are also modified by casual onlookers, mainly
the PROC file-system (several times in "fs/proc/base.c")
(it is also modified by "chroot_fs_refs" - but well, this is really
extremely rare).
This means that "exec" can fail (returning EPERM) just because some
other user was browsing the /proc file-system at the time: of course,
when *bad* hackers find about this bug they can use it systematically
to distrupt other users.
Solution:
----------
The number of references to those two structures for the purpose of
deallocation when no-longer used (that include mere browsers)
must be separated from the number of actual USERS sharing them.
In the attached patch, I added a "users" field to each structure and took
care to increment (and later decrement) both "users" and "count" when a
structure is used by a sharing process, but keep incrementing/decrementing
only "count" when they are used by mere browsers.
Thank you very much for your attention,
Amnon Shiloh.
Content-Description: 'diff' output text
diff -Naur linux-2.4.0-test8/arch/i386/kernel/apm.c
linux-2.4.0-test8+/arch/i386/kernel/apm.c
--- linux-2.4.0-test8/arch/i386/kernel/apm.c Thu Aug 10 06:51:14 2000
+++ linux-2.4.0-test8+/arch/i386/kernel/apm.c Fri Sep 22 12:48:18 2000
@@ -1425,6 +1425,7 @@
exit_files(current); /* daemonize doesn't do exit_files */
current->files = init_task.files;
atomic_inc(¤t->files->count);
+ atomic_inc(¤t->files->users);
daemonize();
strcpy(current->comm, "kapmd");
diff -Naur linux-2.4.0-test8/drivers/usb/hub.c linux-2.4.0-test8+/drivers/usb/hub.c
--- linux-2.4.0-test8/drivers/usb/hub.c Sat Sep 9 19:40:26 2000
+++ linux-2.4.0-test8+/drivers/usb/hub.c Fri Sep 22 12:51:54 2000
@@ -590,6 +590,7 @@
exit_files(current); /* daemonize doesn't do exit_files */
current->files = init_task.files;
atomic_inc(¤t->files->count);
+ atomic_inc(¤t->files->users);
daemonize();
/* Setup a nice name */
diff -Naur linux-2.4.0-test8/fs/binfmt_elf.c linux-2.4.0-test8+/fs/binfmt_elf.c
--- linux-2.4.0-test8/fs/binfmt_elf.c Sat Sep 9 19:40:26 2000
+++ linux-2.4.0-test8+/fs/binfmt_elf.c Fri Sep 22 13:13:04 2000
@@ -493,6 +493,7 @@
struct fs_struct *old_fs = current->fs, *new_fs;
get_exec_domain(old_domain);
atomic_inc(&old_fs->count);
+ atomic_inc(&old_fs->users);
set_personality(PER_SVR4);
interpreter = open_exec(elf_interpreter);
@@ -503,6 +504,7 @@
current->exec_domain = old_domain;
current->fs = old_fs;
put_exec_domain(new_domain);
+ atomic_dec(&new_fs->users);
put_fs_struct(new_fs);
} else
#endif
diff -Naur linux-2.4.0-test8/fs/exec.c linux-2.4.0-test8+/fs/exec.c
--- linux-2.4.0-test8/fs/exec.c Sat Sep 9 19:40:26 2000
+++ linux-2.4.0-test8+/fs/exec.c Fri Sep 22 12:51:10 2000
@@ -673,9 +673,9 @@
/* (current->mm->mm_users > 1 is ok, as we'll get a new mm anyway) */
if (IS_NOSUID(inode)
|| must_not_trace_exec(current)
- || (atomic_read(¤t->fs->count) > 1)
+ || (atomic_read(¤t->fs->users) > 1)
|| (atomic_read(¤t->sig->count) > 1)
- || (atomic_read(¤t->files->count) > 1)) {
+ || (atomic_read(¤t->files->users) > 1)) {
if (id_change && !capable(CAP_SETUID))
return -EPERM;
if (cap_raised && !capable(CAP_SETPCAP))
diff -Naur linux-2.4.0-test8/include/linux/fs_struct.h
linux-2.4.0-test8+/include/linux/fs_struct.h
--- linux-2.4.0-test8/include/linux/fs_struct.h Thu Jul 6 04:01:00 2000
+++ linux-2.4.0-test8+/include/linux/fs_struct.h Fri Sep 22 12:47:15 2000
@@ -4,6 +4,7 @@
struct fs_struct {
atomic_t count;
+ atomic_t users;
rwlock_t lock;
int umask;
struct dentry * root, * pwd, * altroot;
@@ -11,6 +12,7 @@
};
#define INIT_FS { \
+ ATOMIC_INIT(1), \
ATOMIC_INIT(1), \
RW_LOCK_UNLOCKED, \
0022, \
diff -Naur linux-2.4.0-test8/include/linux/sched.h
linux-2.4.0-test8+/include/linux/sched.h
--- linux-2.4.0-test8/include/linux/sched.h Sat Sep 9 19:40:27 2000
+++ linux-2.4.0-test8+/include/linux/sched.h Fri Sep 22 13:21:08 2000
@@ -161,6 +161,7 @@
*/
struct files_struct {
atomic_t count;
+ atomic_t users;
rwlock_t file_lock;
int max_fds;
int max_fdset;
@@ -174,6 +175,7 @@
};
#define INIT_FILES { \
+ ATOMIC_INIT(1), \
ATOMIC_INIT(1), \
RW_LOCK_UNLOCKED, \
NR_OPEN_DEFAULT, \
diff -Naur linux-2.4.0-test8/kernel/exec_domain.c
linux-2.4.0-test8+/kernel/exec_domain.c
--- linux-2.4.0-test8/kernel/exec_domain.c Tue Jun 27 04:06:43 2000
+++ linux-2.4.0-test8+/kernel/exec_domain.c Fri Sep 22 13:16:31 2000
@@ -113,7 +113,7 @@
}
if (!it)
return;
- if (atomic_read(¤t->fs->count) != 1) {
+ if (atomic_read(¤t->fs->users) != 1) {
struct fs_struct *new = copy_fs_struct(current->fs);
struct fs_struct *old;
if (!new) {
@@ -124,6 +124,7 @@
old = current->fs;
current->fs = new;
task_unlock(current);
+ atomic_dec(&old->users);
put_fs_struct(old);
}
/*
diff -Naur linux-2.4.0-test8/kernel/exit.c linux-2.4.0-test8+/kernel/exit.c
--- linux-2.4.0-test8/kernel/exit.c Sat Sep 9 19:40:27 2000
+++ linux-2.4.0-test8+/kernel/exit.c Fri Sep 22 13:17:39 2000
@@ -221,6 +221,7 @@
task_lock(tsk);
tsk->files = NULL;
task_unlock(tsk);
+ atomic_dec(&files->users);
put_files_struct(files);
}
}
@@ -258,6 +259,7 @@
task_lock(tsk);
tsk->fs = NULL;
task_unlock(tsk);
+ atomic_dec(&fs->users);
__put_fs_struct(fs);
}
}
diff -Naur linux-2.4.0-test8/kernel/fork.c linux-2.4.0-test8+/kernel/fork.c
--- linux-2.4.0-test8/kernel/fork.c Sat Sep 9 19:40:27 2000
+++ linux-2.4.0-test8+/kernel/fork.c Fri Sep 22 12:54:01 2000
@@ -342,6 +342,7 @@
/* We don't need to lock fs - think why ;-) */
if (fs) {
atomic_set(&fs->count, 1);
+ atomic_set(&fs->users, 1);
fs->lock = RW_LOCK_UNLOCKED;
fs->umask = old->umask;
read_lock(&old->lock);
@@ -370,6 +371,7 @@
{
if (clone_flags & CLONE_FS) {
atomic_inc(¤t->fs->count);
+ atomic_inc(¤t->fs->users);
return 0;
}
tsk->fs = __copy_fs_struct(current->fs);
@@ -406,6 +408,7 @@
if (clone_flags & CLONE_FILES) {
atomic_inc(&oldf->count);
+ atomic_inc(&oldf->users);
goto out;
}
@@ -416,6 +419,7 @@
goto out;
atomic_set(&newf->count, 1);
+ atomic_set(&newf->users, 1);
newf->file_lock = RW_LOCK_UNLOCKED;
newf->next_fd = 0;
----- End forwarded message -----
--
Revolutions do not require corporate support.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
