[REVIEW][PATCH 1/3] vfs: In d_path don't call d_dname on a mount point
Al Viro
viro at ZenIV.linux.org.uk
Sat Nov 30 17:02:26 UTC 2013
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...
More information about the Containers
mailing list