lkml.org 
[lkml]   [2017]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] xattr: Enable security.capability in user namespaces
On Tue, Jul 11, 2017 at 11:05:11AM -0400, Stefan Berger wrote:

[..]
> @@ -301,14 +721,39 @@ ssize_t
> __vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name,
> void *value, size_t size)
> {
> - const struct xattr_handler *handler;
> + const struct xattr_handler *handler = NULL;
> + char *newname = NULL;
> + int ret, userns_supt_xattr;
> + struct user_namespace *userns = current_user_ns();
> +
> + userns_supt_xattr = (xattr_is_userns_supported(name, false) >= 0);
> +

Hi Stephan,

> + do {
> + kfree(newname);
> +
> + newname = xattr_userns_name(name, userns);
^^^
Will name be pointing to a freed string in second iteration of loop.

> + if (IS_ERR(newname))
> + return PTR_ERR(newname);
> +
> + if (!handler) {
> + name = newname;

Here we assign name and at the beginning of second iteration we free
newname.

Also I am not sure why do we do this assignment only if handler is NULL.

BTW, I set cap_sys_admin on a file outside usernamespace and then launched
user namespace (mapping 1000 to 0). And then tried to do getcap on file
and I am not seeing security.capability set by host. Not sure what am I
doing wrong. getxattr() seems to return -ENODATA. Still debugging it.

Also, have we resovled the question of stacked filesystem like overlayfs.
There we are switching creds to mounter's creds when doing operations on
underlying filesystem. I am concenrned does that mean, we will get and
return security.capability to caller in usernamespace instead of
security.capability@uid=1000.

Vivek

> + handler = xattr_resolve_name(inode, &name);
> + if (IS_ERR(handler)) {
> + ret = PTR_ERR(handler);
> + goto out;
> + }
> + if (!handler->get) {
> + ret = -EOPNOTSUPP;
> + goto out;
> + }
> + }
> + ret = handler->get(handler, dentry, inode, name, value, size);
> + userns = userns->parent;
> + } while ((ret == -ENODATA) && userns && userns_supt_xattr);
>
> - handler = xattr_resolve_name(inode, &name);
> - if (IS_ERR(handler))
> - return PTR_ERR(handler);
> - if (!handler->get)
> - return -EOPNOTSUPP;
> - return handler->get(handler, dentry, inode, name, value, size);
> +out:
> + kfree(newname);
> + return ret;
> }
> EXPORT_SYMBOL(__vfs_getxattr);
>

Thanks
Vivek

\
 
 \ /
  Last update: 2017-07-12 19:54    [W:0.266 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site