On Wed, Feb 25, 2009 at 12:28:33AM +0000, Al Viro wrote:
> On Tue, Feb 24, 2009 at 03:40:23PM -0800, Andrew Morton wrote:
> > ./Documentation/filesystems/inotify.txt
> > ./kernel/audit.c
> > ./kernel/audit_tree.c
> > ./fs/notify/inotify/inotify_user.c
> > ./fs/notify/inotify/inotify.c
> > ./include/linux/inotify.h
> >
> > I assume it's inotify_dev_queue_event()?
> >
> > if (mask & IN_IGNORED || w->mask & IN_ONESHOT)
> > put_inotify_watch(w); /* final put */
>
> Duh. Nevermind, I'd misparsed the report. If we are talking about
> inotify_user.c (and not some new client), then we are back to "let's
> see what gets leaked"...
>
> Actually, looking at inotify_user.c, we seem to be doing something rather
> fishy. Look: event gets triggered, we pick the watch, get inotify_device
> (inotify_user-specific stuff) from it, grap mutex on it (dev->ev_mutex)
> and drop reference to inotify_watch. Which happily triggers ->destroy_watch,
> which does put_inotify_dev(). Which is
> if (atomic_dec_and_test(&dev->count)) {
> atomic_dec(&dev->user->inotify_devs);
> free_uid(dev->user);
> kfree(dev);
> }
> What's to stop that from happening when we'd been holding the last reference
> to that sucker? kfree() while holding a mutex inside the structure being
> freed is not nice...
That shouldn't happen, we should only be doing the last put on the dev when all
watches have been removed. When we do the inotify_init we do the get on the
dev, and then for every watch theres a pair of get/put for instantiation and
removal, so we can't free the dev when doing a put for a watch (well obviously
we can, but we shouldn't be anyway). If the user is getting back -ENOSPC from
inotify then it can only mean we've run out of watches
if (atomic_read(&dev->user->inotify_watches) >=
inotify_max_user_watches)
return -ENOSPC;
is from create_watch in inotify_user.c. So it seems we are just forgetting to
do a put somewhere on our watch, or the user unknowingly hitting their max.
Thanks,
Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
