Re: [git pull] apparmor fix for __d_path() misuse

[Al Viro <viro@ZenIV.linux.org.uk>]

On Wed, Dec 07, 2011 at 12:26:25PM +0900, Tetsuo Handa wrote:
> >         char *pos = ERR_PTR(-ENOMEM);
> >         if (buflen >= 256) {
> > -               struct path ns_root = { };
> >                 /* go to whatever namespace root we are under */
> > -               pos = __d_path(path, &ns_root, buffer, buflen - 1);
> > +               pos = d_absolute_path(path, buffer, buflen - 1);
> >                 if (!IS_ERR(pos) && *pos == '/' && pos[1]) {
> >                         struct inode *inode = path->dentry->d_inode;
> >                         if (inode && S_ISDIR(inode->i_mode)) {
> 
> Currently, TOMOYO assumes that -ENAMETOOLONG is the only error which __d_path()
> might return (and retries with larger buffer size unless kmalloc() fails).
> If d_absolute_path() starts returning -EINVAL, TOMOYO will deny requests even
> if "partial (I mean the result would have been different if reachable)"
> pathname is granted by the policy.
> 
> How commonly can conditions that make d_absolute_path() return -EINVAL happen?

Race with umount -l, basically.  In that case the pathname is completely
unreliable - if I do umount -l /mnt, pathnames that would be under mnt
may get truncated on *ANY* mountpoint.  Not "always cut on /mnt"; not "always
cut on the last mountpoint"; it's "everything from root to arbitrary mountpoint
on that path is not noticed".  If your policy really has to deal with such
situations (unexpected umount -l racing with operations in the subtree that
gets dissolved), you do have a problem.  Right now, in the mainline kernel.
Because the pathname you are currently getting is unreliable as hell and
making any decisions basing on it is, er, not particulary wise.

If you want to add handling of that -EINVAL in some form, feel free.  But
keep in mind that the *ONLY* part of pathname you can really recover at
that point is what dentry_path() would give you - i.e. from fs root to
object in question.  Everything prior to that is absolutely unreliable
and is bound to go away in a very short while - ->mnt_parent on the entire
chain is in process of being dissolved when we hit that race.