Re: [REVIEW][PATCH 1/3] vfs: In d_path don't call d_dname on a mount point

[Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>]

On Sat, Nov 30, 2013 at 06:15:26AM +0000, Al Viro wrote:

> There's a bunch of unpleasant details around the handling of "what if
> the final vfsmount is detached or internal" (and induced weirdness
> with d_absolute_path(), etc. callers deciding whether they want to
> call ->d_dname() manually, since only d_path() calls it); I'll look into
> that.

FWIW, the other callers of prepend_path() boil down to /proc/mountinfo
handling, apparmour d_namespace_path() (separate handling of MNT_INTERNAL,
__d_path() or d_absolute_path() for the rest) and tomoyo_get_absolute_path()
(this one directly calls ->d_dname() itself).  Note that /proc/mountinfo
will spew garbage for your case (binding /proc/<pid>/ns/mnt somewhere);
the mountpoint will show correctly, but the relative name won't - it
goes through seq_dentry()->dentry_path() (this and apparmour d_namespace_path()
being the only codepaths to dentry_path(), BTW).  Eric, what behaviour
do you want there?

While we are at it: lustre contains this gem:
static char* ll_d_path(struct dentry *dentry, char *buf, int bufsize)
{
        char *path = NULL;

        struct path p;

        p.dentry = dentry;
        p.mnt = current->fs->root.mnt;
        path_get(&p);
        path = d_path(&p, buf, bufsize);
        path_put(&p);

        return path;
}
That should've been dentry_path(), reimplemented here with the usual braino.
Think what this hack produces if current is chrooted into fs in question;
the "clever" trick fails and instead of intended path from fs root we get
path from wherever current's chrooted to.  There's a reason why dentry_path()
had been introduced...

<looks at the callers of that wonder>
        /* this can be called inside spin lock so use GFP_ATOMIC. */
        buf = (char *)__get_free_page(GFP_ATOMIC);
        if (buf != NULL) {
                dentry = d_find_alias(page->mapping->host);
                if (dentry != NULL)
                        path = ll_d_path(dentry, buf, PAGE_SIZE);
        }
	...
	if (dentry)
		dput(dentry);

Good luck if it ever gets called under spinlock - dput() is not a thing you
should call in such place, ditto for path_put() from ll_d_path() itself...
What are the callchains leading there?  I've tried to track them, but gave
up after a while ;-/  I really hope that it's just "called under spinlock"
and not "called from softirq" or something like that...  Who maintains
that thing, anyway?

BTW, what happens if svc_export_request() ends up with pathname filling
almost all space left, so that qword_add(bpp, blen, pth) right after the
call of d_path() in there overwrites the beginning of d_path() output?
Neil?  And while we are at it, handling of overflow in there looks also
looks fishy...