On 2019-07-12, Al Viro viro@zeniv.linux.org.uk wrote:
On Sun, Jul 07, 2019 at 12:57:28AM +1000, Aleksa Sarai wrote:
@@ -514,7 +516,14 @@ static void set_nameidata(struct nameidata *p, int dfd, struct filename *name) p->stack = p->internal; p->dfd = dfd; p->name = name;
- p->total_link_count = old ? old->total_link_count : 0;
- p->total_link_count = 0;
- p->acc_mode = 0;
- p->opath_mask = FMODE_PATH_READ | FMODE_PATH_WRITE;
- if (old) {
p->total_link_count = old->total_link_count;p->acc_mode = old->acc_mode;p->opath_mask = old->opath_mask;- }
Huh? Could somebody explain why traversals of NFS4 referrals should inherit ->acc_mode and ->opath_mask?
I'll be honest -- I don't understand what set_nameidata() did so I just did what I thought would be an obvious change (to just copy the contents). I thought it was related to some aspect of the symlink stack handling.
In that case, should they both be set to 0 on set_nameidata()? This will mean that fd re-opening (or magic-link opening) through a set_nameidata() would always fail.
static __always_inline -const char *get_link(struct nameidata *nd) +const char *get_link(struct nameidata *nd, bool trailing) { struct saved *last = nd->stack + nd->depth - 1; struct dentry *dentry = last->link.dentry; @@ -1081,6 +1134,44 @@ const char *get_link(struct nameidata *nd) } else { res = get(dentry, inode, &last->done); }
/* If we just jumped it was because of a magic-link. */if (unlikely(nd->flags & LOOKUP_JUMPED)) {[...] In any case, this "bool trailing" is completely wrong; whether that check belongs in trailing_symlink() or (some of) its callers, putting it into get_link() is a mistake, forced by kludgy check for procfs-style symlinks.
The error path for LOOKUP_JUMPED comes from the old O_BENEATH patchset, but all of the "bool trailing" logic is definitely my gaff (I was quietly hoping you'd have a much better solution than the whole get_link() thing -- it definitely felt very kludgey to write).
I will work on the suggestion in your follow-up email. Thanks!