Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] selinux: selinux_dma_buf_charge_cgroup() bypasses check on genfscon cgroups --
commit 6b412783b17727c382f1a67078326c58c39264e7 Author: Albert Esteve aesteve@redhat.com
selinux: Restrict dma-buf direct cgroup charging
This commit implements the selinux_dma_buf_charge_cgroup() LSM hook and adds the charge_cgroup permission to the dma_buf security class to control which processes may charge dma-buf memory to a cgroup. It reads the target security context from the cgroup kernfs node's security.selinux extended attribute and checks permission via avc_has_perm().
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 8f39d91eabc97..2f99eaa910af4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2210,6 +2210,40 @@ static int selinux_dma_buf_charge_task(const struct cred *from, const struct cre SECCLASS_DMA_BUF, DMA_BUF__CHARGE_TASK, NULL); } +#ifdef CONFIG_CGROUPS +static int selinux_dma_buf_charge_cgroup(const struct cred *cred, struct cgroup *cgrp) +{
- u32 cgrp_sid, clen;
- int rc;
- char *context;
- rc = kernfs_xattr_get(cgrp->kn, XATTR_NAME_SELINUX, NULL, 0);
- if (rc == -ENODATA)
return 0;
[Severity: High] Does returning 0 on -ENODATA here bypass the permission check for all cgroups that carry their default genfscon label (such as cgroup_t)?
In selinux_set_mnt_opts(), cgroup and cgroup2 filesystems are configured with SE_SBGENFS | SE_SBGENFS_XATTR. When a cgroup VFS inode is initialized in inode_doinit_with_dentry(), SELinux obtains the default SID from policy via selinux_genfs_get_sid() and then checks for an optional xattr override via inode_doinit_use_xattr():
security/selinux/hooks.c:inode_doinit_with_dentry() { ... rc = selinux_genfs_get_sid(dentry, sclass, sbsec->flags, &sid); if (rc) { dput(dentry); goto out; }
if ((sbsec->flags & SE_SBGENFS_XATTR) && (inode->i_opflags & IOP_XATTR)) { rc = inode_doinit_use_xattr(inode, dentry, sid, &sid); ... }
When no explicit setxattr() has been performed on the cgroup directory, inode_doinit_use_xattr() sees -ENODATA from the underlying kernfs node and falls back to def_sid in memory without writing a security.selinux xattr to cgrp->kn:
security/selinux/hooks.c:inode_doinit_use_xattr() { ... if (rc < 0) { kfree(context); if (rc != -ENODATA) { pr_warn("SELinux: %s: getxattr returned %d for dev=%s ino=%llu\n", __func__, -rc, inode->i_sb->s_id, inode->i_ino); return rc; } *sid = def_sid; return 0; } ... }
Likewise, selinux_kernfs_init_security() sees -ENODATA on the parent kernfs node and returns 0 without storing an xattr on newly created child cgroups.
As a result, unless a cgroup directory has been explicitly relabeled with setxattr(), kernfs_xattr_get(cgrp->kn, XATTR_NAME_SELINUX, NULL, 0) will return -ENODATA even though the cgroup is labeled cgroup_t via genfscon. When userspace passes a cgroup fd via DMA_HEAP_IOCTL_ALLOC or DMA_BUF_IOCTL_XFER_CHARGE, mem_cgroup_get_from_fd() invokes security_dma_buf_charge_cgroup() -> selinux_dma_buf_charge_cgroup(), which will return 0 without calling avc_has_perm(), leaving default-labeled cgroup_t cgroups unrestricted?