Messages in this thread | | | Date | Fri, 28 Nov 2008 05:42:41 +0000 | From | Al Viro <> | Subject | Re: [PATCH -v3 7/8] fsnotify: add in inode fsnotify markings |
| |
On Tue, Nov 25, 2008 at 12:21:28PM -0500, Eric Paris wrote:
> +void fsnotify_mark_get(struct fsnotify_mark_entry *entry) > +{ > + spin_lock(&entry->lock); > + entry->refcnt++; > + spin_unlock(&entry->lock); > +}
> +void fsnotify_mark_put(struct fsnotify_mark_entry *entry) > +{ > + spin_lock(&entry->lock); > + entry->refcnt--; > + /* if (!refcnt && killme) we are off both lists and nothing else can find us. */ > + if ((!entry->refcnt) && (entry->killme)) { > + spin_unlock(&entry->lock); > + fsnotify_mark_kill(entry); > + return; > + } > + spin_unlock(&entry->lock); > +}
Uh-huh... And what happens if fsnotify_mark_get() comes in the middle of final fsnotify_mark_put()? You spin on entry->lock, gain it just before fsnotify_mark_kill() which proceeds to kfree entry under you just as you increment its refcnt...
> +void fsnotify_clear_mark_group(struct fsnotify_group *group) > +{ > + struct fsnotify_mark_entry *entry; > + struct inode *inode; > + > + mutex_lock(&group->mark_mutex); > + while (!list_empty(&group->mark_entries)) { > + entry = list_first_entry(&group->mark_entries, struct fsnotify_mark_entry, g_list); > + > + /* make sure the entry survives until it is off both lists */ > + fsnotify_mark_get(entry); > + > + /* remove from g_list */ > + list_del_init(&entry->g_list); > + mutex_unlock(&group->mark_mutex); > + > + inode = entry->inode; > + > + spin_lock(&entry->lock); > + entry->killme = 1; > + spin_unlock(&entry->lock); > > + /* remove from i_list */ > + spin_lock(&inode->i_fsnotify_lock);
... and just what would keep the inode from being freed under you here?
| |