Re: [PATCH v4 6/8] selinux: Restrict cross-cgroup dma-buf charging

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selinux: one-off dma_buf class instead of general cgroup access control model
--

commit 675e985fff50ea6716b8c00023a75c07ff345a6f
Author: Albert Esteve <aesteve@redhat.com>

selinux: Restrict cross-cgroup dma-buf charging

This commit implements the selinux_dma_buf_charge_task() hook and adds a
new dma_buf object class with a charge_task permission in SELinux to
restrict cross-cgroup dma-buf memory charging.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 30776b0107f29..8f39d91eabc97 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2204,6 +2204,12 @@ static int selinux_capable(const struct cred *cred, struct user_namespace *ns,
>  	return cred_has_capability(cred, cap, opts, ns == &init_user_ns);
>  }
>  
> +static int selinux_dma_buf_charge_task(const struct cred *from, const struct cred *to)
> +{
> +	return avc_has_perm(cred_sid(from), cred_sid(to),
> +			    SECCLASS_DMA_BUF, DMA_BUF__CHARGE_TASK, NULL);
> +}
> +
>  static int selinux_quotactl(int cmds, int type, int id, const struct super_block *sb)
>  {
>  	const struct cred *cred = current_cred();

[ ... ]

> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 90cb61b164256..b4ebedc6e4240 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -181,6 +181,7 @@ const struct security_class_mapping secclass_map[] = {
>  	{ "user_namespace", { "create", NULL } },
>  	{ "memfd_file",
>  	  { COMMON_FILE_PERMS, "execute_no_trans", "entrypoint", NULL } },
> +	{ "dma_buf", { "charge_task", NULL } },

[Severity: Medium]
As reported by Paul Moore on RFC [1], adding a one-off SELinux object class
and permission for a specific subsystem's cgroup charging (previously
dma_heap with charge_to, now dma_buf with charge_task in secclass_map[]
and selinux_dma_buf_charge_task(), along with charge_cgroup in a later
patch in this series) introduces a narrow, subsystem-specific policy class
rather than a general cgroup and resource access control model for SELinux.

Although resolving the target memcg from charge_fd moved into generic
memcontrol code in mem_cgroup_get_from_fd(), the SELinux hook in
selinux_dma_buf_charge_task() and class definition in secclass_map[] still
define a dma-buf-specific class instead of a broader cgroup access control
abstraction.

Should this permission check be designed around a general cgroup or resource
SELinux object class rather than a one-off dma_buf class?

[1] https://lore.kernel.org/r/16093a0278a6d7d1a0a8bc055c228bed@paul-moore.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-v2_20230123_tjmercier_google_com-v4-0-1819a017000b@redhat.com?part=6