| Message ID | 20150316044320.23648.45500.stgit@notabene.brown (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 16, 2015 at 03:43:20PM +1100, NeilBrown wrote: > When we support ->follow_link in RCU-walk we will not want to > take a reference to the 'struct path *link' passed to follow_link, > and correspondingly will not want to drop that reference. > > As link_path_walk will complete_walk() in the case of an error, > and as complete_walk() will clear LOOKUP_RCU, we cannot test > LOOKUP_RCU to determine if the path should be 'put'. > > So introduce a new flag: LOOKUP_LINK_RCU. This is set on > entry to follow_link() if appropriate and put_link() will > only call path_put() if it is clear. Umm... How is it different from nd->depth > 0 && nd->flags & LOOKUP_RCU? IOW, could we bump nd->depth before that (conditional) mntget()? -- 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
On Mon, Mar 16, 2015 at 10:33:45PM +0000, Al Viro wrote: > On Mon, Mar 16, 2015 at 03:43:20PM +1100, NeilBrown wrote: > > When we support ->follow_link in RCU-walk we will not want to > > take a reference to the 'struct path *link' passed to follow_link, > > and correspondingly will not want to drop that reference. > > > > As link_path_walk will complete_walk() in the case of an error, > > and as complete_walk() will clear LOOKUP_RCU, we cannot test > > LOOKUP_RCU to determine if the path should be 'put'. > > > > So introduce a new flag: LOOKUP_LINK_RCU. This is set on > > entry to follow_link() if appropriate and put_link() will > > only call path_put() if it is clear. > > Umm... How is it different from nd->depth > 0 && nd->flags & LOOKUP_RCU? > IOW, could we bump nd->depth before that (conditional) mntget()? OK, I see... So you are holding that flag for as long as we are traversing any part of a symlink body, including that of a trailing symlink... -- 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/namei.c b/fs/namei.c index 8cb89a0d30ba..e0f889192f59 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -550,6 +550,9 @@ static int unlazy_walk(struct nameidata *nd, struct dentry *dentry) struct dentry *parent = nd->path.dentry; BUG_ON(!(nd->flags & LOOKUP_RCU)); + if (nd->flags & LOOKUP_LINK_RCU) + /* Cannot unlazy in the middle of following a symlink */ + return -ECHILD; /* * After legitimizing the bastards, terminate_walk() @@ -766,7 +769,8 @@ static inline void put_link(struct nameidata *nd, struct path *link, void *cooki struct inode *inode = link->dentry->d_inode; if (inode->i_op->put_link) inode->i_op->put_link(link->dentry, cookie); - path_put(link); + if (!(nd->flags & LOOKUP_LINK_RCU)) + path_put(link); } int sysctl_protected_symlinks __read_mostly = 0; @@ -892,9 +896,10 @@ follow_link(struct path *link, struct nameidata *nd, void **p) int error; char *s; - BUG_ON(nd->flags & LOOKUP_RCU); - - if (link->mnt == nd->path.mnt) + nd->flags &= ~LOOKUP_LINK_RCU; + if (nd->flags & LOOKUP_RCU) + nd->flags |= LOOKUP_LINK_RCU; + else if (link->mnt == nd->path.mnt) mntget(link->mnt); error = -ELOOP; @@ -944,7 +949,8 @@ follow_link(struct path *link, struct nameidata *nd, void **p) out_put_nd_path: *p = NULL; terminate_walk(nd); - path_put(link); + if (!(nd->flags & LOOKUP_LINK_RCU)) + path_put(link); return error; } @@ -1667,6 +1673,8 @@ static inline int nested_symlink(struct path *path, struct nameidata *nd) current->nameidata->link_count--; nd->depth--; + if (!nd->depth) + nd->flags &= ~LOOKUP_LINK_RCU; return res; } diff --git a/include/linux/namei.h b/include/linux/namei.h index 368eb3d721b8..05b6b9c18801 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -31,6 +31,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND}; #define LOOKUP_PARENT 0x0010 #define LOOKUP_REVAL 0x0020 #define LOOKUP_RCU 0x0040 +#define LOOKUP_LINK_RCU 0x0080 /* * Intent data
When we support ->follow_link in RCU-walk we will not want to take a reference to the 'struct path *link' passed to follow_link, and correspondingly will not want to drop that reference. As link_path_walk will complete_walk() in the case of an error, and as complete_walk() will clear LOOKUP_RCU, we cannot test LOOKUP_RCU to determine if the path should be 'put'. So introduce a new flag: LOOKUP_LINK_RCU. This is set on entry to follow_link() if appropriate and put_link() will only call path_put() if it is clear. Also, unlazy_walk() will fail if LOOKUP_LINK_RCU is set. This is because there is no way for unlazy_walk to get references on all the "struct path *link"s that are protected by that flag. Signed-off-by: NeilBrown <neilb@suse.de> --- fs/namei.c | 18 +++++++++++++----- include/linux/namei.h | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) -- 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