SRU Bug #104837 - kernel warns BUG: at fs/inotify.c:172 set_dentry_child_flags()

Stefan Bader stefan.bader at canonical.com
Wed Jul 2 17:32:13 UTC 2008


Tim Gardner wrote:
> SRU Justification
> 
> Impact: There is a race between setting an inode's children's "parent watched" flag when placing the first watch on a parent, and instantiating new children of that parent: a child could miss having its flags set by set_dentry_child_flags, but then inotify_d_instantiate might still see !inotify_inode_watched.
> 
> Patch Description: The solution is to set_dentry_child_flags after adding the watch. Locking is taken care of, because both set_dentry_child_flags and inotify_d_instantiate hold dcache_lock and child->d_locks.
> 
> Patches: http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=commit;h=ab67a74144886e464f5b905558876145e711f17a, http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=commit;h=670dcd8597ed0aa7f68d05d384226bdb81b0e956
> 
> Test Case: See bug description.
> 
> From ab67a74144886e464f5b905558876145e711f17a Mon Sep 17 00:00:00 2001
> From: Nick Piggin <npiggin at suse.de>
> Date: Wed, 6 Feb 2008 01:37:28 -0800
> Subject: [PATCH] inotify: fix race
>  Bug: #104837
> 
> There is a race between setting an inode's children's "parent watched" flag
> when placing the first watch on a parent, and instantiating new children of
> that parent: a child could miss having its flags set by
> set_dentry_child_flags, but then inotify_d_instantiate might still see
> !inotify_inode_watched.
> 
> The solution is to set_dentry_child_flags after adding the watch.  Locking is
> taken care of, because both set_dentry_child_flags and inotify_d_instantiate
> hold dcache_lock and child->d_locks.
> 
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> Signed-off-by: Nick Piggin <npiggin at suse.de>
> Cc: Robert Love <rlove at google.com>
> Cc: John McCutchan <ttb at tentacle.dhs.org>
> Cc: Jan Kara <jack at ucw.cz>
> Cc: Yan Zheng <yanzheng at 21cn.com>
> Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> ---
>  fs/inotify.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/inotify.c b/fs/inotify.c
> index 2c5b921..b2b109b 100644
> --- a/fs/inotify.c
> +++ b/fs/inotify.c
> @@ -627,6 +627,7 @@ s32 inotify_add_watch(struct inotify_handle *ih, struct inotify_watch *watch,
>  		      struct inode *inode, u32 mask)
>  {
>  	int ret = 0;
> +	int newly_watched;
>  
>  	/* don't allow invalid bits: we don't want flags set */
>  	mask &= IN_ALL_EVENTS | IN_ONESHOT;
> @@ -653,12 +654,18 @@ s32 inotify_add_watch(struct inotify_handle *ih, struct inotify_watch *watch,
>  	 */
>  	watch->inode = igrab(inode);
>  
> -	if (!inotify_inode_watched(inode))
> -		set_dentry_child_flags(inode, 1);
> -
>  	/* Add the watch to the handle's and the inode's list */
> +	newly_watched = !inotify_inode_watched(inode);
>  	list_add(&watch->h_list, &ih->watches);
>  	list_add(&watch->i_list, &inode->inotify_watches);
> +	/*
> +	 * Set child flags _after_ adding the watch, so there is no race
> +	 * windows where newly instantiated children could miss their parent's
> +	 * watched flag.
> +	 */
> +	if (newly_watched)
> +		set_dentry_child_flags(inode, 1);
> +
>  out:
>  	mutex_unlock(&ih->mutex);
>  	mutex_unlock(&inode->inotify_mutex);
Ack




More information about the kernel-team mailing list