+static struct inode * find_inode(const char __user *dirname)
+{
+ struct inode *inode;
+ struct nameidata nd;
+ int error;
+
+ error = __user_walk(dirname, LOOKUP_FOLLOW, &nd);
+ if (error)
+ return ERR_PTR(error);
+
+ inode = nd.dentry->d_inode;
+
+ /* you can only watch an inode if you have read permissions on it */
+ error = permission(inode, MAY_READ, NULL);
+ if (error) {
+ inode = ERR_PTR(error);
+ goto release_and_out;
+ }
+
+ spin_lock(&inode_lock);
+ __iget(inode);
+ spin_unlock(&inode_lock);
+release_and_out:
+ path_release(&nd);
+ return inode;
+}
Yawn... OK, so what happens if we get umount in the middle of your
find_inode(), so that path_release() in there drops the last remaining
reference to vfsmount (and superblock)?
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Al Viro wrote:
> +static struct inode * find_inode(const char __user *dirname)
> +{
> + struct inode *inode;
> + struct nameidata nd;
> + int error;
> +
> + error = __user_walk(dirname, LOOKUP_FOLLOW, &nd);
> + if (error)
> + return ERR_PTR(error);
> +
> + inode = nd.dentry->d_inode;
> +
> + /* you can only watch an inode if you have read permissions on it */
> + error = permission(inode, MAY_READ, NULL);
> + if (error) {
> + inode = ERR_PTR(error);
> + goto release_and_out;
> + }
> +
> + spin_lock(&inode_lock);
> + __iget(inode);
> + spin_unlock(&inode_lock);
> +release_and_out:
> + path_release(&nd);
> + return inode;
> +}
>
> Yawn... OK, so what happens if we get umount in the middle of your
> find_inode(), so that path_release() in there drops the last remaining
> reference to vfsmount (and superblock)?
How about inotify hold on to the nameidata until it is sure to have
updated the watches? That way, the inode will be seen by
inotify_super_block_umount when called by generic_shutdown_super.
There remains a small race though, in that the inotify ioctl may return
success for an added watch after the umount, but this would have to be
resolved by synchronizing in userspace anyway.
Incremental patch attached.
- --
Mike Waychison
Sun Microsystems, Inc.
1 (650) 352-5299 voice
1 (416) 202-8336 voice
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: The opinions expressed in this email are held by me,
and may not represent the views of Sun Microsystems, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFB9rpKdQs4kOxk3/MRAl0RAJ9vssocI3P93AnfO+zwyhf/BX1V9wCffy9N
r6t7TV0ZxpT3bT5pcYfpCHk=
=RmBk
-----END PGP SIGNATURE-----
On Tue, 2005-01-25 at 16:29 -0500, Mike Waychison wrote:
> How about inotify hold on to the nameidata until it is sure to have
> updated the watches? That way, the inode will be seen by
> inotify_super_block_umount when called by generic_shutdown_super.
I was chatting with John as soon as Al pointed out the problem and
suggested we look at something like this. I am glad you were able to
crank it out so readily. Thank you!
Barring a more elegant solution, I think just holding nameidata down
until the watches are added is the way to go. We will test this and
then merge it into the next patch.
Thanks,
Robert Love