Project

General

Profile

Actions
Copy link

Bug #7454

closed

renaming of mount points should not be allowed

Added by Gordon Ross almost 7 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Gordon Ross
Category:
kernel
Start date:
2016-10-06
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:
External Bug:

Description

Interesting little surprise:

zfs create tank/test
zfs create tank/test/sub1
mv /tank/fs/sub1 /tank/fs/sub2
(no error)

If there are shares, (i.e. sharesmb=name=sub1 on tank/test/sub1)
then this leaves the sub1 share inaccessible, and
the system is quite confused, i.e. sharemanager still shows
shares pointing to the old path, etc.

We really should not allow renaming a mount point.

Actions
Copy link
 #1

Updated by Gordon Ross almost 7 years ago

This has been there for a long time. We're missing a check in vn_renameat.
Interestingly, vn_removeat already has a similar check.

Here's a simple fix:

diff --git a/usr/src/uts/common/fs/vnode.c b/usr/src/uts/common/fs/vnode.c
index 79851dd..a0928ac 100644
--- a/usr/src/uts/common/fs/vnode.c
+++ b/usr/src/uts/common/fs/vnode.c
@@ -1692,6 +1692,16 @@ top:
                goto out;
        }

+       /*
+        * Make sure "from" vp is not a mount point.
+        * Note, lookup did traverse() already, so
+        * we'll be looking at the mounted FS.
+        */
+       if ((fvp->v_flag & VROOT) != 0) {
+               error = EBUSY;
+               goto out;
+       }
+
        if (auditing && tdvp != NULL)
                audit_setfsat_path(3);
        if (error = lookuppnat(&tpn, NULL, NO_FOLLOW, &tovp, &targvp, tdvp)) {
Actions
Copy link
 #2

Updated by Electric Monk almost 7 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit cd00b393e31079bcde69cb2775a2a639eee9fd59

commit  cd00b393e31079bcde69cb2775a2a639eee9fd59
Author: Gordon Ross <gwr@nexenta.com>
Date:   2016-10-28T18:16:09.000Z

    7454 renaming of mount points should not be allowed
    Reviewed by: Igor Kozhukhov <ikozhukhov@gmail.com>
    Reviewed by: Andy Stormont <astormont@racktopsystems.com>
    Approved by: Matthew Ahrens <mahrens@delphix.com>

Actions
Copy link
 #3

Updated by Vitaliy Gusev over 2 years ago

It seems better is moving check to zfs_rename(), because local renaming is checked fine, but if do renaming from SMB/NFS clients side, protection doesn't work.

So it still is possible to rename mountpoint from remote side. If you don't mind, I will prepare fix with moving check code to zfs_rename().

Actions
Copy link
 #4

Updated by Joshua M. Clulow over 2 years ago

You'll need to open a new bug describing the defect and get review for your proposed fix. You can link it as a related issue to this one.

Actions
Copy link

Also available in: Atom PDF