Race in the inotify debug code

Race in the inotify debug code

[Chuck Ebbert]

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248355

Description of problem:
Warnings in the kernel log (dmesg):
BUG: warning at fs/inotify.c:172/set_dentry_child_flags() (Not tainted)
 [<c0497a37>] set_dentry_child_flags+0x67/0x13d
 [<c0497c27>] remove_watch_no_event+0x2f/0x3b
 [<c0497d15>] inotify_remove_watch_locked+0x12/0x3e
 [<c0600c17>] mutex_lock+0x1a/0x29
 [<c0497fdd>] inotify_rm_wd+0x6d/0x8a
 [<c04983b1>] sys_inotify_rm_watch+0x38/0x4f
 [<c0404f70>] syscall_call+0x7/0xb

Appears randomly, about every second/third day.

Still happening in kernel 2.6.22.


static void set_dentry_child_flags(struct inode *inode, int watched)
...
        spin_lock(&dcache_lock);
        list_for_each_entry(alias, &inode->i_dentry, d_alias) {
                struct dentry *child;

                list_for_each_entry(child, &alias->d_subdirs, d_u.d_child) {
                        if (!child->d_inode) {
                                WARN_ON(child->d_flags & DCACHE_INOTIFY_PARENT_WATCHED);
                                continue;
                        }

But in dcache.c, the locks are dropped before this flag is cleared, leaving
a race window:

void d_delete(struct dentry * dentry)
...
        spin_lock(&dcache_lock);
        spin_lock(&dentry->d_lock);
        isdir = S_ISDIR(dentry->d_inode->i_mode);
        if (atomic_read(&dentry->d_count) == 1) {
                dentry_iput(dentry);  <================ drops dcache_lock and dentry->d_lock
                fsnotify_nameremove(dentry, isdir);

                /* remove this and other inotify debug checks after 2.6.18 */
                dentry->d_flags &= ~DCACHE_INOTIFY_PARENT_WATCHED;
                return;
        }

(The comment is nice, it says the debug code should have been removed long ago.)

Re: Race in the inotify debug code

[Andrew Morton]

> On Thu, 23 Aug 2007 11:25:18 -0400 Chuck Ebbert <cebbert@redhat.com> wrote:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248355
> 
> Description of problem:
> Warnings in the kernel log (dmesg):
> BUG: warning at fs/inotify.c:172/set_dentry_child_flags() (Not tainted)
>  [<c0497a37>] set_dentry_child_flags+0x67/0x13d
>  [<c0497c27>] remove_watch_no_event+0x2f/0x3b
>  [<c0497d15>] inotify_remove_watch_locked+0x12/0x3e
>  [<c0600c17>] mutex_lock+0x1a/0x29
>  [<c0497fdd>] inotify_rm_wd+0x6d/0x8a
>  [<c04983b1>] sys_inotify_rm_watch+0x38/0x4f
>  [<c0404f70>] syscall_call+0x7/0xb
> 
> Appears randomly, about every second/third day.
> 
> Still happening in kernel 2.6.22.
> 
> 
> static void set_dentry_child_flags(struct inode *inode, int watched)
> ...
>         spin_lock(&dcache_lock);
>         list_for_each_entry(alias, &inode->i_dentry, d_alias) {
>                 struct dentry *child;
> 
>                 list_for_each_entry(child, &alias->d_subdirs, d_u.d_child) {
>                         if (!child->d_inode) {
>                                 WARN_ON(child->d_flags & DCACHE_INOTIFY_PARENT_WATCHED);
>                                 continue;
>                         }
> 
> But in dcache.c, the locks are dropped before this flag is cleared, leaving
> a race window:
> 
> void d_delete(struct dentry * dentry)
> ...
>         spin_lock(&dcache_lock);
>         spin_lock(&dentry->d_lock);
>         isdir = S_ISDIR(dentry->d_inode->i_mode);
>         if (atomic_read(&dentry->d_count) == 1) {
>                 dentry_iput(dentry);  <================ drops dcache_lock and dentry->d_lock
>                 fsnotify_nameremove(dentry, isdir);
> 
>                 /* remove this and other inotify debug checks after 2.6.18 */
>                 dentry->d_flags &= ~DCACHE_INOTIFY_PARENT_WATCHED;
>                 return;
>         }
> 
> (The comment is nice, it says the debug code should have been removed long ago.)

We've been chasing this bug for a year or so.  Thanks for maybe-solving it.
I forwarded your email to Nick a few days ago but he's presently tied up
with kernel slummit.  Please let us not forget about this?

Re: Race in the inotify debug code

[Nick Piggin]

On Thursday 06 September 2007 02:05, Andrew Morton wrote:
> > On Thu, 23 Aug 2007 11:25:18 -0400 Chuck Ebbert <cebbert@redhat.com>
> > wrote: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248355
> >
> > Description of problem:
> > Warnings in the kernel log (dmesg):
> > BUG: warning at fs/inotify.c:172/set_dentry_child_flags() (Not tainted)
> >  [<c0497a37>] set_dentry_child_flags+0x67/0x13d
> >  [<c0497c27>] remove_watch_no_event+0x2f/0x3b
> >  [<c0497d15>] inotify_remove_watch_locked+0x12/0x3e
> >  [<c0600c17>] mutex_lock+0x1a/0x29
> >  [<c0497fdd>] inotify_rm_wd+0x6d/0x8a
> >  [<c04983b1>] sys_inotify_rm_watch+0x38/0x4f
> >  [<c0404f70>] syscall_call+0x7/0xb
> >
> > Appears randomly, about every second/third day.
> >
> > Still happening in kernel 2.6.22.
> >
> >
> > static void set_dentry_child_flags(struct inode *inode, int watched)
> > ...
> >         spin_lock(&dcache_lock);
> >         list_for_each_entry(alias, &inode->i_dentry, d_alias) {
> >                 struct dentry *child;
> >
> >                 list_for_each_entry(child, &alias->d_subdirs,
> > d_u.d_child) { if (!child->d_inode) {
> >                                 WARN_ON(child->d_flags &
> > DCACHE_INOTIFY_PARENT_WATCHED); continue;
> >                         }
> >
> > But in dcache.c, the locks are dropped before this flag is cleared,
> > leaving a race window:
> >
> > void d_delete(struct dentry * dentry)
> > ...
> >         spin_lock(&dcache_lock);
> >         spin_lock(&dentry->d_lock);
> >         isdir = S_ISDIR(dentry->d_inode->i_mode);
> >         if (atomic_read(&dentry->d_count) == 1) {
> >                 dentry_iput(dentry);  <================ drops dcache_lock
> > and dentry->d_lock fsnotify_nameremove(dentry, isdir);
> >
> >                 /* remove this and other inotify debug checks after
> > 2.6.18 */ dentry->d_flags &= ~DCACHE_INOTIFY_PARENT_WATCHED; return;
> >         }
> >
> > (The comment is nice, it says the debug code should have been removed
> > long ago.)
>
> We've been chasing this bug for a year or so.  Thanks for maybe-solving it.
> I forwarded your email to Nick a few days ago but he's presently tied up
> with kernel slummit.  Please let us not forget about this?


There is some race in the debug code, yes, but I think there could also be
a real race in there too. I've posted a trial patch for it in one of the other
inotify bug reports.

Anyway, yes I'm inclined to just fix that and rip out the debug code. OTOH,
I have been trying to get some tester to reproduce with patches and had
no takers as yet...

BTW, I will be away for the next few weeks so I'll be going slower than usual