On 2019-07-12, Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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! -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH <https://www.cyphar.com/>
Attachment:
signature.asc
Description: PGP signature
