From 53d2be79d5981b7efc8c5ec1169613bba95bde20 Mon Sep 17 00:00:00 2001 From: Davide Libenzi Date: Fri, 16 Sep 2005 19:28:06 -0700 Subject: [PATCH] epoll: fix delayed initialization bug Al found a potential problem in epoll_create(), where the file->private_data member was set after fd_install(). This is obviously wrong since another thread might do a close() on that fd# before we set the file->private_data member. This goes over 2.6.13 and passes a few basic tests I've done here. (akpm: snuck in a kzalloc() cleanup too) Signed-off-by: Davide Libenzi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/eventpoll.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) (limited to 'fs') diff --git a/fs/eventpoll.c b/fs/eventpoll.c index 6ab1dd0..403b90a 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -231,8 +231,9 @@ struct ep_pqueue { static void ep_poll_safewake_init(struct poll_safewake *psw); static void ep_poll_safewake(struct poll_safewake *psw, wait_queue_head_t *wq); -static int ep_getfd(int *efd, struct inode **einode, struct file **efile); -static int ep_file_init(struct file *file); +static int ep_getfd(int *efd, struct inode **einode, struct file **efile, + struct eventpoll *ep); +static int ep_alloc(struct eventpoll **pep); static void ep_free(struct eventpoll *ep); static struct epitem *ep_find(struct eventpoll *ep, struct file *file, int fd); static void ep_use_epitem(struct epitem *epi); @@ -501,38 +502,37 @@ void eventpoll_release_file(struct file *file) asmlinkage long sys_epoll_create(int size) { int error, fd; + struct eventpoll *ep; struct inode *inode; struct file *file; DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d)\n", current, size)); - /* Sanity check on the size parameter */ + /* + * Sanity check on the size parameter, and create the internal data + * structure ( "struct eventpoll" ). + */ error = -EINVAL; - if (size <= 0) + if (size <= 0 || (error = ep_alloc(&ep)) != 0) goto eexit_1; /* * Creates all the items needed to setup an eventpoll file. That is, * a file structure, and inode and a free file descriptor. */ - error = ep_getfd(&fd, &inode, &file); - if (error) - goto eexit_1; - - /* Setup the file internal data structure ( "struct eventpoll" ) */ - error = ep_file_init(file); + error = ep_getfd(&fd, &inode, &file, ep); if (error) goto eexit_2; - DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d) = %d\n", current, size, fd)); return fd; eexit_2: - sys_close(fd); + ep_free(ep); + kfree(ep); eexit_1: DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d) = %d\n", current, size, error)); @@ -706,7 +706,8 @@ eexit_1: /* * Creates the file descriptor to be used by the epoll interface. */ -static int ep_getfd(int *efd, struct inode **einode, struct file **efile) +static int ep_getfd(int *efd, struct inode **einode, struct file **efile, + struct eventpoll *ep) { struct qstr this; char name[32]; @@ -756,7 +757,7 @@ static int ep_getfd(int *efd, struct inode **einode, struct file **efile) file->f_op = &eventpoll_fops; file->f_mode = FMODE_READ; file->f_version = 0; - file->private_data = NULL; + file->private_data = ep; /* Install the new setup file into the allocated fd. */ fd_install(fd, file); @@ -777,14 +778,13 @@ eexit_1: } -static int ep_file_init(struct file *file) +static int ep_alloc(struct eventpoll **pep) { - struct eventpoll *ep; + struct eventpoll *ep = kzalloc(sizeof(*ep), GFP_KERNEL); - if (!(ep = kmalloc(sizeof(struct eventpoll), GFP_KERNEL))) + if (!ep) return -ENOMEM; - memset(ep, 0, sizeof(*ep)); rwlock_init(&ep->lock); init_rwsem(&ep->sem); init_waitqueue_head(&ep->wq); @@ -792,9 +792,9 @@ static int ep_file_init(struct file *file) INIT_LIST_HEAD(&ep->rdllist); ep->rbr = RB_ROOT; - file->private_data = ep; + *pep = ep; - DNPRINTK(3, (KERN_INFO "[%p] eventpoll: ep_file_init() ep=%p\n", + DNPRINTK(3, (KERN_INFO "[%p] eventpoll: ep_alloc() ep=%p\n", current, ep)); return 0; } -- cgit v1.1 From a464adeb7e8f1cd65ca911e20a7c02e452dc2c17 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Fri, 16 Sep 2005 19:28:09 -0700 Subject: [PATCH] Add smp_mb__after_clear_bit() to unlock_kiocb() Add smp_mb__after_clear_bit() to unlock_kiocb() AIO's use of wait_on_bit_lock()/wake_up_bit() forgot to add a barrier between clearing its lock bit and calling wake_up_bit() so wake_up_bit()'s unlocked waitqueue_active() can race. This puts AIO's use in line with the others and the comment above wake_up_bit(). Signed-off-by: Zach Brown Acked-by: Benjamin LaHaise Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/aio.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs') diff --git a/fs/aio.c b/fs/aio.c index 38f6268..0e11e31 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -562,6 +562,7 @@ static inline void lock_kiocb(struct kiocb *iocb) static inline void unlock_kiocb(struct kiocb *iocb) { kiocbClearLocked(iocb); + smp_mb__after_clear_bit(); wake_up_bit(&iocb->ki_flags, KIF_LOCKED); } -- cgit v1.1 From 4fb3a53860cee2aaaf81186c451b7da0b95b45c1 Mon Sep 17 00:00:00 2001 From: Dipankar Sarma Date: Fri, 16 Sep 2005 19:28:13 -0700 Subject: [PATCH] files: fix preemption issues With the new fdtable locking rules, you have to protect fdtable with either ->file_lock or rcu_read_lock/unlock(). There are some places where we aren't doing either. This patch fixes those places. Signed-off-by: Dipankar Sarma Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/locks.c | 3 +++ fs/proc/array.c | 3 +++ 2 files changed, 6 insertions(+) (limited to 'fs') diff --git a/fs/locks.c b/fs/locks.c index c2c09b4..f7daa5f 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -124,6 +124,7 @@ #include #include #include +#include #include #include @@ -2205,6 +2206,7 @@ void steal_locks(fl_owner_t from) lock_kernel(); j = 0; + rcu_read_lock(); fdt = files_fdtable(files); for (;;) { unsigned long set; @@ -2222,6 +2224,7 @@ void steal_locks(fl_owner_t from) set >>= 1; } } + rcu_read_unlock(); unlock_kernel(); } EXPORT_SYMBOL(steal_locks); diff --git a/fs/proc/array.c b/fs/proc/array.c index d88d518..d84eeca 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -74,6 +74,7 @@ #include #include #include +#include #include #include @@ -180,12 +181,14 @@ static inline char * task_state(struct task_struct *p, char *buffer) p->gid, p->egid, p->sgid, p->fsgid); read_unlock(&tasklist_lock); task_lock(p); + rcu_read_lock(); if (p->files) fdt = files_fdtable(p->files); buffer += sprintf(buffer, "FDSize:\t%d\n" "Groups:\t", fdt ? fdt->max_fds : 0); + rcu_read_unlock(); group_info = p->group_info; get_group_info(group_info); -- cgit v1.1 From ef402268f7c9ab1872cafa1e638eb78a75b7c18f Mon Sep 17 00:00:00 2001 From: OGAWA Hirofumi Date: Fri, 16 Sep 2005 19:28:13 -0700 Subject: [PATCH] FAT: miss-sync issues on sync mount (miss-sync on write) This patch fixes miss-sync issue on write() system call. This updates inode attrs flags, mtime and ctime on every comit_write call, due to locking. Signed-off-by: Hiroyuki Machida Signed-off-by: OGAWA Hirofumi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/fat/file.c | 37 ++----------------------------------- fs/fat/inode.c | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 36 deletions(-) (limited to 'fs') diff --git a/fs/fat/file.c b/fs/fat/file.c index 62ffa91..7134403 100644 --- a/fs/fat/file.c +++ b/fs/fat/file.c @@ -12,39 +12,6 @@ #include #include -static ssize_t fat_file_aio_write(struct kiocb *iocb, const char __user *buf, - size_t count, loff_t pos) -{ - struct inode *inode = iocb->ki_filp->f_dentry->d_inode; - int retval; - - retval = generic_file_aio_write(iocb, buf, count, pos); - if (retval > 0) { - inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC; - MSDOS_I(inode)->i_attrs |= ATTR_ARCH; - mark_inode_dirty(inode); -// check the locking rules -// if (IS_SYNC(inode)) -// fat_sync_inode(inode); - } - return retval; -} - -static ssize_t fat_file_writev(struct file *filp, const struct iovec *iov, - unsigned long nr_segs, loff_t *ppos) -{ - struct inode *inode = filp->f_dentry->d_inode; - int retval; - - retval = generic_file_writev(filp, iov, nr_segs, ppos); - if (retval > 0) { - inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC; - MSDOS_I(inode)->i_attrs |= ATTR_ARCH; - mark_inode_dirty(inode); - } - return retval; -} - int fat_generic_ioctl(struct inode *inode, struct file *filp, unsigned int cmd, unsigned long arg) { @@ -148,9 +115,9 @@ struct file_operations fat_file_operations = { .read = do_sync_read, .write = do_sync_write, .readv = generic_file_readv, - .writev = fat_file_writev, + .writev = generic_file_writev, .aio_read = generic_file_aio_read, - .aio_write = fat_file_aio_write, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .ioctl = fat_generic_ioctl, .fsync = file_fsync, diff --git a/fs/fat/inode.c b/fs/fat/inode.c index a7cbe68..51b1d15 100644 --- a/fs/fat/inode.c +++ b/fs/fat/inode.c @@ -102,6 +102,19 @@ static int fat_prepare_write(struct file *file, struct page *page, &MSDOS_I(page->mapping->host)->mmu_private); } +static int fat_commit_write(struct file *file, struct page *page, + unsigned from, unsigned to) +{ + struct inode *inode = page->mapping->host; + int err = generic_commit_write(file, page, from, to); + if (!err && !(MSDOS_I(inode)->i_attrs & ATTR_ARCH)) { + inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC; + MSDOS_I(inode)->i_attrs |= ATTR_ARCH; + mark_inode_dirty(inode); + } + return err; +} + static sector_t _fat_bmap(struct address_space *mapping, sector_t block) { return generic_block_bmap(mapping, block, fat_get_block); @@ -112,7 +125,7 @@ static struct address_space_operations fat_aops = { .writepage = fat_writepage, .sync_page = block_sync_page, .prepare_write = fat_prepare_write, - .commit_write = generic_commit_write, + .commit_write = fat_commit_write, .bmap = _fat_bmap }; -- cgit v1.1