linux-kernel - Re: [git pull] apparmor fix for __d

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111207034238.GX2203@ZenIV.linux.org.uk>
Date:	Wed, 7 Dec 2011 03:42:38 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:	john.johansen@...onical.com, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, torvalds@...ux-foundation.org
Subject: Re: [git pull] apparmor fix for __d_path() misuse

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/