| Message ID | 20160224151511.GR17997@ZenIV.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 24, 2016 at 4:15 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Wed, Feb 24, 2016 at 02:35:18PM +0100, Dmitry Vyukov wrote: > >> The warning is this one: >> >> static inline int should_follow_link(struct nameidata *nd, struct path *link, >> int follow, >> struct inode *inode, unsigned seq) >> { >> .... >> WARN_ON(!inode); // now, _that_ should not happen. >> return pick_link(nd, link, inode, seq); >> } > > Let's try it with less chatty should_follow_link() and better set of > tripwires on the way to it: Humm... I've left it running over night but no GPFs happened... Usually they happened within two hours or so. I would think that your patch fixes it and I did not actually apply it last time (or did not rebuild kernel). But I saw the new warnings that the patch adds, so I should have been rebuilt it... What I saw is a dozen of pairs of warnings like the one below. Is it possible the warning printing introduces enough delay to close the inconsistency window?.... [46839.557154] ------------[ cut here ]------------ [46839.557663] WARNING: CPU: 0 PID: 10503 at fs/namei.c:1587 lookup_fast+0x2bf/0x420() [46839.558239] Modules linked in: [46839.558466] CPU: 0 PID: 10503 Comm: syz-executor Tainted: G W 4.5.0-rc5+ #73 [46839.558980] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 [46839.559174] 0000000000000000 ffff88002b6ebc48 ffffffff8194acd9 0000000000000000 [46839.559564] ffffffff83344ffc ffff88002b6ebc80 ffffffff81172291 ffff88002b6ebde0 [46839.559564] ffff8800314cc720 ffff88002b6ebd90 ffff88002b6ebd98 ffff88002b6ebd8c [46839.560614] Call Trace: [46839.560614] [<ffffffff8194acd9>] dump_stack+0x99/0xd0 [46839.561294] [<ffffffff81172291>] warn_slowpath_common+0x81/0xc0 [46839.561712] [<ffffffff81172385>] warn_slowpath_null+0x15/0x20 [46839.561712] [<ffffffff81314ddf>] lookup_fast+0x2bf/0x420 [46839.561712] [<ffffffff81315568>] ? link_path_walk+0x68/0x4e0 [46839.561712] [<ffffffff813172b5>] path_openat+0x375/0x1520 [46839.563137] [<ffffffff81319499>] do_filp_open+0x79/0xd0 [46839.563137] [<ffffffff82b11cf2>] ? _raw_spin_unlock+0x22/0x30 [46839.563137] [<ffffffff81328e58>] ? __alloc_fd+0xf8/0x200 [46839.564353] [<ffffffff81306eb0>] do_sys_open+0x110/0x1f0 [46839.564353] [<ffffffff81306fbf>] SyS_openat+0xf/0x20 [46839.564353] [<ffffffff82b12776>] entry_SYSCALL_64_fastpath+0x16/0x7a [46839.565558] ---[ end trace 3c3bc0f927bf4e90 ]--- [46839.565858] ------------[ cut here ]------------ [46839.566203] WARNING: CPU: 0 PID: 10503 at fs/namei.c:3121 path_openat+0x12bc/0x1520() [46839.566700] Modules linked in: [46839.566914] CPU: 0 PID: 10503 Comm: syz-executor Tainted: G W 4.5.0-rc5+ #73 [46839.567154] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 [46839.567154] 0000000000000000 ffff88002b6ebcb8 ffffffff8194acd9 0000000000000000 [46839.567154] ffffffff83344ffc ffff88002b6ebcf0 ffffffff81172291 0000000000000005 [46839.567154] ffff88002b6ebd98 0000000000048000 ffff88002b6ebde0 ffff88002b6ebefc [46839.567154] Call Trace: [46839.567154] [<ffffffff8194acd9>] dump_stack+0x99/0xd0 [46839.567154] [<ffffffff81172291>] warn_slowpath_common+0x81/0xc0 [46839.567154] [<ffffffff81172385>] warn_slowpath_null+0x15/0x20 [46839.567154] [<ffffffff813181fc>] path_openat+0x12bc/0x1520 [46839.567154] [<ffffffff81319499>] do_filp_open+0x79/0xd0 [46839.567154] [<ffffffff82b11cf2>] ? _raw_spin_unlock+0x22/0x30 [46839.567154] [<ffffffff81328e58>] ? __alloc_fd+0xf8/0x200 [46839.567154] [<ffffffff81306eb0>] do_sys_open+0x110/0x1f0 [46839.567154] [<ffffffff81306fbf>] SyS_openat+0xf/0x20 [46839.567154] [<ffffffff82b12776>] entry_SYSCALL_64_fastpath+0x16/0x7a [46839.573763] ---[ end trace 3c3bc0f927bf4e91 ]--- -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index c6d7d3d..86f81e3 100644 --- a/fs/autofs4/root.c +++ b/fs/autofs4/root.c @@ -323,6 +323,7 @@ static struct dentry *autofs4_mountpoint_changed(struct path *path) struct dentry *new = d_lookup(parent, &dentry->d_name); if (!new) return NULL; + WARN_ON(d_is_negative(new)); ino = autofs4_dentry_ino(new); ino->last_used = jiffies; dput(path->dentry); diff --git a/fs/namei.c b/fs/namei.c index f624d13..071a4ba 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1209,6 +1209,7 @@ static int follow_managed(struct path *path, struct nameidata *nd) /* Handle an automount point */ if (managed & DCACHE_NEED_AUTOMOUNT) { ret = follow_automount(path, nd, &need_mntput); + WARN_ON(d_is_negative(path->dentry)); if (ret < 0) break; continue; @@ -1582,8 +1583,10 @@ static int lookup_fast(struct nameidata *nd, return -ENOENT; path->mnt = mnt; path->dentry = dentry; - if (likely(__follow_mount_rcu(nd, path, inode, seqp))) + if (likely(__follow_mount_rcu(nd, path, inode, seqp))) { + WARN_ON(!*inode); return 0; + } unlazy: if (unlazy_walk(nd, dentry, seq)) return -ECHILD; @@ -1613,8 +1616,10 @@ unlazy: path->mnt = mnt; path->dentry = dentry; err = follow_managed(path, nd); - if (likely(!err)) + if (likely(!err)) { *inode = d_backing_inode(path->dentry); + WARN_ON(!*inode); + } return err; need_lookup: @@ -1712,6 +1717,12 @@ static inline int should_follow_link(struct nameidata *nd, struct path *link, return 0; if (!follow) return 0; + /* make sure that d_is_symlink above matches inode */ + if (nd->flags & LOOKUP_RCU) { + if (read_seqcount_retry(&link->dentry->d_seq, seq)) + return -ECHILD; + } + WARN_ON(!inode); // now, _that_ should not happen. return pick_link(nd, link, inode, seq); } @@ -1743,11 +1754,11 @@ static int walk_component(struct nameidata *nd, int flags) if (err < 0) return err; - inode = d_backing_inode(path.dentry); seq = 0; /* we are already out of RCU mode */ err = -ENOENT; if (d_is_negative(path.dentry)) goto out_path_put; + inode = d_backing_inode(path.dentry); } if (flags & WALK_PUT) @@ -3106,8 +3117,10 @@ static int do_last(struct nameidata *nd, nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY; /* we _can_ be in RCU mode here */ error = lookup_fast(nd, &path, &inode, &seq); - if (likely(!error)) + if (likely(!error)) { + WARN_ON(!inode); goto finish_lookup; + } if (error < 0) return error; @@ -3192,12 +3205,13 @@ retry_lookup: return error; BUG_ON(nd->flags & LOOKUP_RCU); - inode = d_backing_inode(path.dentry); seq = 0; /* out of RCU mode, so the value doesn't matter */ if (unlikely(d_is_negative(path.dentry))) { path_to_nameidata(&path, nd); return -ENOENT; } + inode = d_backing_inode(path.dentry); + WARN_ON(!inode); finish_lookup: if (nd->depth) put_link(nd); @@ -3206,11 +3220,6 @@ finish_lookup: if (unlikely(error)) return error; - if (unlikely(d_is_symlink(path.dentry)) && !(open_flag & O_PATH)) { - path_to_nameidata(&path, nd); - return -ELOOP; - } - if ((nd->flags & LOOKUP_RCU) || nd->path.mnt != path.mnt) { path_to_nameidata(&path, nd); } else { @@ -3229,6 +3238,10 @@ finish_open: return error; } audit_inode(nd->name, nd->path.dentry, 0); + if (unlikely(d_is_symlink(nd->path.dentry)) && !(open_flag & O_PATH)) { + error = -ELOOP; + goto out; + } error = -EISDIR; if ((open_flag & O_CREAT) && d_is_dir(nd->path.dentry)) goto out; @@ -3273,6 +3286,10 @@ opened: goto exit_fput; } out: + if (unlikely(error > 0)) { + WARN_ON(1); + error = -EINVAL; + } if (got_write) mnt_drop_write(nd->path.mnt); path_put(&save_parent);