Re: [PATCH v1] cgroup,bpf: Add access check for cgroup_get_from_fd()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 20, 2016 at 12:49:13AM +0200, Mickaël Salaün wrote:
> Add security access check for cgroup backed FD. The "cgroup.procs" file
> of the corresponding cgroup should be readable to identify the cgroup,
> and writable to prove that the current process can manage this cgroup
> (e.g. through delegation). This is similar to the check done by
> cgroup_procs_write_permission().
> 
> Fixes: 4ed8ec521ed5 ("cgroup: bpf: Add BPF_MAP_TYPE_CGROUP_ARRAY")

I don't understand what 'fixes' is about.
Looks like new feature or tightening?
Since cgroup was opened by the process and it got an fd,
it had an access, so extra check here looks unnecessary.

> -struct cgroup *cgroup_get_from_fd(int fd)
> +struct cgroup *cgroup_get_from_fd(int fd, int access_mask)
>  {
>  	struct cgroup_subsys_state *css;
>  	struct cgroup *cgrp;
>  	struct file *f;
> +	struct inode *inode;
> +	int ret;
>  
>  	f = fget_raw(fd);
>  	if (!f)
>  		return ERR_PTR(-EBADF);
>  
>  	css = css_tryget_online_from_dir(f->f_path.dentry, NULL);
> -	fput(f);

why move it down?

> -	if (IS_ERR(css))
> -		return ERR_CAST(css);
> +	if (IS_ERR(css)) {
> +		ret = PTR_ERR(css);
> +		goto put_f;
> +	}
>  
>  	cgrp = css->cgroup;
>  	if (!cgroup_on_dfl(cgrp)) {
> -		cgroup_put(cgrp);
> -		return ERR_PTR(-EBADF);
> +		ret = -EBADF;
> +		goto put_cgrp;
> +	}
> +
> +	ret = -ENOMEM;
> +	inode = kernfs_get_inode(f->f_path.dentry->d_sb, cgrp->procs_file.kn);
> +	if (inode) {
> +		ret = inode_permission(inode, access_mask);
> +		iput(inode);
>  	}
> +	if (ret)
> +		goto put_cgrp;
>  
> +	fput(f);
>  	return cgrp;
> +
> +put_cgrp:
> +	cgroup_put(cgrp);
> +put_f:
> +	fput(f);
> +	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(cgroup_get_from_fd);
>  
> -- 
> 2.9.3
> 
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux