diff options
author | Willy Tarreau <w@1wt.eu> | 2008-04-05 15:31:59 +0200 |
---|---|---|
committer | Willy Tarreau <w@1wt.eu> | 2008-07-20 18:25:43 +0200 |
commit | c8bd3b7ff9fbf102779fffcd1475635d67516c33 (patch) | |
tree | 7ced75a2ce7220601cb545c89ddfa45fcbccde70 | |
parent | 202251ef905afd67b70f999e66a9ca901e9770e6 (diff) | |
download | linux-2.4-c8bd3b7ff9fbf102779fffcd1475635d67516c33.tar.gz |
Fix dnotify/close race (CVE-2008-1375)
Issue reported by Al Viro with description taken from 2.6 commit
214b7049a7929f03bbd2786aaef04b8b79db34e2 :
We have a race between fcntl() and close() that can lead to
dnotify_struct inserted into inode's list *after* the last descriptor
had been gone from current->files.
Since that's the only point where dnotify_struct gets evicted, we are
screwed - it will stick around indefinitely. Even after struct file in
question is gone and freed. Worse, we can trigger send_sigio() on it at
any later point, which allows to send an arbitrary signal to arbitrary
process if we manage to apply enough memory pressure to get the page
that used to host that struct file and fill it with the right pattern...
-rw-r--r-- | fs/dnotify.c | 12 |
1 files changed, 12 insertions, 0 deletions
diff --git a/fs/dnotify.c b/fs/dnotify.c index 7ccdb3187d1c4b..1602277cfc5bb3 100644 --- a/fs/dnotify.c +++ b/fs/dnotify.c @@ -19,6 +19,7 @@ #include <linux/init.h> #include <linux/spinlock.h> #include <linux/slab.h> +#include <linux/file.h> extern void send_sigio(struct fown_struct *fown, int fd, int band); @@ -68,6 +69,7 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg) struct dnotify_struct **prev; struct inode *inode; fl_owner_t id = current->files; + struct file *f; if ((arg & ~DN_MULTISHOT) == 0) { dnotify_flush(filp, id); @@ -93,6 +95,16 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg) } prev = &odn->dn_next; } + + /* we'd lost the race with close(), sod off silently */ + read_lock(¤t->files->file_lock); + f = fcheck(fd); + read_unlock(¤t->files->file_lock); + if (f != filp) { + kmem_cache_free(dn_cache, dn); + goto out; + } + filp->f_owner.pid = current->pid; filp->f_owner.uid = current->uid; filp->f_owner.euid = current->euid; |