[OpenAFS-devel] Re: mountpoints on linux

[Andrew Deason]

On Mon, 2 Jul 2012 18:59:34 -0500
Andrew Deason <adeason@sinenomine.net> wrote:

> I have a guess at this modification to solution "E" in gerrit 7657

For anybody that may be trying to follow this, but isn't following
gerrit closely enough, the most recent attempt is in 7741/7742. 7657 &co
have a few issues I'm not sure how to sort out.

> First of all, we have this usage in afs_mnt_follow_link:
> 
> 	static const struct qstr anonstring = { .name = "" };
> 	newdp = d_alloc(dentry->d_parent, &anonstring);
> 
> I'm not really sure if that's valid, giving a new dentry an empty name
> like that when it's not an "anonymous"/disconnected dentry. But nothing
> immediately explodes when I do it.

This breaks 'pwd'. And it probably breaks other stuff, too; I get the
feeling this is way outside the realm of expected behavior.

> Doing something of this sort seems necessary, though, unless we symlink
> to /afs/.:mount/. Normally I am trying to 'symlink' the dir to the
> existing alias dentry for the mountpoint target, so we don't need to
> construct a new dentry. When there is no alias, we could in theory just
> bypass the whole mntpt-symlink thing, and just return a regular dir like
> we always have. However, if we rely on an alias dentry existing in
> afs_mnt_follow_link, I think that creates a race. Consider:

I'm still not sure how to address this. So, in 7741 I started to take
the approach of just looking up the dir/mtpt like normal, and assigning
a follow_link function that only does something if we have multiple
dentry aliases. So, we can suddenly sort of "turn off" the redirection
thing when we're the only dentry by making follow_link do effectively
nothing. (The details get a little more complex, but that's the basic
idea.)

If we had separate inodes for this "redirection" step, it would get rid
of some other problems. But embedding the follow_link in the dir inode
itself seems to get rid of the issue in the previous paragraph, so I
don't see a way around doing that.

> Along a similar line of thought, it seems like we can combine this
> approach with the existing approach of solution "B". That is, we try
> the current approach (reparent the mtpt dentry when possible), and if
> it's not possible to reparent (the dentry alias cannot be
> invalidated), only then do we try to create a mtpt with the
> follow_link function to point at the other mtpt.

This is what 7741 does.

> However, I am a bit concerned about this kind of behavior for 2 reasons:
> 
>   -- The fact that we "almost always" do the current reparenting
>   behavior means that if there is any mistake in the symlink/mtpt
>   stuff, it will be difficult to see, reproduce, etc.

This doesn't seem so bad anymore, considering there are easy ways to
induce the "cannot invalidate" behavior, so it's pretty easy to get any
situation into the case where we start follow_link-redirecting stuff.

>   -- The fact that mtpt access patterns dictates the definition of ..
>   seems troubling, both for causing heisenbugs and from possibly a
>   security standpoint. If you have something with a symlink involving ..
>   (or something else that uses the 'real' ..; I think that does?), you
>   could have a malicious user force .. to point at some AFS space they
>   control or something. That is:

This is still a problem, but this seems unavoidable unless we do
solution "A".

-- 
Andrew Deason
adeason@sinenomine.net