History

reimplement path_mountpoint() with less magic

... and get rid of a bunch of bugs in it. Background: the reason for path_mountpoint() is that umount() really doesn't want attempts to revalidate the root of what it's trying to umount. The thing we want to avoid actually happen from complete_walk(); solution was to do something parallel to normal path_lookupat() and it both went overboard and got the boilerplate subtly (and not so subtly) wrong. A better solution is to do pretty much what the normal path_lookupat() does, but instead of complete_walk() do unlazy_walk(). All it takes to avoid that ->d_weak_revalidate() call... mountpoint_last() goes away, along with everything it got wrong, and so does the magic around LOOKUP_NO_REVAL. Another source of bugs is that when we traverse mounts at the final location (and we need to do that - umount . expects to get whatever's overmounting ., if any, out of the lookup) we really ought to take care of ->d_manage() - as it is, manual umount of autofs automount in progress can lead to unpleasant surprises for the daemon. Easily solved by using handle_lookup_down() instead of follow_mount().

Al Viro committed 6 years ago

Commit c64cd6e1 parent 1edc8eb3 files · +12 −80Browse files

Unified · Split

3 files
  1. fs/namei.c +12 −77
  2. fs/nfs/nfstrace.h +0 −2
  3. include/linux/namei.h +0 −1
fs/namei.c+12−77
@@ -1649,17 +1649,15 @@ again: · 1648 unmodified lines
16491649 if (IS_ERR(dentry))
16501650 return dentry;
16511651 if (unlikely(!d_in_lookup(dentry))) {
1652− if (!(flags & LOOKUP_NO_REVAL)) {
1653− int error = d_revalidate(dentry, flags);
1654− if (unlikely(error <= 0)) {
1655− if (!error) {
1656− d_invalidate(dentry);
1657− dput(dentry);
1658− goto again;
1659− }
1652+ int error = d_revalidate(dentry, flags);
1653+ if (unlikely(error <= 0)) {
1654+ if (!error) {
1655+ d_invalidate(dentry);
16601656 dput(dentry);
1661− dentry = ERR_PTR(error);
1657+ goto again;
16621658 }
1659+ dput(dentry);
1660+ dentry = ERR_PTR(error);
16631661 }
16641662 } else {
16651663 old = inode->i_op->lookup(inode, dentry, flags);
@@ -2617,72 +2615,6 @@ int user_path_at_empty(int dfd, const char __user *name, unsigned flags, · 951 unmodified lines
26172615 }
26182616 EXPORT_SYMBOL(user_path_at_empty);
26192617
2620−/**
2621− * mountpoint_last - look up last component for umount
2622− * @nd: pathwalk nameidata - currently pointing at parent directory of "last"
2623− *
2624− * This is a special lookup_last function just for umount. In this case, we
2625− * need to resolve the path without doing any revalidation.
2626− *
2627− * The nameidata should be the result of doing a LOOKUP_PARENT pathwalk. Since
2628− * mountpoints are always pinned in the dcache, their ancestors are too. Thus,
2629− * in almost all cases, this lookup will be served out of the dcache. The only
2630− * cases where it won't are if nd->last refers to a symlink or the path is
2631− * bogus and it doesn't exist.
2632− *
2633− * Returns:
2634− * -error: if there was an error during lookup. This includes -ENOENT if the
2635− * lookup found a negative dentry.
2636− *
2637− * 0: if we successfully resolved nd->last and found it to not to be a
2638− * symlink that needs to be followed.
2639− *
2640− * 1: if we successfully resolved nd->last and found it to be a symlink
2641− * that needs to be followed.
2642− */
2643−static int
2644−mountpoint_last(struct nameidata *nd)
2645−{
2646− int error = 0;
2647− struct dentry *dir = nd->path.dentry;
2648− struct path path;
2649−
2650− /* If we're in rcuwalk, drop out of it to handle last component */
2651− if (nd->flags & LOOKUP_RCU) {
2652− if (unlazy_walk(nd))
2653− return -ECHILD;
2654− }
2655−
2656− nd->flags &= ~LOOKUP_PARENT;
2657−
2658− if (unlikely(nd->last_type != LAST_NORM)) {
2659− error = handle_dots(nd, nd->last_type);
2660− if (error)
2661− return error;
2662− path.dentry = dget(nd->path.dentry);
2663− } else {
2664− path.dentry = d_lookup(dir, &nd->last);
2665− if (!path.dentry) {
2666− /*
2667− * No cached dentry. Mounted dentries are pinned in the
2668− * cache, so that means that this dentry is probably
2669− * a symlink or the path doesn't actually point
2670− * to a mounted dentry.
2671− */
2672− path.dentry = lookup_slow(&nd->last, dir,
2673− nd->flags | LOOKUP_NO_REVAL);
2674− if (IS_ERR(path.dentry))
2675− return PTR_ERR(path.dentry);
2676− }
2677− }
2678− if (d_flags_negative(smp_load_acquire(&path.dentry->d_flags))) {
2679− dput(path.dentry);
2680− return -ENOENT;
2681− }
2682− path.mnt = nd->path.mnt;
2683− return step_into(nd, &path, 0, d_backing_inode(path.dentry), 0);
2684−}
2685−
26862618 /**
26872619 * path_mountpoint - look up a path to be umounted
26882620 * @nd: lookup context
@@ -2699,14 +2631,17 @@ path_mountpoint(struct nameidata *nd, unsigned flags, struct path *path) · 10 unmodified lines
26992631 int err;
27002632
27012633 while (!(err = link_path_walk(s, nd)) &&
2702− (err = mountpoint_last(nd)) > 0) {
2634+ (err = lookup_last(nd)) > 0) {
27032635 s = trailing_symlink(nd);
27042636 }
2637+ if (!err && (nd->flags & LOOKUP_RCU))
2638+ err = unlazy_walk(nd);
2639+ if (!err)
2640+ err = handle_lookup_down(nd);
27052641 if (!err) {
27062642 *path = nd->path;
27072643 nd->path.mnt = NULL;
27082644 nd->path.dentry = NULL;
2709− follow_mount(path);
27102645 }
27112646 terminate_walk(nd);
27122647 return err;
fs/nfs/nfstrace.h+0−2
@@ -206,7 +206,6 @@ TRACE_DEFINE_ENUM(LOOKUP_AUTOMOUNT); · 205 unmodified lines
206206 TRACE_DEFINE_ENUM(LOOKUP_PARENT);
207207 TRACE_DEFINE_ENUM(LOOKUP_REVAL);
208208 TRACE_DEFINE_ENUM(LOOKUP_RCU);
209−TRACE_DEFINE_ENUM(LOOKUP_NO_REVAL);
210209 TRACE_DEFINE_ENUM(LOOKUP_OPEN);
211210 TRACE_DEFINE_ENUM(LOOKUP_CREATE);
212211 TRACE_DEFINE_ENUM(LOOKUP_EXCL);
@@ -224,7 +223,6 @@ TRACE_DEFINE_ENUM(LOOKUP_DOWN); · 11 unmodified lines
224223 { LOOKUP_PARENT, "PARENT" }, \
225224 { LOOKUP_REVAL, "REVAL" }, \
226225 { LOOKUP_RCU, "RCU" }, \
227− { LOOKUP_NO_REVAL, "NO_REVAL" }, \
228226 { LOOKUP_OPEN, "OPEN" }, \
229227 { LOOKUP_CREATE, "CREATE" }, \
230228 { LOOKUP_EXCL, "EXCL" }, \
include/linux/namei.h+0−1
@@ -34,7 +34,6 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND}; · 33 unmodified lines
3434
3535 /* internal use only */
3636 #define LOOKUP_PARENT 0x0010
37−#define LOOKUP_NO_REVAL 0x0080
3837 #define LOOKUP_JUMPED 0x1000
3938 #define LOOKUP_ROOT 0x2000
4039 #define LOOKUP_ROOT_GRABBED 0x0008