diff options
author | Jan Kara <jack@suse.cz> | 2016-12-12 16:08:41 +0100 |
---|---|---|
committer | Ben Hutchings <ben@decadent.org.uk> | 2017-03-16 02:18:31 +0000 |
commit | 3c67dcc8054bf71be30a91a8c2b10a4c88ed9663 (patch) | |
tree | e1e3b1de1d22173f997df8576a6372ff2af16512 /fs | |
parent | d148f1617ddb4e830e4d6974fda6a5db27bdb7ec (diff) |
fsnotify: Fix possible use-after-free in inode iteration on umount
commit 5716863e0f8251d3360d4cbfc0e44e08007075df upstream.
fsnotify_unmount_inodes() plays complex tricks to pin next inode in the
sb->s_inodes list when iterating over all inodes. Furthermore the code has a
bug that if the current inode is the last on i_sb_list that does not have e.g.
I_FREEING set, then we leave next_i pointing to inode which may get removed
from the i_sb_list once we drop s_inode_list_lock thus resulting in
use-after-free issues (usually manifesting as infinite looping in
fsnotify_unmount_inodes()).
Fix the problem by keeping current inode pinned somewhat longer. Then we can
make the code much simpler and standard.
Signed-off-by: Jan Kara <jack@suse.cz>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/notify/inode_mark.c | 46 |
1 files changed, 9 insertions, 37 deletions
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c index df6daccf49ba..4c633ca8a7df 100644 --- a/fs/notify/inode_mark.c +++ b/fs/notify/inode_mark.c @@ -243,12 +243,10 @@ out: */ void fsnotify_unmount_inodes(struct list_head *list) { - struct inode *inode, *next_i, *need_iput = NULL; + struct inode *inode, *iput_inode = NULL; spin_lock(&inode_sb_list_lock); - list_for_each_entry_safe(inode, next_i, list, i_sb_list) { - struct inode *need_iput_tmp; - + list_for_each_entry(inode, list, i_sb_list) { /* * We cannot __iget() an inode in state I_FREEING, * I_WILL_FREE, or I_NEW which is fine because by that point @@ -271,50 +269,24 @@ void fsnotify_unmount_inodes(struct list_head *list) continue; } - need_iput_tmp = need_iput; - need_iput = NULL; - - /* In case fsnotify_inode_delete() drops a reference. */ - if (inode != need_iput_tmp) - __iget(inode); - else - need_iput_tmp = NULL; + __iget(inode); spin_unlock(&inode->i_lock); - - /* In case the dropping of a reference would nuke next_i. */ - while (&next_i->i_sb_list != list) { - spin_lock(&next_i->i_lock); - if (!(next_i->i_state & (I_FREEING | I_WILL_FREE)) && - atomic_read(&next_i->i_count)) { - __iget(next_i); - need_iput = next_i; - spin_unlock(&next_i->i_lock); - break; - } - spin_unlock(&next_i->i_lock); - next_i = list_entry(next_i->i_sb_list.next, - struct inode, i_sb_list); - } - - /* - * We can safely drop inode_sb_list_lock here because either - * we actually hold references on both inode and next_i or - * end of list. Also no new inodes will be added since the - * umount has begun. - */ spin_unlock(&inode_sb_list_lock); - if (need_iput_tmp) - iput(need_iput_tmp); + if (iput_inode) + iput(iput_inode); /* for each watch, send FS_UNMOUNT and then remove it */ fsnotify(inode, FS_UNMOUNT, inode, FSNOTIFY_EVENT_INODE, NULL, 0); fsnotify_inode_delete(inode); - iput(inode); + iput_inode = inode; spin_lock(&inode_sb_list_lock); } spin_unlock(&inode_sb_list_lock); + + if (iput_inode) + iput(iput_inode); } |