Hi Eric,
On Sun, May 30, 2010 at 11:54:23PM -0400, Eric Paris wrote:
> We need to call this function in the SELinux case. So you'll need a
> patch like the one attached (not even compiled but I think it is right)
> [..]
> static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *nameidata)
> [..]
> + rc = cap_inode_follow_link(dentry, nameidata);
Yeah, when I quickly checked SELinux and AppArmor, it seemed that they
were always calling down to all commoncaps functions, but it looks like
not in all cases. I think that Eric Biederman's observations here makes
the most sense: this check needs to happen without involving the LSMs
at all.
> > +int cap_inode_follow_link(struct dentry *dentry,
> > + struct nameidata *nameidata)
> > +{
> > + const struct inode *parent = dentry->d_parent->d_inode;
> > + const struct inode *inode = dentry->d_inode;
> > + const struct cred *cred = current_cred();
> > +
> > + if (weak_sticky_symlinks)
> > + return 0;
> > +
> > + if ((parent->i_mode & (S_ISVTX|S_IWOTH)) == (S_ISVTX|S_IWOTH) &&
> > + parent->i_uid != inode->i_uid &&
> > + cred->fsuid != inode->i_uid) {
> > + printk_ratelimited(KERN_NOTICE "non-matching-uid symlink "
> > + "following attempted in sticky-directory by "
> > + "%s (fsuid %d)\n", current->comm, cred->fsuid);
> > + return -EACCES;
> > + }
> > + return 0;
> > +}
>
> What stops us from racing between the assignment of parent and it's
> first use with a rename on our object and rmdir on the old parent? I'm
> wondering if we need to be doing this test holding dentry->d_lock (which
> is what protects dentry->d_parent if I recall correctly)
>
> Certainly doesn't fix all of the raciness, but I think it would close
> the opps part. Maybe someone who knows the VFS better can tell me if I
> am misguided.
The only other use of d_parent I can see there is in may_delete(). With
vfs_unlink() calling that, it would seem to be racey too if we needed to
hold a lock for that. But it's not clear to me in vfs_follow_link is doing
locking somehow.
Thanks,
-Kees
--
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
