On Tue, Feb 24, 2009 at 09:38:50PM -0500, Josef Bacik wrote:
> > 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).
Hold on. Either that sucker can happen after inotify_release() has dropped
its reference or it can not. In the former case, we are screwed since _we_
are holding the last remaining reference. In the latter, WTF are we grabbing
references to ->dev at watch creation?
> 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;
... or that idr_pre_get() has failed in inotify.c
--
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
