On Thu, May 13, 1999 at 11:25:57AM -0400, Alexander Viro wrote:
>       The thing that actually worries me here being that all analysis of
> potential races was done in assumption that mountpoints never move. Quite
> probably your patch *is* safe (with addition of loop check, that is). As
> for lookups - yes, you are right. It's not worse than renames (which are
> not race-free wrt lookups ;-/) and the same fix will work here too.
>       Well, it's 2.3, so... Let's try it and look whether it will give
> any troubles wrt NFS. Final word here belongs to Trond and Linus, indeed.

Great.  Here's the patch again, revised to include the loop check you
requested.  Your check missed a boundary condition (remounting / on itself),
but it's the right idea:

--- fs/super.c.linus    Wed May 12 14:10:46 1999
+++ fs/super.c  Thu May 13 22:32:45 1999
@@ -49,7 +49,7 @@
 
 extern int root_mountflags;
 
-static int do_remount_sb(struct super_block *sb, int flags, char * data);
+static int do_remount_sb(struct vfsmount *vfsmnt, int flags, char * data);
 
 /* this is initialized in init/main.c */
 kdev_t ROOT_DEV;
@@ -86,7 +86,6 @@
                }
 
        return ((struct vfsmount *)NULL);
-       /* NOTREACHED */
 }
 
 static struct vfsmount *add_vfsmnt(struct super_block *sb,
@@ -684,8 +683,11 @@
                 * we just try to remount it readonly.
                 */
                retval = 0;
-               if (!(sb->s_flags & MS_RDONLY))
-                       retval = do_remount_sb(sb, MS_RDONLY, 0);
+               if (!(sb->s_flags & MS_RDONLY)) {
+                       struct vfsmount *vfsmnt = lookup_vfsmnt(sb->s_dev);
+                       if (vfsmnt)
+                               retval = do_remount_sb(vfsmnt, MS_RDONLY, 0);
+               }
                return retval;
        }
 
@@ -911,11 +913,11 @@
  * FS-specific mount options can't be altered by remounting.
  */
 
-static int do_remount_sb(struct super_block *sb, int flags, char *data)
+static int do_remount_sb(struct vfsmount *vfsmnt, int flags, char *data)
 {
        int retval;
-       struct vfsmount *vfsmnt;
-       
+       struct super_block *sb = vfsmnt->mnt_sb;
+
        /*
         * Invalidate the inodes, as some mount options may be changed.
         * N.B. If we are changing media, we should check the return
@@ -936,35 +938,79 @@
                        return retval;
        }
        sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK);
-       vfsmnt = lookup_vfsmnt(sb->s_dev);
-       if (vfsmnt)
-               vfsmnt->mnt_flags = sb->s_flags;
+       vfsmnt->mnt_flags = sb->s_flags;
        return 0;
 }
 
-static int do_remount(const char *dir,int flags,char *data)
+static int do_remount(const char *dev, const char *dir, int flags, char *data)
 {
-       struct dentry *dentry;
+       struct dentry *dev_d, *ndir_d, *odir_d;
        int retval;
+       struct vfsmount *vfsmnt;
+       struct super_block *sb, *parent_sb;
 
-       dentry = namei(dir);
-       retval = PTR_ERR(dentry);
-       if (!IS_ERR(dentry)) {
-               struct super_block * sb = dentry->d_inode->i_sb;
+       dev_d = namei(dev);
+       retval = PTR_ERR(dev_d);
+       if (IS_ERR(dev_d))
+               return retval;
+       vfsmnt = lookup_vfsmnt(dev_d->d_inode->i_rdev);
+       retval = -EINVAL;
+       if (!vfsmnt)
+               goto out;
+       sb = vfsmnt->mnt_sb;
 
-               retval = -EINVAL;
-               if (dentry == sb->s_root) {
-                       /*
-                        * Shrink the dcache and sync the device.
-                        */
-                       shrink_dcache_sb(sb);
-                       fsync_dev(sb->s_dev);
-                       if (flags & MS_RDONLY)
-                               acct_auto_close(sb->s_dev);
-                       retval = do_remount_sb(sb, flags, data);
-               }
-               dput(dentry);
+       /*
+        * Shrink the dcache and sync the device.
+        */
+       shrink_dcache_sb(sb);
+       fsync_dev(sb->s_dev);
+       if (flags & MS_RDONLY)
+               acct_auto_close(sb->s_dev);
+       retval = do_remount_sb(vfsmnt, flags, data);
+       if (retval != 0)
+               goto out;
+
+       ndir_d = namei(dir);
+       retval = PTR_ERR(ndir_d);
+       if (IS_ERR(ndir_d))
+               goto out;
+
+       /* if this is the same mount point, there's no more to do. */
+       if (ndir_d == sb->s_root)
+               goto dput_and_out;
+       /* is this a directory? */
+       retval = -ENOTDIR;
+       if (!S_ISDIR(ndir_d->d_inode->i_mode))
+               goto dput_and_out;
+       /* is something else already mounted on this directory? */
+       retval = -EBUSY;
+       if (ndir_d->d_covers != ndir_d)
+               goto dput_and_out;
+       /* or is the new directory on this filesystem? */
+       parent_sb = ndir_d->d_sb;
+       while (parent_sb != parent_sb->s_root->d_covers->d_inode->i_sb) {
+               if (parent_sb == sb)
+                       goto dput_and_out;
+               parent_sb = parent_sb->s_root->d_covers->d_inode->i_sb;
        }
+       if (parent_sb == sb)
+               goto dput_and_out;
+
+       retval = 0;
+       odir_d = sb->s_root->d_covers;
+       ndir_d->d_mounts = odir_d->d_mounts;
+       odir_d->d_mounts = odir_d;
+       sb->s_root->d_covers = ndir_d;
+       ndir_d = odir_d;                        /* free the old one instead */
+
+       kfree(vfsmnt->mnt_dirname);
+       vfsmnt->mnt_dirname = (char *) kmalloc(strlen(dir)+1, GFP_KERNEL);
+       if (vfsmnt->mnt_dirname != NULL)
+               strcpy(vfsmnt->mnt_dirname, dir);
+dput_and_out:
+       dput(ndir_d);
+out:
+       dput(dev_d);
        return retval;
 }
 
@@ -1031,7 +1077,7 @@
                retval = copy_mount_options (data, &page);
                if (retval < 0)
                        goto out;
-               retval = do_remount(dir_name,
+               retval = do_remount(dev_name, dir_name,
                                    new_flags & ~MS_MGC_MSK & ~MS_REMOUNT,
                                    (char *) page);
                free_page(page);
-- 
Matthew Wilcox <[EMAIL PROTECTED]>
"Windows and MacOS are products, contrived by engineers in the service of
specific companies. Unix, by contrast, is not so much a product as it is a
painstakingly compiled oral history of the hacker subculture." - N Stephenson

Reply via email to