Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] dma-heap: missing permission check in mem_cgroup_get_from_fd() - [High] dma-buf: system_heap: 32-bit nr_pages truncation in system_heap_allocate() - [Medium] mm: memcontrol: missing EXPORT_SYMBOL_GPL() on dmabuf memcg helpers - [Medium] mm: memcontrol: mem_cgroup_get_from_fd() returns NULL when memcg is disabled --

commit 961ee31303129349ee7fa6988bef4bc244f22a61 Author: Albert Esteve aesteve@redhat.com

dma-heap: charge dma-buf memory via explicit memcg

This commit adds a charge_fd field to struct dma_heap_allocation_data so DMA_HEAP_IOCTL_ALLOC can resolve a target cgroup from a pidfd or cgroupfd via mem_cgroup_get_from_fd() and charge system heap allocations to that memcg.

[ ... ]

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index c8959eadc71d1..adeea8cd328b8 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -17,6 +17,7 @@ #include <linux/err.h> #include <linux/highmem.h> #include <linux/mem_encrypt.h> +#include <linux/memcontrol.h> #include <linux/mm.h> #include <linux/set_memory.h> #include <linux/module.h> @@ -388,8 +389,6 @@ static struct page *alloc_largest_available(unsigned long size, if (max_order < orders[i]) continue; flags = order_flags[i];

• if (mem_accounting)
  

• 	flags |= __GFP_ACCOUNT;
  

  page = alloc_pages(flags, orders[i]); if (!page) continue;

@@ -401,11 +400,13 @@ static struct page *alloc_largest_available(unsigned long size, static struct dma_buf *system_heap_allocate(struct dma_heap *heap, unsigned long len, u32 fd_flags,

• 			    u64 heap_flags)
  

• 			    u64 heap_flags,
  

• 			    struct mem_cgroup *memcg)
  

{ struct system_heap_buffer *buffer; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); unsigned long size_remaining = len;

• unsigned int nr_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;

[Severity: High] Can nr_pages truncate on 64-bit systems when a caller passes a large heap_allocation->len via DMA_HEAP_IOCTL_ALLOC?

In dma_heap_ioctl_allocate(), heap_allocation->len is page-aligned into a size_t len and only checked for zero before being passed to system_heap_allocate() as an unsigned long:

drivers/dma-buf/dma-heap.c:dma_heap_ioctl_allocate() { ... len = PAGE_ALIGN(heap_allocation->len); if (!len) { mem_cgroup_put(memcg); return ERR_PTR(-EINVAL); }

dmabuf = heap->ops->allocate(heap, len, heap_allocation->fd_flags, heap_allocation->heap_flags, memcg); }

In system_heap_allocate(), size_remaining keeps the full 64-bit len, while nr_pages is stored in a 32-bit unsigned int. Because __GFP_ACCOUNT is removed from alloc_largest_available(), cgroup enforcement now relies solely on mem_cgroup_reserve_dmabuf(memcg, GFP_KERNEL, nr_pages).

If userspace passes len >= (1ULL << (32 + PAGE_SHIFT)) (for example, 16 TiB with 4 KiB pages), nr_pages wraps to 0 (or a small value), so mem_cgroup_reserve_dmabuf() succeeds without charging the cgroup while the while (size_remaining > 0) loop continues allocating uncharged physical pages until global memory exhaustion. In addition, values of nr_pages in [0x80000000U, 0xffffffffU] are passed by mem_cgroup_commit_dmabuf() to mod_memcg_state(), which takes a signed int val and would corrupt MEMCG_DMABUF statistics.

unsigned int max_order = orders[0]; struct system_heap_priv *priv = dma_heap_get_drvdata(heap); bool cc_shared = priv->cc_shared; @@ -416,10 +417,15 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap, struct page *page, *tmp_page; int i, ret = -ENOMEM;

• buffer = kzalloc_obj(*buffer);
• if (!buffer)

• if (!mem_cgroup_reserve_dmabuf(memcg, GFP_KERNEL, nr_pages)) return ERR_PTR(-ENOMEM);

• buffer = kzalloc_obj(*buffer);
• if (!buffer) {

• ret = -ENOMEM;
  

• goto cancel_charge;
  

• }
• INIT_LIST_HEAD(&buffer->attachments); mutex_init(&buffer->lock); buffer->heap = heap;

@@ -478,6 +484,9 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap, ret = PTR_ERR(dmabuf); goto free_pages; }

• mem_cgroup_commit_dmabuf(dmabuf, memcg, nr_pages);
• return dmabuf;

free_pages: @@ -498,6 +507,8 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap, list_for_each_entry_safe(page, tmp_page, &pages, lru) __free_pages(page, compound_order(page)); kfree(buffer); +cancel_charge:

• mem_cgroup_cancel_dmabuf(memcg, nr_pages);

[Severity: Medium] Will building with CONFIG_DMABUF_HEAPS_SYSTEM=m and CONFIG_MEMCG=y fail at modpost with undefined symbol errors for mem_cgroup_reserve_dmabuf(), mem_cgroup_commit_dmabuf(), and mem_cgroup_cancel_dmabuf()?

In drivers/dma-buf/heaps/Kconfig, DMABUF_HEAPS_SYSTEM is tristate, so drivers/dma-buf/heaps/system_heap.c can be compiled as a loadable module (system_heap.ko). However, mem_cgroup_reserve_dmabuf(), mem_cgroup_commit_dmabuf(), and mem_cgroup_cancel_dmabuf() in mm/memcontrol.c do not have EXPORT_SYMBOL_GPL() annotations.

Should those symbols be exported (or should the reservation/commit/cancel calls live in the built-in drivers/dma-buf/dma-heap.c layer as described in the commit message)?

[ ... ]

diff --git a/mm/memcontrol.c b/mm/memcontrol.c index bbd46ba40350b..f66254f52b999 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5661,6 +5661,53 @@ void mem_cgroup_flush_workqueue(void) flush_workqueue(memcg_wq); } +/**

• • mem_cgroup_get_from_fd - resolve a memcg from a cgroupfd or pidfd
• • @fd: file descriptor (cgroupfd or pidfd)
• • Tries @fd as a cgroupfd first; if that fails, tries it as a pidfd.
• • On success returns a referenced memcg pointer. On failure returns
• • an ERR_PTR.
• */

+struct mem_cgroup *mem_cgroup_get_from_fd(int fd) +{

• struct cgroup_subsys_state *css;
• struct mem_cgroup *memcg;
• struct cgroup *cgrp;
• struct task_struct *task;
• struct mm_struct *mm;
• unsigned int pidfd_flags;
• cgrp = cgroup_get_from_fd(fd);
• if (!IS_ERR(cgrp)) {

• rcu_read_lock();
  

• css = cgroup_css(cgrp, &memory_cgrp_subsys);
  

• if (!css || !css_tryget_online(css)) {
  

• 	rcu_read_unlock();
  

• 	cgroup_put(cgrp);
  

• 	return ERR_PTR(-ENOENT);
  

• }
  

• memcg = mem_cgroup_from_css(css);
  

• rcu_read_unlock();
  

• cgroup_put(cgrp);
  

• return memcg;
  

• }
• task = pidfd_get_task(fd, &pidfd_flags);
• if (IS_ERR(task))

• return ERR_CAST(task);
  

• mm = get_task_mm(task);
• put_task_struct(task);
• if (!mm)

• return ERR_PTR(-ESRCH);
  

• memcg = get_mem_cgroup_from_mm(mm);
• mmput(mm);
• return memcg;

+}

[Severity: Critical] What prevents an unprivileged user with access to a dma-heap device from passing a cgroupfd (opened with O_RDONLY | O_DIRECTORY or O_PATH on any world-readable /sys/fs/cgroup directory) or a pidfd (via pidfd_open() on PID 1 or another user's process) as charge_fd to DMA_HEAP_IOCTL_ALLOC?

Through dma_heap_ioctl_allocate() -> mem_cgroup_get_from_fd() and system_heap_allocate() -> mem_cgroup_reserve_dmabuf() -> try_charge_memcg(), charging to the root cgroup (where mem_cgroup_is_root(memcg) skips page counter charging) allows a restricted process to bypass its own memory.max and memory.high limits, while charging to another cgroup allows exhausting that victim cgroup's quota and triggering mem_cgroup_oom() kills inside it.

Later patches in the series ("security: dma-buf: Add dma_buf_charge_task LSM hook", "security: dma-buf: Add dma_buf_charge_cgroup LSM hook", "selinux: Restrict cross-cgroup dma-buf charging", and "selinux: Restrict dma-buf direct cgroup charging") add LSM hooks and SELinux checks, but there are no baseline DAC, capability, ptrace_may_access(), or cgroup write permission checks when CONFIG_SECURITY is disabled, when SELinux is not active, or when a cgroup directory has no explicit security.selinux xattr (where selinux_dma_buf_charge_cgroup() returns 0 on -ENODATA).

Also, when charging a remote memcg via try_charge_memcg() breaches that target cgroup's memory.high limit, try_charge_memcg() increments current->memcg_nr_pages_over_high and calls __mem_cgroup_handle_over_high(), which resolves get_mem_cgroup_from_mm(current->mm) and reclaims from the allocating process's own cgroup rather than the remotely charged memcg. Should remote charging require appropriate permissions over the target task or cgroup, and how should memory.high reclaim be handled when charging a foreign memcg?

[Severity: Medium] In mem_cgroup_get_from_fd(), what happens on the pidfd path when CONFIG_MEMCG=y and memory cgroups are disabled at boot via cgroup_disable=memory?

When mem_cgroup_disabled() is true, get_mem_cgroup_from_mm(mm) returns NULL, so mem_cgroup_get_from_fd() returns NULL instead of an ERR_PTR as documented in its kernel-doc comment.

In dma_heap_ioctl_allocate() (and in dma_buf_ioctl_xfer_charge() later in the series), IS_ERR(memcg) is false for NULL, so a pidfd charge_fd silently falls through while a cgroupfd charge_fd fails with -ENOENT (because cgroup_css() returns NULL when the subsystem is disabled). Should the pidfd path also return an ERR_PTR when get_mem_cgroup_from_mm(mm) returns NULL?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] memcg: task cred and mm->owner mismatch in mem_cgroup_get_from_fd() --

commit 6c6031732d7f42644e3d99679d750fcc12907447 Author: Albert Esteve aesteve@redhat.com

security: dma-buf: Add dma_buf_charge_task LSM hook

This commit adds the security_dma_buf_charge_task() LSM hook and calls it in mem_cgroup_get_from_fd() when resolving a pidfd, allowing security modules to mediate cross-process dma-buf cgroup charging.

diff --git a/mm/memcontrol.c b/mm/memcontrol.c index f66254f52b999..ad195cfd1b5f6 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c

[ ... ]

@@ -5698,6 +5701,14 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd) if (IS_ERR(task)) return ERR_CAST(task);

• tcred = get_task_cred(task);
• ret = security_dma_buf_charge_task(current_cred(), tcred);
• put_cred(tcred);
• if (ret) {

• put_task_struct(task);
  

• return ERR_PTR(ret);
  

• }
• mm = get_task_mm(task); put_task_struct(task); if (!mm)

[Severity: High] Can the cgroup returned by mem_cgroup_get_from_fd() diverge from the task credentials checked by security_dma_buf_charge_task()?

Here security_dma_buf_charge_task() validates get_task_cred(task), and then mem_cgroup_get_from_fd() resolves the target mem_cgroup via get_task_mm(task) and get_mem_cgroup_from_mm(mm):

mm/memcontrol.c:mem_cgroup_get_from_fd() { ... mm = get_task_mm(task); put_task_struct(task); if (!mm) return ERR_PTR(-ESRCH);

memcg = get_mem_cgroup_from_mm(mm); mmput(mm); return memcg; }

However, get_mem_cgroup_from_mm() resolves the mem_cgroup from mm->owner rather than from task, and falls back to root_mem_cgroup when mm->owner is NULL:

mm/memcontrol.c:get_mem_cgroup_from_mm() { ... rcu_read_lock(); do { memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); if (unlikely(!memcg)) memcg = root_mem_cgroup; } while (!css_tryget(&memcg->css)); rcu_read_unlock(); return memcg; }

If task exits after get_task_mm(task) increments mm->mm_users but before get_mem_cgroup_from_mm(mm) runs, exit_mm() -> mm_update_next_owner() finds no live owner with mm_users > 1 and clears mm->owner:

kernel/exit.c:mm_update_next_owner() { ... /* * We found no owner yet mm_users > 1: this implies that we are * most likely racing with swapoff (try_to_unuse()) or /proc or * ptrace or page migration (get_task_mm()). Mark owner as NULL. */ WRITE_ONCE(mm->owner, NULL); ... }

In that case, get_mem_cgroup_from_mm(mm) returns root_mem_cgroup, where try_charge() skips charging via mem_cgroup_is_root(memcg). A confined caller in DMA_HEAP_IOCTL_ALLOC or DMA_BUF_IOCTL_XFER_CHARGE could fork a short-lived helper child with matching credentials to pass security_dma_buf_charge_task() and race its exit to charge allocations to root_mem_cgroup, escaping its own cgroup memory limit.

Similarly, if task shares an mm_struct with another task via clone(CLONE_VM) that has different credentials or cgroup membership, mm->owner can refer to that other task while security_dma_buf_charge_task() only checks task's credentials.

Would it make sense to resolve the task and its credentials from mm->owner (or look up the mem_cgroup directly from task) so that the LSM check and the mem_cgroup lookup operate on the same task and cannot fall back to root_mem_cgroup when mm->owner is cleared?

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?

On 10/27/26 16:02, Albert Esteve wrote:

dma-heap already supports charging a specific cgroup at allocation time via charge_fd in DMA_HEAP_IOCTL_ALLOC. However, dma-bufs can originate from any exporter (DRM, V4L2, etc.), and the allocator may not know the final owner at allocation time.

Add DMA_BUF_IOCTL_XFER_CHARGE to transfer a dma-buf's memcg charge to a different cgroup after allocation. The target is identified by charge_fd, which may be a cgroupfd or a pidfd.

The transfer charges the new cgroup before uncharging the old one, preserving the original charge on failure. An atomic cmpxchg on dmabuf->memcg prevents races with concurrent transfers.

Not all dma-buf exporters back their buffers with reclaimable system memory. Some represent device registers, VRAM, or carved-out pools where size is not a meaningful memory cost. Add a memcg_account flag to dma_buf_export_info that exporters set when their buffers should participate in memcg accounting. The xfer ioctl returns -EOPNOTSUPP for buffers that do not set the flag. Currently only the system heap sets it, other exporters can opt in as needed.

Usage example: int cg_fd = open("/sys/fs/cgroup/clients/app1", O_RDONLY | O_DIRECTORY);

struct dma_buf_transfer_charge xfer = { .charge_fd = cg_fd, }; ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer); /* charge now on app1's cgroup */ close(cg_fd);

Signed-off-by: Albert Esteve aesteve@redhat.com

drivers/dma-buf/dma-buf.c | 43 +++++++++++++++++++++++++++++++++++++ drivers/dma-buf/heaps/system_heap.c | 1 + include/linux/dma-buf.h | 5 +++++ include/linux/memcontrol.h | 9 ++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++ mm/memcontrol.c | 41 +++++++++++++++++++++++++++++++++++ 6 files changed, 125 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index f31f49ebd0af..07513ecedc10 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -540,6 +540,45 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf, } #endif +/* Transfer a dma-buf's memcg charge; see struct dma_buf_transfer_charge */ +static long dma_buf_xfer_charge(struct dma_buf *dmabuf,

• 		const void __user *user_data)
  

+{

• struct dma_buf_transfer_charge arg;
• struct mem_cgroup *memcg;
• int ret = 0;
• if (copy_from_user(&arg, user_data, sizeof(arg)))

• return -EFAULT;
  

• if (!IS_ENABLED(CONFIG_MEMCG))

• return 0;
  

• if (arg.flags != 0)

• return -EINVAL;
  

• if (!dmabuf->memcg_account)

• return -EOPNOTSUPP;
  

• memcg = mem_cgroup_get_from_fd(arg.charge_fd);
• if (IS_ERR(memcg))

• return PTR_ERR(memcg);
  

• if (dmabuf->memcg == memcg)

• goto skip_transfer;
  

• if (!mem_cgroup_transfer_dmabuf_charge(dmabuf, memcg)) {

• ret = -ENOMEM;
  

• goto skip_transfer;
  

• }
• return 0;

+skip_transfer:

• mem_cgroup_put(memcg);
• return ret;

+}

static long dma_buf_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -590,6 +629,9 @@ static long dma_buf_ioctl(struct file *file, return dma_buf_import_sync_file(dmabuf, (const void __user *)arg); #endif

• case DMA_BUF_IOCTL_XFER_CHARGE:

• return dma_buf_xfer_charge(dmabuf, (const void __user *)arg);
  

• default: return -ENOTTY; }

@@ -750,6 +792,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) dmabuf->size = exp_info->size; dmabuf->exp_name = exp_info->exp_name; dmabuf->owner = exp_info->owner;

• dmabuf->memcg_account = exp_info->memcg_account;

The initial charging should probably be done here as well.

This way DMA-buf heaps only needs to pass in the charge_fd and we can re-use the functionality for other DMA-buf exporters as well.

spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index adeea8cd328b..a764167167ba 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -479,6 +479,7 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap, exp_info.size = buffer->len; exp_info.flags = fd_flags; exp_info.priv = buffer;

• exp_info.memcg_account = true; dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf);

diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..ac0ec88cab93 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -433,6 +433,9 @@ struct dma_buf { /** @memcg: the cgroup to which this buffer is currently attributed */ struct mem_cgroup *memcg;

• /** @memcg_account: account buffer memory to cgroup */
• bool memcg_account;

}; /** @@ -505,6 +508,7 @@ struct dma_buf_attachment {

• @flags: mode flags for the file
• @resv: reservation-object, NULL to allocate default one
• @priv: Attach private data of allocator to this buffer

• • @memcg_account: account buffer memory to cgroup
  • This structure holds the information required to export the buffer. Used
  • with dma_buf_export() only.

@@ -517,6 +521,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv;

• bool memcg_account;

}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 6b30baa79fa9..36fddafbef4e 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -679,6 +679,9 @@ bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, void mem_cgroup_commit_dmabuf(struct dma_buf *dmabuf, struct mem_cgroup *memcg, unsigned int nr_pages); +bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 		       struct mem_cgroup *new_memcg);
  

void __mem_cgroup_uncharge(struct folio *folio); /** @@ -1206,6 +1209,12 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { } +static inline bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 				     struct mem_cgroup *new_memcg)
  

+{

• return true;

+}

static inline void mem_cgroup_uncharge(struct folio *folio) { } diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h index e827c9d20c5d..ea957f4b1d38 100644 --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -168,6 +168,31 @@ struct dma_buf_import_sync_file { __s32 fd; }; +/**

• • struct dma_buf_transfer_charge - Transfer a dma-buf's memcg charge
• • Any holder of a dma-buf file descriptor can perform a
• • DMA_BUF_IOCTL_XFER_CHARGE to attribute the buffer's memory
• • cost to a specific cgroup, or move it from one cgroup to
• • another. The ioctl charges the new cgroup before uncharging
• • the old one atomically. On failure the original charge is
• • preserved. If the buffer has no existing charge, a new
• • charge is established.
• */

+struct dma_buf_transfer_charge {

• /** @flags: Reserved, must be zero */
• __u32 flags; /* unused for now */
• /**

• * @charge_fd: File descriptor identifying the target cgroup.
  

• *
  

• * May be either a pidfd, in which case the charge moves to the
  

• * memory cgroup of the referenced process, or a cgroupfd obtained
  

• * by opening a cgroup v2 directory, in which case the charge moves
  

• * directly to that cgroup's memory controller.
  

• */
  

• __u32 charge_fd;

+};

#define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) @@ -179,5 +204,6 @@ struct dma_buf_import_sync_file { #define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, __u64) #define DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct dma_buf_export_sync_file) #define DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct dma_buf_import_sync_file) +#define DMA_BUF_IOCTL_XFER_CHARGE _IOW(DMA_BUF_BASE, 4, struct dma_buf_transfer_charge) #endif diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 7b841b905759..3adc84fe6668 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5786,6 +5786,47 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) refill_stock(memcg, nr_pages); } +/**

• • mem_cgroup_transfer_dmabuf_charge - move a dma-buf charge
• • @dmabuf: buffer to transfer charge
• • @new_memcg: target memcg; caller must hold a reference
• • Transfers the charge for @dmabuf to @new_memcg and, on
• • success, stores @new_memcg in @dmabuf->memcg.
• • Returns %true on success, %false if the charge against @new_memcg
• • failed or a concurrent transfer raced on @dmabuf->memcg.
• • On failure the caller retains its reference to @new_memcg.
• */

+bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 		       struct mem_cgroup *new_memcg)
  

I would make that more generic instead of dmabuf specific. E.g. something like this:

bool mem_cgroup_transfer_charge(struct struct mem_cgroup **ptr_memcg, struct mem_cgroup *new_memcg, enum memcg_stat_item item, unsigned int nr_pages)

+{

• struct mem_cgroup *old_memcg = READ_ONCE(dmabuf->memcg);

• unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;

This way we can also document in the DMA-buf caller that this conversation must be ok when memcg_account is true.

Regards, Christian.

• if (mem_cgroup_disabled())

• return true;
  

• if (!mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages))

• return false;
  

• mod_memcg_state(new_memcg, MEMCG_DMABUF, nr_pages);
• if (cmpxchg(&dmabuf->memcg, old_memcg, new_memcg) != old_memcg) {

• mod_memcg_state(new_memcg, MEMCG_DMABUF, -nr_pages);
  

• mem_cgroup_cancel_dmabuf(new_memcg, nr_pages);
  

• return false;
  

• }
• if (old_memcg) {

• /* Uncharge old cgroup */
  

• mod_memcg_state(old_memcg, MEMCG_DMABUF, -nr_pages);
  

• mem_cgroup_cancel_dmabuf(old_memcg, nr_pages);
  

• mem_cgroup_put(old_memcg);
  

• }
• return true;

+}

/**

• mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
• @dmabuf: buffer to uncharge

On Wed, Oct 28, 2026 at 05:09:37PM +0100, Albert Esteve via OP-TEE wrote:

On Wed, Oct 28, 2026 at 10:08:19AM +0100, Christian König wrote:

...

On 10/27/26 16:02, Albert Esteve wrote:

...

dma-heap already supports charging a specific cgroup at allocation time via charge_fd in DMA_HEAP_IOCTL_ALLOC. However, dma-bufs can originate from any exporter (DRM, V4L2, etc.), and the allocator may not know the final owner at allocation time.

Add DMA_BUF_IOCTL_XFER_CHARGE to transfer a dma-buf's memcg charge to a different cgroup after allocation. The target is identified by charge_fd, which may be a cgroupfd or a pidfd.

The transfer charges the new cgroup before uncharging the old one, preserving the original charge on failure. An atomic cmpxchg on dmabuf->memcg prevents races with concurrent transfers.

Not all dma-buf exporters back their buffers with reclaimable system memory. Some represent device registers, VRAM, or carved-out pools where size is not a meaningful memory cost. Add a memcg_account flag to dma_buf_export_info that exporters set when their buffers should participate in memcg accounting. The xfer ioctl returns -EOPNOTSUPP for buffers that do not set the flag. Currently only the system heap sets it, other exporters can opt in as needed.

Usage example: int cg_fd = open("/sys/fs/cgroup/clients/app1", O_RDONLY | O_DIRECTORY);

struct dma_buf_transfer_charge xfer = { .charge_fd = cg_fd, }; ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer); /* charge now on app1's cgroup */ close(cg_fd);

Signed-off-by: Albert Esteve aesteve@redhat.com

drivers/dma-buf/dma-buf.c | 43 +++++++++++++++++++++++++++++++++++++ drivers/dma-buf/heaps/system_heap.c | 1 + include/linux/dma-buf.h | 5 +++++ include/linux/memcontrol.h | 9 ++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++ mm/memcontrol.c | 41 +++++++++++++++++++++++++++++++++++ 6 files changed, 125 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index f31f49ebd0af..07513ecedc10 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -540,6 +540,45 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf, } #endif +/* Transfer a dma-buf's memcg charge; see struct dma_buf_transfer_charge */ +static long dma_buf_xfer_charge(struct dma_buf *dmabuf,

• 		const void __user *user_data)
  

+{

• struct dma_buf_transfer_charge arg;
• struct mem_cgroup *memcg;
• int ret = 0;
• if (copy_from_user(&arg, user_data, sizeof(arg)))

• return -EFAULT;
  

• if (!IS_ENABLED(CONFIG_MEMCG))

• return 0;
  

• if (arg.flags != 0)

• return -EINVAL;
  

• if (!dmabuf->memcg_account)

• return -EOPNOTSUPP;
  

• memcg = mem_cgroup_get_from_fd(arg.charge_fd);
• if (IS_ERR(memcg))

• return PTR_ERR(memcg);
  

• if (dmabuf->memcg == memcg)

• goto skip_transfer;
  

• if (!mem_cgroup_transfer_dmabuf_charge(dmabuf, memcg)) {

• ret = -ENOMEM;
  

• goto skip_transfer;
  

• }
• return 0;

+skip_transfer:

• mem_cgroup_put(memcg);
• return ret;

+}

static long dma_buf_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -590,6 +629,9 @@ static long dma_buf_ioctl(struct file *file, return dma_buf_import_sync_file(dmabuf, (const void __user *)arg); #endif

• case DMA_BUF_IOCTL_XFER_CHARGE:

• return dma_buf_xfer_charge(dmabuf, (const void __user *)arg);
  

• default: return -ENOTTY; }

@@ -750,6 +792,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) dmabuf->size = exp_info->size; dmabuf->exp_name = exp_info->exp_name; dmabuf->owner = exp_info->owner;

• dmabuf->memcg_account = exp_info->memcg_account;

The initial charging should probably be done here as well.

This way DMA-buf heaps only needs to pass in the charge_fd and we can re-use the functionality for other DMA-buf exporters as well.

The problem with doing the charge here is that it happens too late. That is the reasoning for this proposal to move all the charging logic into heap allocators before pages are allocated.

What we could do is fill the dmabuf->memcg field when the flag is set (i.e., committing into the dmabuf in the patch semantics). But for that we'd need adding the memcg we want to charge it to into exp_info in order to support charging to cgroups other than the allocator's (for the common case would be simpler as we can just look at current). Maybe that's what you meant?

Actually if I do this we can remove the memcg_account flag and just uses exp_info->memcg as both the memcg to set *and* the flag when it is != NULL. Exporters that set the memcg info are implicitly flagging the dmabuf as chargeable (and transfereable).

The only thing is that mod_memcg_state() still needs nr_pages and we cannot use size as that varies from one exporter to another as you explained before. So that can be left for the exporter to call. WDYT?

...

...

spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index adeea8cd328b..a764167167ba 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -479,6 +479,7 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap, exp_info.size = buffer->len; exp_info.flags = fd_flags; exp_info.priv = buffer;

• exp_info.memcg_account = true; dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf);

diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..ac0ec88cab93 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -433,6 +433,9 @@ struct dma_buf { /** @memcg: the cgroup to which this buffer is currently attributed */ struct mem_cgroup *memcg;

• /** @memcg_account: account buffer memory to cgroup */
• bool memcg_account;

}; /** @@ -505,6 +508,7 @@ struct dma_buf_attachment {

• @flags: mode flags for the file
• @resv: reservation-object, NULL to allocate default one
• @priv: Attach private data of allocator to this buffer

• • @memcg_account: account buffer memory to cgroup
  • This structure holds the information required to export the buffer. Used
  • with dma_buf_export() only.

@@ -517,6 +521,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv;

• bool memcg_account;

}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 6b30baa79fa9..36fddafbef4e 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -679,6 +679,9 @@ bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, void mem_cgroup_commit_dmabuf(struct dma_buf *dmabuf, struct mem_cgroup *memcg, unsigned int nr_pages); +bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 		       struct mem_cgroup *new_memcg);
  

void __mem_cgroup_uncharge(struct folio *folio); /** @@ -1206,6 +1209,12 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { } +static inline bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 				     struct mem_cgroup *new_memcg)
  

+{

• return true;

+}

static inline void mem_cgroup_uncharge(struct folio *folio) { } diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h index e827c9d20c5d..ea957f4b1d38 100644 --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -168,6 +168,31 @@ struct dma_buf_import_sync_file { __s32 fd; }; +/**

• • struct dma_buf_transfer_charge - Transfer a dma-buf's memcg charge
• • Any holder of a dma-buf file descriptor can perform a
• • DMA_BUF_IOCTL_XFER_CHARGE to attribute the buffer's memory
• • cost to a specific cgroup, or move it from one cgroup to
• • another. The ioctl charges the new cgroup before uncharging
• • the old one atomically. On failure the original charge is
• • preserved. If the buffer has no existing charge, a new
• • charge is established.
• */

+struct dma_buf_transfer_charge {

• /** @flags: Reserved, must be zero */
• __u32 flags; /* unused for now */
• /**

• * @charge_fd: File descriptor identifying the target cgroup.
  

• *
  

• * May be either a pidfd, in which case the charge moves to the
  

• * memory cgroup of the referenced process, or a cgroupfd obtained
  

• * by opening a cgroup v2 directory, in which case the charge moves
  

• * directly to that cgroup's memory controller.
  

• */
  

• __u32 charge_fd;

+};

#define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) @@ -179,5 +204,6 @@ struct dma_buf_import_sync_file { #define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, __u64) #define DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct dma_buf_export_sync_file) #define DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct dma_buf_import_sync_file) +#define DMA_BUF_IOCTL_XFER_CHARGE _IOW(DMA_BUF_BASE, 4, struct dma_buf_transfer_charge) #endif diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 7b841b905759..3adc84fe6668 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5786,6 +5786,47 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) refill_stock(memcg, nr_pages); } +/**

• • mem_cgroup_transfer_dmabuf_charge - move a dma-buf charge
• • @dmabuf: buffer to transfer charge
• • @new_memcg: target memcg; caller must hold a reference
• • Transfers the charge for @dmabuf to @new_memcg and, on
• • success, stores @new_memcg in @dmabuf->memcg.
• • Returns %true on success, %false if the charge against @new_memcg
• • failed or a concurrent transfer raced on @dmabuf->memcg.
• • On failure the caller retains its reference to @new_memcg.
• */

+bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 		       struct mem_cgroup *new_memcg)
  

I would make that more generic instead of dmabuf specific. E.g. something like this:

bool mem_cgroup_transfer_charge(struct struct mem_cgroup **ptr_memcg, struct mem_cgroup *new_memcg, enum memcg_stat_item item, unsigned int nr_pages)

Ah yes, I like this generalization. And the implementation remains mostly the same. Thanks!

...

...

+{

• struct mem_cgroup *old_memcg = READ_ONCE(dmabuf->memcg);

...

• unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;

This way we can also document in the DMA-buf caller that this conversation must be ok when memcg_account is true.

Regards, Christian.

...

• if (mem_cgroup_disabled())

• return true;
  

• if (!mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages))

• return false;
  

• mod_memcg_state(new_memcg, MEMCG_DMABUF, nr_pages);
• if (cmpxchg(&dmabuf->memcg, old_memcg, new_memcg) != old_memcg) {

• mod_memcg_state(new_memcg, MEMCG_DMABUF, -nr_pages);
  

• mem_cgroup_cancel_dmabuf(new_memcg, nr_pages);
  

• return false;
  

• }
• if (old_memcg) {

• /* Uncharge old cgroup */
  

• mod_memcg_state(old_memcg, MEMCG_DMABUF, -nr_pages);
  

• mem_cgroup_cancel_dmabuf(old_memcg, nr_pages);
  

• mem_cgroup_put(old_memcg);
  

• }
• return true;

+}

/**

• mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
• @dmabuf: buffer to uncharge

On Thu, Oct 29, 2026 at 03:16:12PM +0100, Christian König wrote:

On 10/28/26 17:09, Albert Esteve wrote:

...

On Wed, Oct 28, 2026 at 10:08:19AM +0100, Christian König wrote:

...

On 10/27/26 16:02, Albert Esteve wrote:

...

dma-heap already supports charging a specific cgroup at allocation time via charge_fd in DMA_HEAP_IOCTL_ALLOC. However, dma-bufs can originate from any exporter (DRM, V4L2, etc.), and the allocator may not know the final owner at allocation time.

Add DMA_BUF_IOCTL_XFER_CHARGE to transfer a dma-buf's memcg charge to a different cgroup after allocation. The target is identified by charge_fd, which may be a cgroupfd or a pidfd.

The transfer charges the new cgroup before uncharging the old one, preserving the original charge on failure. An atomic cmpxchg on dmabuf->memcg prevents races with concurrent transfers.

Not all dma-buf exporters back their buffers with reclaimable system memory. Some represent device registers, VRAM, or carved-out pools where size is not a meaningful memory cost. Add a memcg_account flag to dma_buf_export_info that exporters set when their buffers should participate in memcg accounting. The xfer ioctl returns -EOPNOTSUPP for buffers that do not set the flag. Currently only the system heap sets it, other exporters can opt in as needed.

Usage example: int cg_fd = open("/sys/fs/cgroup/clients/app1", O_RDONLY | O_DIRECTORY);

struct dma_buf_transfer_charge xfer = { .charge_fd = cg_fd, }; ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer); /* charge now on app1's cgroup */ close(cg_fd);

Signed-off-by: Albert Esteve aesteve@redhat.com

drivers/dma-buf/dma-buf.c | 43 +++++++++++++++++++++++++++++++++++++ drivers/dma-buf/heaps/system_heap.c | 1 + include/linux/dma-buf.h | 5 +++++ include/linux/memcontrol.h | 9 ++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++ mm/memcontrol.c | 41 +++++++++++++++++++++++++++++++++++ 6 files changed, 125 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index f31f49ebd0af..07513ecedc10 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -540,6 +540,45 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf, } #endif +/* Transfer a dma-buf's memcg charge; see struct dma_buf_transfer_charge */ +static long dma_buf_xfer_charge(struct dma_buf *dmabuf,

• 		const void __user *user_data)
  

+{

• struct dma_buf_transfer_charge arg;
• struct mem_cgroup *memcg;
• int ret = 0;
• if (copy_from_user(&arg, user_data, sizeof(arg)))

• return -EFAULT;
  

• if (!IS_ENABLED(CONFIG_MEMCG))

• return 0;
  

• if (arg.flags != 0)

• return -EINVAL;
  

• if (!dmabuf->memcg_account)

• return -EOPNOTSUPP;
  

• memcg = mem_cgroup_get_from_fd(arg.charge_fd);
• if (IS_ERR(memcg))

• return PTR_ERR(memcg);
  

• if (dmabuf->memcg == memcg)

• goto skip_transfer;
  

• if (!mem_cgroup_transfer_dmabuf_charge(dmabuf, memcg)) {

• ret = -ENOMEM;
  

• goto skip_transfer;
  

• }
• return 0;

+skip_transfer:

• mem_cgroup_put(memcg);
• return ret;

+}

static long dma_buf_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -590,6 +629,9 @@ static long dma_buf_ioctl(struct file *file, return dma_buf_import_sync_file(dmabuf, (const void __user *)arg); #endif

• case DMA_BUF_IOCTL_XFER_CHARGE:

• return dma_buf_xfer_charge(dmabuf, (const void __user *)arg);
  

• default: return -ENOTTY; }

@@ -750,6 +792,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) dmabuf->size = exp_info->size; dmabuf->exp_name = exp_info->exp_name; dmabuf->owner = exp_info->owner;

• dmabuf->memcg_account = exp_info->memcg_account;

The initial charging should probably be done here as well.

This way DMA-buf heaps only needs to pass in the charge_fd and we can re-use the functionality for other DMA-buf exporters as well.

The problem with doing the charge here is that it happens too late. That is the reasoning for this proposal to move all the charging logic into heap allocators before pages are allocated.

Oh, that is a really good point we need to talk about.

DMA-buf heaps is actually a rather simple use case, but we have way more complex things.

For example in some use cases the backing store of a DMA-buf is only allocated on first use and only then it needs to be comitted to it's memcg or dmem.

Yes. The idea is focus on heaps for now. But the uAPI is actually agnostic of the time when the charge happens. When we deal with lazy-allocation in the future we can just call the reserve/cancel helpers wherever is more appropiate. As long as the pages are not allocated, exporters do not set dmabuf->memcg, and XFER does not work (returns -EOPNOTSUPP).

I don't mind implementing it for DMA-buf heaps first, but we should have the uAPI and it's handling so solid that it also works for future use cases.

Agreed. I think we should cover all usecases with the current status. Your feedback helped a lot in getting there!

...

What we could do is fill the dmabuf->memcg field when the flag is set (i.e., committing into the dmabuf in the patch semantics). But for that we'd need adding the memcg we want to charge it to into exp_info in order to support charging to cgroups other than the allocator's (for the common case would be simpler as we can just look at current). Maybe that's what you meant?

At least for the DMA-buf heaps use case that sounds reasonable for now.

Thanks! Next version will have the flag removed and just use dmabuf->memcg as indicator, set within dma_buf_export(). Also, `nr_pages` will be computed within dma_buf_export(), the contract being that exporters setting exp_info.memcg assert that size reflects actual system memory bytes.

Regards, Christian.

...

...

...

spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index adeea8cd328b..a764167167ba 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -479,6 +479,7 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap, exp_info.size = buffer->len; exp_info.flags = fd_flags; exp_info.priv = buffer;

• exp_info.memcg_account = true; dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf);

diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..ac0ec88cab93 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -433,6 +433,9 @@ struct dma_buf { /** @memcg: the cgroup to which this buffer is currently attributed */ struct mem_cgroup *memcg;

• /** @memcg_account: account buffer memory to cgroup */
• bool memcg_account;

}; /** @@ -505,6 +508,7 @@ struct dma_buf_attachment {

• @flags: mode flags for the file
• @resv: reservation-object, NULL to allocate default one
• @priv: Attach private data of allocator to this buffer

• • @memcg_account: account buffer memory to cgroup
  • This structure holds the information required to export the buffer. Used
  • with dma_buf_export() only.

@@ -517,6 +521,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv;

• bool memcg_account;

}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 6b30baa79fa9..36fddafbef4e 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -679,6 +679,9 @@ bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, void mem_cgroup_commit_dmabuf(struct dma_buf *dmabuf, struct mem_cgroup *memcg, unsigned int nr_pages); +bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 		       struct mem_cgroup *new_memcg);
  

void __mem_cgroup_uncharge(struct folio *folio); /** @@ -1206,6 +1209,12 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { } +static inline bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 				     struct mem_cgroup *new_memcg)
  

+{

• return true;

+}

static inline void mem_cgroup_uncharge(struct folio *folio) { } diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h index e827c9d20c5d..ea957f4b1d38 100644 --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -168,6 +168,31 @@ struct dma_buf_import_sync_file { __s32 fd; }; +/**

• • struct dma_buf_transfer_charge - Transfer a dma-buf's memcg charge
• • Any holder of a dma-buf file descriptor can perform a
• • DMA_BUF_IOCTL_XFER_CHARGE to attribute the buffer's memory
• • cost to a specific cgroup, or move it from one cgroup to
• • another. The ioctl charges the new cgroup before uncharging
• • the old one atomically. On failure the original charge is
• • preserved. If the buffer has no existing charge, a new
• • charge is established.
• */

+struct dma_buf_transfer_charge {

• /** @flags: Reserved, must be zero */
• __u32 flags; /* unused for now */
• /**

• * @charge_fd: File descriptor identifying the target cgroup.
  

• *
  

• * May be either a pidfd, in which case the charge moves to the
  

• * memory cgroup of the referenced process, or a cgroupfd obtained
  

• * by opening a cgroup v2 directory, in which case the charge moves
  

• * directly to that cgroup's memory controller.
  

• */
  

• __u32 charge_fd;

+};

#define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) @@ -179,5 +204,6 @@ struct dma_buf_import_sync_file { #define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, __u64) #define DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct dma_buf_export_sync_file) #define DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct dma_buf_import_sync_file) +#define DMA_BUF_IOCTL_XFER_CHARGE _IOW(DMA_BUF_BASE, 4, struct dma_buf_transfer_charge) #endif diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 7b841b905759..3adc84fe6668 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5786,6 +5786,47 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) refill_stock(memcg, nr_pages); } +/**

• • mem_cgroup_transfer_dmabuf_charge - move a dma-buf charge
• • @dmabuf: buffer to transfer charge
• • @new_memcg: target memcg; caller must hold a reference
• • Transfers the charge for @dmabuf to @new_memcg and, on
• • success, stores @new_memcg in @dmabuf->memcg.
• • Returns %true on success, %false if the charge against @new_memcg
• • failed or a concurrent transfer raced on @dmabuf->memcg.
• • On failure the caller retains its reference to @new_memcg.
• */

+bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 		       struct mem_cgroup *new_memcg)
  

I would make that more generic instead of dmabuf specific. E.g. something like this:

bool mem_cgroup_transfer_charge(struct struct mem_cgroup **ptr_memcg, struct mem_cgroup *new_memcg, enum memcg_stat_item item, unsigned int nr_pages)

Ah yes, I like this generalization. And the implementation remains mostly the same. Thanks!

...

...

+{

• struct mem_cgroup *old_memcg = READ_ONCE(dmabuf->memcg);

...

• unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;

This way we can also document in the DMA-buf caller that this conversation must be ok when memcg_account is true.

Regards, Christian.

...

• if (mem_cgroup_disabled())

• return true;
  

• if (!mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages))

• return false;
  

• mod_memcg_state(new_memcg, MEMCG_DMABUF, nr_pages);
• if (cmpxchg(&dmabuf->memcg, old_memcg, new_memcg) != old_memcg) {

• mod_memcg_state(new_memcg, MEMCG_DMABUF, -nr_pages);
  

• mem_cgroup_cancel_dmabuf(new_memcg, nr_pages);
  

• return false;
  

• }
• if (old_memcg) {

• /* Uncharge old cgroup */
  

• mod_memcg_state(old_memcg, MEMCG_DMABUF, -nr_pages);
  

• mem_cgroup_cancel_dmabuf(old_memcg, nr_pages);
  

• mem_cgroup_put(old_memcg);
  

• }
• return true;

+}

/**

• mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
• @dmabuf: buffer to uncharge

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] dma-buf: unprivileged cgroup escape and OOM via dma_buf_xfer_charge() - [High] dma-buf: race in mem_cgroup_transfer_dmabuf_charge() double-charges memcg - [Medium] dma-buf: broken disabled-memcg handling in dma_buf_xfer_charge() --

commit 1660729e62b15fd915c69f26225530bd5ae30ce0 Author: Albert Esteve aesteve@redhat.com

dma-buf: transfer memcg charges via DMA_BUF_IOCTL_XFER_CHARGE

This commit adds DMA_BUF_IOCTL_XFER_CHARGE to allow transferring a dma-buf's memory cgroup charge to a cgroup identified by a cgroupfd or pidfd after allocation. It also introduces a memcg_account flag in dma_buf_export_info for exporters to opt in to memcg accounting, enabling it for the system heap.

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index f31f49ebd0af3..07513ecedc105 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -540,6 +540,45 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf, } #endif +/* Transfer a dma-buf's memcg charge; see struct dma_buf_transfer_charge */ +static long dma_buf_xfer_charge(struct dma_buf *dmabuf,

• 		const void __user *user_data)
  

+{

• struct dma_buf_transfer_charge arg;
• struct mem_cgroup *memcg;
• int ret = 0;
• if (copy_from_user(&arg, user_data, sizeof(arg)))

• return -EFAULT;
  

• if (!IS_ENABLED(CONFIG_MEMCG))

• return 0;
  

• if (arg.flags != 0)

• return -EINVAL;
  

• if (!dmabuf->memcg_account)

• return -EOPNOTSUPP;
  

[Severity: Medium] When CONFIG_MEMCG is disabled, should dma_buf_xfer_charge() still validate arg.flags and dmabuf->memcg_account, or return -EOPNOTSUPP?

Returning 0 before checking arg.flags != 0 and !dmabuf->memcg_account causes calls with invalid flags, non-accounting exporters, or invalid file descriptors to silently succeed under CONFIG_MEMCG=n, while also leaving the !CONFIG_MEMCG stubs for mem_cgroup_get_from_fd() (which returns ERR_PTR(-EOPNOTSUPP)) and mem_cgroup_transfer_dmabuf_charge() in include/linux/memcontrol.h unreachable.

Also, when CONFIG_MEMCG=y and cgroup_disable=memory is passed on the kernel command line (so mem_cgroup_disabled() is true at runtime), mem_cgroup_get_from_fd() behaves inconsistently across its two branches:

mm/memcontrol.c:mem_cgroup_get_from_fd() { cgrp = cgroup_get_from_fd(fd); if (!IS_ERR(cgrp)) { rcu_read_lock(); css = cgroup_css(cgrp, &memory_cgrp_subsys); if (!css || !css_tryget_online(css)) { rcu_read_unlock(); cgroup_put(cgrp); return ERR_PTR(-ENOENT); } ... memcg = get_mem_cgroup_from_mm(mm); mmput(mm); return memcg; }

For a cgroupfd, cgroup_css() is NULL and it returns ERR_PTR(-ENOENT), whereas for a pidfd, get_mem_cgroup_from_mm() returns NULL rather than an ERR_PTR, so dma_buf_xfer_charge() returns 0 for pidfds and -ENOENT for cgroupfds.

• memcg = mem_cgroup_get_from_fd(arg.charge_fd);
• if (IS_ERR(memcg))

• return PTR_ERR(memcg);
  

• if (dmabuf->memcg == memcg)

• goto skip_transfer;
  

• if (!mem_cgroup_transfer_dmabuf_charge(dmabuf, memcg)) {

• ret = -ENOMEM;
  

• goto skip_transfer;
  

• }

[Severity: High] Can an unprivileged holder of a dma-buf file descriptor use dma_buf_xfer_charge() to escape its own memory cgroup limit or trigger OOM kills in another cgroup?

In the call chain:

dma_buf_ioctl() -> dma_buf_xfer_charge() -> mem_cgroup_get_from_fd() -> mem_cgroup_transfer_dmabuf_charge() -> mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages) -> try_charge()

neither dma_buf_xfer_charge() nor mem_cgroup_get_from_fd() checks DAC or cgroup delegation/write permissions (such as inode_permission() with MAY_WRITE or cgroup_procs_write_permission()), capabilities, or ptrace_may_access() on either the source cgroup (dmabuf->memcg) being uncharged or the target cgroup/task (new_memcg) being charged.

Because /sys/fs/cgroup directories are normally world-readable (mode 0755, so any user can open them with O_RDONLY | O_DIRECTORY) and pidfd_open() can open a pidfd for PID 1 or any visible process without privilege, an unprivileged process can resolve a handle to the root cgroup or to another cgroup on the system.

When SELinux is not active, security_dma_buf_charge_cgroup() and security_dma_buf_charge_task() return 0. Even with SELinux enabled, selinux_dma_buf_charge_cgroup() returns 0 when the cgroup kernfs node has no explicit security.selinux xattr set:

security/selinux/hooks.c:selinux_dma_buf_charge_cgroup() { rc = kernfs_xattr_get(cgrp->kn, XATTR_NAME_SELINUX, NULL, 0); if (rc == -ENODATA) return 0; }

and neither hook checks permission to uncharge the existing dmabuf->memcg. As a result, a confined client receiving a pre-charged dma-buf fd can move the charge to the root cgroup to bypass its own memory.max limit, or move the charge into a victim cgroup where mem_cgroup_reserve_dmabuf() with GFP_KERNEL triggers direct reclaim and the memcg OOM killer.

diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 7b841b9057594..3adc84fe6668d 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5786,6 +5786,47 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) refill_stock(memcg, nr_pages); } +/**

• • mem_cgroup_transfer_dmabuf_charge - move a dma-buf charge
• • @dmabuf: buffer to transfer charge
• • @new_memcg: target memcg; caller must hold a reference
• • Transfers the charge for @dmabuf to @new_memcg and, on
• • success, stores @new_memcg in @dmabuf->memcg.
• • Returns %true on success, %false if the charge against @new_memcg
• • failed or a concurrent transfer raced on @dmabuf->memcg.
• • On failure the caller retains its reference to @new_memcg.
• */

+bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,

• 		       struct mem_cgroup *new_memcg)
  

+{

• struct mem_cgroup *old_memcg = READ_ONCE(dmabuf->memcg);
• unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
• if (mem_cgroup_disabled())

• return true;
  

• if (!mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages))

• return false;
  

• mod_memcg_state(new_memcg, MEMCG_DMABUF, nr_pages);
• if (cmpxchg(&dmabuf->memcg, old_memcg, new_memcg) != old_memcg) {

• mod_memcg_state(new_memcg, MEMCG_DMABUF, -nr_pages);
  

• mem_cgroup_cancel_dmabuf(new_memcg, nr_pages);
  

• return false;
  

• }

[Severity: High] What happens if two threads concurrently call DMA_BUF_IOCTL_XFER_CHARGE on the same dma-buf?

In dma_buf_xfer_charge(), dmabuf->memcg == memcg is checked without synchronization before calling mem_cgroup_transfer_dmabuf_charge(), which then re-reads old_memcg = READ_ONCE(dmabuf->memcg) without checking whether old_memcg == new_memcg.

If thread 1 transfers the buffer to new_memcg after thread 2 passes the dmabuf->memcg == memcg check in dma_buf_xfer_charge() (or if both threads concurrently transfer the buffer to the same new_memcg), thread 2 sees old_memcg == new_memcg and still calls mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages) before uncharging old_memcg. That transiently charges 2 * nr_pages against new_memcg for the same buffer, which can trigger reclaim, memcg OOM kills, or -ENOMEM in new_memcg even though the buffer is already charged to it.

In addition, old_memcg is read before mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages), which can sleep in direct reclaim. If another thread updates dmabuf->memcg while mem_cgroup_reserve_dmabuf() is sleeping, the cmpxchg(&dmabuf->memcg, old_memcg, new_memcg) fails without retrying against the updated old_memcg and returns false, which dma_buf_xfer_charge() reports to userspace as -ENOMEM even though charging new_memcg succeeded.

Finally, if mem_cgroup_transfer_dmabuf_charge() is called when mem_cgroup_disabled() is true, returning true at the top without storing new_memcg in dmabuf->memcg or dropping new_memcg's reference would leak the caller's reference to new_memcg, since callers only call mem_cgroup_put() when this function returns false.

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [Medium] selftests/dmabuf-heaps: check_deps() runs before handle_build() in vmtest.sh - [Medium] selftests/dmabuf-heaps: fail instead of skip on EOPNOTSUPP in XFER_CHARGE test - [Medium] selftests/dmabuf-heaps: pidfd_alloc_supported() treats EOPNOTSUPP as supported - [Medium] selftests: dmabuf-heaps: no memory controller check in open_self_cgroupfd() - [Medium] selftests: cgroup: MEMCG_DMABUF_WAIT_RETRIES is shorter than 2s rstat FLUSH_TIME --

commit 764a620a8574ac6b7e6c57e6f71e4c743c373656 Author: Albert Esteve aesteve@redhat.com

selftests/dmabuf-heaps: Add dma-buf memcg accounting tests

This commit adds selftests for dma-buf memcg accounting via charge_fd in DMA_HEAP_IOCTL_ALLOC and DMA_BUF_IOCTL_XFER_CHARGE across the dmabuf-heaps and cgroup test suites, along with a vmtest.sh runner script.

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c index 0ed82347044ed..0b41c3c6f62c1 100644 --- a/tools/testing/selftests/cgroup/test_memcontrol.c +++ b/tools/testing/selftests/cgroup/test_memcontrol.c @@ -19,10 +19,20 @@ #include <errno.h> #include <sys/mman.h> +#include <linux/dma-buf.h> +#include <linux/dma-heap.h> +#include <signal.h> +#include <sys/ioctl.h>

+#include "../pidfd/pidfd.h" #include "kselftest.h" #include "cgroup_util.h" +#define DMA_HEAP_SYSTEM "/dev/dma_heap/system" +#define ONE_MEG (1024 * 1024)

#define MEMCG_SOCKSTAT_WAIT_RETRIES 30 +#define MEMCG_DMABUF_WAIT_RETRIES 15

[Severity: Medium] Is MEMCG_DMABUF_WAIT_RETRIES (15 retries at 100ms intervals, or 1.4 seconds before the final read) long enough to avoid races with the periodic rstat flush in test_memcg_dmabuf() and test_memcg_dmabuf_xfer_at_limit()?

In mm/memcontrol.c, reading memory.stat calls mem_cgroup_flush_stats(), which only flushes rstat synchronously when stats_updates exceeds MEMCG_CHARGE_BATCH * num_online_cpus() (64 * num_online_cpus()):

mm/memcontrol.c:mem_cgroup_flush_stats() if (atomic_long_read(&vmstats->stats_updates) > MEMCG_CHARGE_BATCH * num_online_cpus()) ...

Charging or uncharging a ONE_MEG (1 MB) dma-buf in a fresh child cgroup updates MEMCG_DMABUF by 256 pages on 4KB-page kernels (or 16 pages on 64KB-page kernels). When num_online_cpus() >= 4 (where 256 > 64 * 4 is false) or on >4KB-page kernels, reading memory.stat skips synchronous flushing and relies on the periodic deferred work stats_flush_dwork, which runs every FLUSH_TIME (2UL * HZ, or 2.0 seconds).

In test_memcg_dmabuf(), even if the first poll for ONE_MEG catches a periodic flush, that flush resets stats_updates to 0 and schedules the next stats_flush_dwork 2.0 seconds later. Immediately closing dmabuf_fd adds only 256 to stats_updates, so the subsequent 1.4-second poll for dmabuf == 0 can time out before the next 2.0-second periodic flush runs. Should MEMCG_DMABUF_WAIT_RETRIES be at least 30, matching MEMCG_SOCKSTAT_WAIT_RETRIES?

diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c index 1d49df6719198..90ef14d2ecba0 100644 --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c

[ ... ]

@@ -496,6 +505,218 @@ static void test_alloc_no_fd_leak_on_efault(char *heap_name)

[ ... ]

+/*

• • Probe whether the kernel honours charge_fd in DMA_HEAP_IOCTL_ALLOC.
• */

+static bool pidfd_alloc_supported(int heap_fd) +{

• int devnull_fd, dmabuf_fd = -1, ret;
• devnull_fd = open("/dev/null", O_RDONLY);
• if (devnull_fd < 0)

• return false;
  

• ret = dmabuf_heap_alloc_pidfd(heap_fd, ONE_MEG, 0, devnull_fd, &dmabuf_fd);
• if (dmabuf_fd >= 0) {

• close(dmabuf_fd);
  

• dmabuf_fd = -1;
  

• }
• close(devnull_fd);
• return ret < 0;

+}

[Severity: Medium] What happens in pidfd_alloc_supported() when the kernel is built with CONFIG_MEMCG=n?

In drivers/dma-buf/dma-heap.c, dma_heap_ioctl_allocate() passes a non-zero charge_fd to mem_cgroup_get_from_fd():

drivers/dma-buf/dma-heap.c:dma_heap_ioctl_allocate() if (heap_allocation->charge_fd) { memcg = mem_cgroup_get_from_fd(heap_allocation->charge_fd); if (IS_ERR(memcg)) return ERR_CAST(memcg); }

When CONFIG_MEMCG is disabled, the stub in include/linux/memcontrol.h returns ERR_PTR(-EOPNOTSUPP):

include/linux/memcontrol.h:mem_cgroup_get_from_fd() static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) { return ERR_PTR(-EOPNOTSUPP); }

Because dmabuf_heap_alloc_pidfd() returns -1 with errno == EOPNOTSUPP, returning ret < 0 causes pidfd_alloc_supported() to return true when CONFIG_MEMCG=n, and test_alloc_pidfd_self(), test_alloc_pidfd_child(), test_alloc_cgroupfd_self(), and test_xfer_charge_cgroupfd_self() then proceed and report test failures instead of skipping. Should pidfd_alloc_supported() also verify that errno != EOPNOTSUPP (or errno == EINVAL)?

[ ... ]

+static int open_self_cgroupfd(void) +{

• char cgroup_path[PATH_MAX];
• char buf[PATH_MAX];
• FILE *f;
• f = fopen("/proc/self/cgroup", "r");
• if (!f)

• return -1;
  

• cgroup_path[0] = '\0';
• while (fgets(buf, sizeof(buf), f)) {

• char *p = buf;
  

• if (strncmp(p, "0::", 3) == 0) {
  

• 	p += 3;
  

• 	p[strcspn(p, "\n")] = '\0';
  

• 	snprintf(cgroup_path, sizeof(cgroup_path),
  

• 		 "/sys/fs/cgroup%s", p);
  

• 	break;
  

• }
  

• }
• fclose(f);
• if (!cgroup_path[0])

• return -1;
  

• return open(cgroup_path, O_RDONLY | O_DIRECTORY);

+}

+/*

• • Test: allocate charging the calling process's own cgroup via a cgroupfd.
• */

+static void test_alloc_cgroupfd_self(char *heap_name) +{

• int heap_fd = -1, cgroup_fd = -1, dmabuf_fd = -1, ret;
• heap_fd = dmabuf_heap_open(heap_name);
• if (!pidfd_alloc_supported(heap_fd)) {

• ksft_test_result_skip("charge_fd not supported by this kernel\n");
  

• goto out;
  

• }
• cgroup_fd = open_self_cgroupfd();
• if (cgroup_fd < 0) {

• ksft_test_result_skip("cannot open self cgroupfd\n");
  

• goto out;
  

• }
• ret = dmabuf_heap_alloc_pidfd(heap_fd, ONE_MEG, 0, cgroup_fd, &dmabuf_fd);
• ksft_test_result(!ret, "Allocation with cgroupfd %d\n", ret);
• if (dmabuf_fd >= 0)

• close(dmabuf_fd);
  

• close(cgroup_fd);

+out:

• close(heap_fd);

+}

[Severity: Medium] Can test_alloc_cgroupfd_self() and test_xfer_charge_cgroupfd_self() fail with ENOENT instead of skipping when the test binary is run outside vmtest.sh (for example via make run_tests or vmtest.sh -s) in a cgroup where the memory controller is not enabled?

Unlike the pidfd path in mem_cgroup_get_from_fd(), which uses get_mem_cgroup_from_mm() and walks up to an online ancestor, the cgroupfd path requires the memory controller CSS to be directly enabled on the target cgroup itself:

mm/memcontrol.c:mem_cgroup_get_from_fd() cgrp = cgroup_get_from_fd(fd); if (!IS_ERR(cgrp)) { rcu_read_lock(); css = cgroup_css(cgrp, &memory_cgrp_subsys); if (!css || !css_tryget_online(css)) { rcu_read_unlock(); cgroup_put(cgrp); return ERR_PTR(-ENOENT); }

When dmabuf-heap runs in a leaf cgroup (such as a standard systemd user or service scope) whose parent has not enabled +memory in cgroup.subtree_control, or when booted with cgroup_disable=memory, open_self_cgroupfd() still succeeds in opening the directory, and the ioctl fails with ENOENT. Should open_self_cgroupfd() check that memory is active in that cgroup (or should these two tests skip when the ioctl fails with errno == ENOENT)?

+/*

• • Test: transfer a dma-buf's charge to self cgroup via DMA_BUF_IOCTL_XFER_CHARGE.
• */

+static void test_xfer_charge_cgroupfd_self(char *heap_name) +{

• int heap_fd = -1, cgroup_fd = -1, dmabuf_fd = -1, ret;
• struct dma_buf_transfer_charge xfer = { 0 };
• heap_fd = dmabuf_heap_open(heap_name);
• if (!pidfd_alloc_supported(heap_fd)) {

• ksft_test_result_skip("charge_fd not supported by this kernel\n");
  

• goto out;
  

• }
• ret = dmabuf_heap_alloc(heap_fd, ONE_MEG, 0, &dmabuf_fd);
• if (ret) {

• ksft_test_result_fail("Allocation failed %d\n", ret);
  

• goto out;
  

• }
• cgroup_fd = open_self_cgroupfd();
• if (cgroup_fd < 0) {

• ksft_test_result_skip("cannot open self cgroupfd\n");
  

• close(dmabuf_fd);
  

• goto out;
  

• }
• xfer.charge_fd = cgroup_fd;
• ret = ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer);
• ksft_test_result(!ret, "XFER_CHARGE to self cgroupfd %d\n", ret);

[Severity: Medium] Does test_xfer_charge_cgroupfd_self() report a failure instead of skipping when main() iterates over non-system heaps in /dev/dma_heap (such as CMA heaps when CONFIG_DMABUF_HEAPS_CMA=y)?

In drivers/dma-buf/dma-heap.c, dma_heap_ioctl_allocate() validates charge_fd before calling heap->ops->allocate(), so pidfd_alloc_supported() returns true for all heaps. However, in drivers/dma-buf/dma-buf.c, dma_buf_xfer_charge() checks whether the exporter enabled memcg accounting on the buffer:

drivers/dma-buf/dma-buf.c:dma_buf_xfer_charge() if (!dmabuf->memcg_account) return -EOPNOTSUPP;

Because only system_heap sets exp_info.memcg_account = true (while exporters such as cma_heap leave it false), ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer) returns -1 with errno == EOPNOTSUPP on those heaps. Should test_xfer_charge_cgroupfd_self() call ksft_test_result_skip() when ret < 0 and errno == EOPNOTSUPP?

diff --git a/tools/testing/selftests/dmabuf-heaps/vmtest.sh b/tools/testing/selftests/dmabuf-heaps/vmtest.sh new file mode 100755 index 0000000000000..3574b71ca83e8 --- /dev/null +++ b/tools/testing/selftests/dmabuf-heaps/vmtest.sh

[ ... ]

+check_deps() {

• for dep in vng make; do

• if [[ ! -x $(command -v "${dep}") ]]; then
  

• 	echo -e "skip:    dependency ${dep} not found!\n"
  

• 	exit "${KSFT_SKIP}"
  

• fi
  

• done
• if ${SHELL_MODE}; then

• return
  

• fi
• if [[ ! -x "${DMABUF_HEAP_TEST}" ]]; then

• printf "skip:    %s not found!" "${DMABUF_HEAP_TEST}"
  

• printf " Please build the kselftest dmabuf-heaps target (or use -b).\n"
  

• exit "${KSFT_SKIP}"
  

• fi
• if [[ ! -x "${MEMCONTROL_TEST}" ]]; then

• printf "skip:    %s not found!" "${MEMCONTROL_TEST}"
  

• printf " Please build the kselftest cgroup target (or use -b).\n"
  

• exit "${KSFT_SKIP}"
  

• fi

+}

[ ... ]

+trap cleanup EXIT

+check_deps +check_vng +handle_build +make_runner

[Severity: Medium] If vmtest.sh is invoked with -b on a clean tree where the selftest binaries have not been built yet, won't check_deps() exit with KSFT_SKIP before handle_build() has a chance to build them?

In the main flow, check_deps() runs before handle_build(), and inside check_deps() only SHELL_MODE returns early before checking for DMABUF_HEAP_TEST and MEMCONTROL_TEST. Should check_deps() also skip the binary existence checks when BUILD is true (or run after handle_build())?