fs: make sure to call fsnotify_inoderemove() only once when inode is dead
Subsystems (2)
Jan Kara
reviewerPATCH 1/1 fs: make sure to call fsnotify_inoderemove() only once when inode is dead
From: Amir Goldstein <[email protected]>
Daehyeon reported a race wherein fsnotify_removeinode() can be called when inode still has a live alias. This can lead to several undesired outcomes such as crashes in fsnotify code or avoidance of inode mark notifications.
Check that inode is really "dead", meaning no nlink and no more aliases before calling fsnotify_inoderemove() for final cleanup of inode marks. Also do not allow open_by_handle() to add new aliases to a "dead" inode.
Ideally, we'd call fsnotify_inoderemove() once last inode reference is dropped to make things simpler. Alas fsnotify inode marks currently hold inode references and thus this doesn't work. Once we teach fsnotify to avoid holding inode references, these games with dead inodes can go as well.
--- fs/dcache.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)
fs/dcache.c +24 -3
@@ -450,6 +450,17 @@ static void dentry_free(struct dentry *dentry)call_rcu (&dentry->d_rcu, __d_free); } +/* +* If inode is unlinked and doesn't have any aliases (i.e., all fds pointing to +* it are closed), it is pretty much dead. Except that file handle lookup could +* still revive it which causes issues to fsnotify. So once inode reaches this +* state we make sure to block creating any new aliases. +*/ +static bool inode_notify_dead (struct inode *inode) +{ +return !inode->i_nlink &&hlist_empty (&inode->i_dentry); +} +/* * Release the dentry's inode, using the filesystem * d_iput() operation if defined.
@@ -459,6 +470,7 @@ static void dentry_unlink_inode(struct dentry * dentry) __releases(dentry->d_inode->i_lock) {struct inode *inode = dentry->d_inode; +bool notify_dead;raw_write_seqcount_begin (&dentry->d_seq);__d_clear_type_and_inode (dentry);
@@ -469,9 +481,10 @@ static void dentry_unlink_inode(struct dentry * dentry)*/ dentry->waiters =NULL ;raw_write_seqcount_end (&dentry->d_seq); + notify_dead =inode_notify_dead (inode);spin_unlock (&dentry->d_lock);spin_unlock (&inode->i_lock); -if (!inode->i_nlink) +if (notify_dead)fsnotify_inoderemove (inode);if (dentry->d_op && dentry->d_op->d_iput) dentry->d_op->d_iput (dentry, inode);
@@ -2237,7 +2250,12 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected) sb = inode->i_sb; - res =d_find_any_alias (inode);/* existing alias? */ +spin_lock (&inode->i_lock); +if (!inode_notify_dead (inode)) + res =__d_find_any_alias (inode);/* existing alias? */ +else + res =ERR_PTR (-ESTALE ); +spin_unlock (&inode->i_lock);if (res) goto out;
@@ -2249,7 +2267,10 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)security_d_instantiate (new, inode);spin_lock (&inode->i_lock); - res =__d_find_any_alias (inode);/* recheck under lock */ +if (!inode_notify_dead (inode)) + res =__d_find_any_alias (inode);/* recheck under lock */ +else + res =ERR_PTR (-ESTALE );if (likely (!res)) {/* still no alias, attach a disconnected dentry */ unsigned add_flags =d_flags_for_inode (inode);
Signature · 1 line
2.51.0
Quoted 1 line · Jan Kara
fs: make sure to call
fsnotify_inoderemove()only once when inode is dead
Sheesh. Not pretty. :)
---
Applied to the vfs-7.4.misc branch of the vfs/vfs.git tree. Patches in the vfs-7.4.misc branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs-7.4.misc
[1/1] fs: make sure to call fsnotify_inoderemove() only once when inode is dead
https://git.kernel.org/vfs/vfs/c/669bb5af15d4