Shared-memory allocator services (e.g., Android's gralloc) allocate dma-bufs on behalf of client processes in different cgroups. Today those allocations are charged to the allocator's own cgroup, leaving client cgroups with no visibility into their actual memory consumption and defeating resource limits.
This series adds memory cgroup accounting for dma-buf allocations, building on T.J. Mercier's earlier series [1] and incorporating feedback from previous versions and LPC discussions [2].
Patch 1 introduces the MEMCG_DMABUF stat and reserve/cancel and uncharge helpers in mm/memcontrol.c
Patch 2 wires up the charging policy: dma_buf_export() stores exp_info.memcg and updates the stat, a new charge_fd field in DMA_HEAP_IOCTL_ALLOC accepts a pidfd or cgroupfd to identify the target cgroup, and __GFP_ACCOUNT is removed from system-heap allocations to avoid double counting.
Patch 3 adds DMA_BUF_IOCTL_XFER_CHARGE, a general ioctl on any dma-buf fd to transfer charges post-allocation. This covers the double-hop allocator case (SurfaceFlinger -> gralloc -> kernel) and works for buffers from any exporter (DRM, V4L2, etc.). Charging is gated by dmabuf->memcg being set at export time, ensuring that transfer is only applied for buffers backed by system memory where size reflects actual bytes. If the receiving cgroup lacks capacity for the charge, the ioctl returns -ENOMEM and the original charge is preserved.
Patches 4-7 add access control for cross-cgroup charging. Letting userspace decide who to charge needs to be a privileged operation because it has material impact on OOM decission and resource limits. We gate it via LSM hooks for the two charge_fd flavours: security_dma_buf_charge_task() gates the pidfd path (target identified by task credentials) and security_dma_buf_charge_cgroup() gates the cgroupfd path (target identified by cgroup directory label), with SELinux implementations under a dma_buf object class.
Patch 8 adds selftests.
Known limitations:
- Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. - Only memcg accounting. CMA heaps would additionally need dmem controller integration. - If a transfer of an uncharged buffer fails, the buffer remains charged to the original cgroup. The allocator (userspace) is expected to handle this (retry, free, ...). - The LSM hooks are intentionally narrow. Broader cgroup access controls (e.g. for the memfd/kexec handover case [2]) could be a separate effort reusing the same pattern. - Only system heap sets exp_info.memcg today. Other exporters (DRM, V4L2, etc.) can opt in by setting it, but are not covered by this series. - Related to the above: lazy backing-store allocation is not supported. This can be extended later, e.g., by adding a flag on the dma-buf struct for explicitly marking when memory is committed.
[1] https://lore.kernel.org/cgroups/20230109213809.418135-1-tjmercier@google.com... [2] https://lpc.events/event/20/contributions/2416/
Signed-off-by: Albert Esteve aesteve@redhat.com --- Changes in v4: - Remove memcg_account flag; exp_info.memcg != NULL is the opt-in - Centralize memcg bookkeeping in dma_buf_export() - Drop mem_cgroup_commit_dmabuf() - Make mem_cgroup_transfer_charge() generic (not dma-buf specific) - Add EXPORT_SYMBOL_GPL for reserve/cancel (system_heap is tristate) - Reorder: move xfer ioctl before LSM patches - Serialize dmabuf->memcg updates with the dma-buf resv lock instead of cmpxchg - Link to v3: https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-0-78d...
Changes in v3: - Split mem_cgroup_charge_dmabuf() into reserve/commit/cancel three-phase API - Extract mem_cgroup_get_from_fd() helper shared by allocate and xfer paths - Rename __padding to flags in struct dma_buf_transfer_charge - Fix UAF race: move mod_memcg_state() before cmpxchg in transfer - Add memcg_account opt-in flag; xfer returns -EOPNOTSUPP if unset - Wrap dma_buf_charge_cgroup hook in #ifdef CONFIG_CGROUPS - Fix TOCTOU in SELinux charge_cgroup xattr length handling - Link to v2: https://lore.kernel.org/r/20261021-v2_20230123_tjmercier_google_com-v2-0-ef6...
Changes in v2: - Move charge_pid_fd to charge_fd, accepting cgroupfd in addition to pidfd per Christian König. - New DMA_BUF_IOCTL_XFER_CHARGE ioctl for post-allocation charge transfer on any dma-buf, not just dma-heap buffers per Christian König and T.J. Mercier. - Moved __GFP_ACCOUNT removal to patch 1 per Barry Song. - dma-buf charge/uncharge helpers take struct dma_buf * directly to avoid the need for caller to manage size calculations. - Added test_xfer_charge_cgroupfd_self to dmabuf-heap selftests. - Added LSM and SELinux hooks for cgroupfd-based charging. - Rebased to v7.3-rc7. - Link to v1: https://lore.kernel.org/r/20260512-v2_20230123_tjmercier_google_com-v1-0-632...
--- Albert Esteve (7): dma-heap: charge dma-buf memory via explicit memcg dma-buf: transfer memcg charges via DMA_BUF_IOCTL_XFER_CHARGE 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 selinux: Restrict dma-buf direct cgroup charging selftests/dmabuf-heaps: Add dma-buf memcg accounting tests
T.J. Mercier (1): memcg: Add MEMCG_DMABUF accounting infrastructure
Documentation/admin-guide/cgroup-v2.rst | 5 + drivers/dma-buf/dma-buf.c | 50 ++++ drivers/dma-buf/dma-heap.c | 28 +- drivers/dma-buf/heaps/cma_heap.c | 3 +- drivers/dma-buf/heaps/system_heap.c | 21 +- drivers/tee/tee_heap.c | 3 +- include/linux/dma-buf.h | 9 + include/linux/dma-heap.h | 3 +- include/linux/lsm_hook_defs.h | 4 + include/linux/memcontrol.h | 47 ++++ include/linux/security.h | 18 ++ include/uapi/linux/dma-buf.h | 26 ++ include/uapi/linux/dma-heap.h | 7 + mm/memcontrol.c | 176 +++++++++++++ security/security.c | 34 +++ security/selinux/hooks.c | 44 ++++ security/selinux/include/classmap.h | 1 + tools/testing/selftests/cgroup/Makefile | 2 +- tools/testing/selftests/cgroup/test_memcontrol.c | 290 ++++++++++++++++++++- tools/testing/selftests/dmabuf-heaps/config | 1 + tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 227 +++++++++++++++- tools/testing/selftests/dmabuf-heaps/vmtest.sh | 226 ++++++++++++++++ 22 files changed, 1210 insertions(+), 15 deletions(-) --- base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907 change-id: 20260508-v2_20230123_tjmercier_google_com-f44fcfb16530
Best regards,
From: "T.J. Mercier" tjmercier@google.com
Introduce a MEMCG_DMABUF stat counter and helpers for charging, cancelling, and uncharging dma-buf memory against a memory cgroup.
The charge is tracked on struct dma_buf via a new memcg pointer. The memcg reference is stored into (or cleared from) dmabuf->memcg by the helpers, removing the need for callers to manage those details. The release path in dma_buf_release() is reduced to a single mem_cgroup_uncharge_dmabuf() call.
Rather than charging at the page level via __GFP_ACCOUNT, which would require exporters to expose their backing pages for cross-cgroup transfer via new dma_buf_ops, charges are tracked externally on struct dma_buf through a single accounting path.
The next patch wires up the charging policy in the dma-heap ioctl.
Signed-off-by: T.J. Mercier tjmercier@google.com Signed-off-by: Albert Esteve aesteve@redhat.com --- Documentation/admin-guide/cgroup-v2.rst | 4 ++ drivers/dma-buf/dma-buf.c | 3 ++ include/linux/dma-buf.h | 4 ++ include/linux/memcontrol.h | 27 +++++++++++++ mm/memcontrol.c | 70 +++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index 86a2a0099178..a3265a4a953e 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1662,6 +1662,10 @@ The following nested keys are defined. Amount of memory used for storing in-kernel data structures.
+ dmabuf (npn) + Amount of memory used for exported DMA buffers allocated by the cgroup. + Stays with the allocating cgroup regardless of how the buffer is shared. + workingset_refault_anon Number of refaults of previously evicted anonymous pages.
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 4c9add51f9ef..1c6b9cee7ac2 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -14,6 +14,7 @@ #include <linux/fs.h> #include <linux/slab.h> #include <linux/dma-buf.h> +#include <linux/memcontrol.h> #include <linux/dma-fence.h> #include <linux/dma-fence-unwrap.h> #include <linux/anon_inodes.h> @@ -182,6 +183,8 @@ static void dma_buf_release(struct dentry *dentry)
dmabuf->ops->release(dmabuf);
+ mem_cgroup_uncharge_dmabuf(dmabuf); + if (dmabuf->resv == (struct dma_resv *)&dmabuf[1]) dma_resv_fini(dmabuf->resv);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index d15b2b31d3c9..23ec0ff23fba 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -27,6 +27,7 @@ struct device; struct dma_buf; struct dma_buf_attachment; +struct mem_cgroup;
/** * struct dma_buf_ops - operations possible on struct dma_buf @@ -429,6 +430,9 @@ struct dma_buf {
__poll_t active; } cb_in, cb_out; + + /** @memcg: the cgroup to which this buffer is currently attributed */ + struct mem_cgroup *memcg; };
/** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 7d1c0ce189a8..4d1e82cf0833 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -10,6 +10,9 @@
#ifndef _LINUX_MEMCONTROL_H #define _LINUX_MEMCONTROL_H + +struct dma_buf; + #include <linux/cgroup.h> #include <linux/vm_event_item.h> #include <linux/hardirq.h> @@ -40,6 +43,7 @@ enum memcg_stat_item { MEMCG_ZSWAP_B, MEMCG_ZSWAPPED, MEMCG_ZSWAP_INCOMP, + MEMCG_DMABUF, MEMCG_NR_STAT, };
@@ -667,6 +671,9 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp);
+bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, + unsigned int nr_pages); + void __mem_cgroup_uncharge(struct folio *folio);
/** @@ -682,6 +689,10 @@ static inline void mem_cgroup_uncharge(struct folio *folio) __mem_cgroup_uncharge(folio); }
+void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages); + +void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf); + void __mem_cgroup_uncharge_folios(struct folio_batch *folios); static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios) { @@ -1167,10 +1178,26 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; }
+static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, + gfp_t gfp, + unsigned int nr_pages) +{ + return true; +} + +static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, + unsigned int nr_pages) +{ +} + static inline void mem_cgroup_uncharge(struct folio *folio) { }
+static inline void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf) +{ +} + static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios) { } diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 856a7d07586c..e53dd7eebc3b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -69,6 +69,7 @@ #include <net/ip.h> #include "slab.h" #include "memcontrol-v1.h" +#include <linux/dma-buf.h>
#include <linux/uaccess.h>
@@ -442,6 +443,7 @@ static const unsigned int memcg_stat_items[] = { MEMCG_ZSWAP_B, MEMCG_ZSWAPPED, MEMCG_ZSWAP_INCOMP, + MEMCG_DMABUF, };
#define NR_MEMCG_NODE_STAT_ITEMS ARRAY_SIZE(memcg_node_stat_items) @@ -1625,6 +1627,7 @@ static const struct memory_stat memory_stats[] = { #ifdef CONFIG_HUGETLB_PAGE { "hugetlb", NR_HUGETLB }, #endif + { "dmabuf", MEMCG_DMABUF },
/* The memory events */ { "workingset_refault_anon", WORKINGSET_REFAULT_ANON }, @@ -5658,6 +5661,73 @@ void mem_cgroup_flush_workqueue(void) flush_workqueue(memcg_wq); }
+/** + * mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf allocation + * @memcg: memcg to charge (may be %NULL) + * @gfp_mask: reclaim mode + * @nr_pages: number of pages to charge + * + * Charges @nr_pages against @memcg. Call this before allocating pages so + * the cgroup limit is enforced up front. On success the caller must + * finalize the charge with the MEMCG_DMABUF stat update, or call + * mem_cgroup_cancel_dmabuf() to undo it. + * + * Returns %true on success, %false if the charge would exceed the limit. + */ +bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp_mask, + unsigned int nr_pages) +{ + if (!memcg || mem_cgroup_disabled()) + return true; + + if (try_charge(memcg, gfp_mask, nr_pages)) + return false; + + return true; +} +EXPORT_SYMBOL_GPL(mem_cgroup_reserve_dmabuf); + +/** + * mem_cgroup_cancel_dmabuf - undo a dma-buf charge + * @memcg: memcg to uncharge (may be %NULL) + * @nr_pages: same value passed to mem_cgroup_reserve_dmabuf() + * + * Returns @nr_pages of charge to @memcg. Use when the allocation fails + * after mem_cgroup_reserve_dmabuf(). + */ +void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) +{ + if (!memcg) + return; + + if (!mem_cgroup_is_root(memcg)) + refill_stock(memcg, nr_pages); +} +EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf); + +/** + * mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg + * @dmabuf: buffer to uncharge + * + * Reverses a finalized dma-buf charge. Drops the memcg + * reference stored in @dmabuf->memcg and clears the pointer. + * Safe to call when @dmabuf->memcg is %NULL. + */ +void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf) +{ + unsigned int nr_pages; + struct mem_cgroup *memcg = dmabuf->memcg; + + if (!memcg) + return; + + nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT; + mod_memcg_state(memcg, MEMCG_DMABUF, -nr_pages); + mem_cgroup_cancel_dmabuf(memcg, nr_pages); + mem_cgroup_put(memcg); + dmabuf->memcg = NULL; +} + static int __init cgroup_memory(char *s) { char *token;
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] mm: memcontrol: 32-bit nr_pages truncation in dmabuf charge helpers --
commit a5e6165b3e0444a33f57a14736c2174d7c009944 Author: T.J. Mercier tjmercier@google.com
memcg: Add MEMCG_DMABUF accounting infrastructure
This commit introduces the MEMCG_DMABUF stat counter and helper functions to reserve, cancel, and uncharge dma-buf memory against a memory cgroup, tracking the active charge via a memcg pointer on struct dma_buf.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 856a7d07586cc..e53dd7eebc3b5 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c
[ ... ]
@@ -5658,6 +5661,73 @@ void mem_cgroup_flush_workqueue(void) flush_workqueue(memcg_wq); } +/**
- mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf allocation
- @memcg: memcg to charge (may be %NULL)
- @gfp_mask: reclaim mode
- @nr_pages: number of pages to charge
- Charges @nr_pages against @memcg. Call this before allocating pages so
- the cgroup limit is enforced up front. On success the caller must
- finalize the charge with the MEMCG_DMABUF stat update, or call
- mem_cgroup_cancel_dmabuf() to undo it.
- Returns %true on success, %false if the charge would exceed the limit.
- */
+bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp_mask,
unsigned int nr_pages)+{
- if (!memcg || mem_cgroup_disabled())
return true;- if (try_charge(memcg, gfp_mask, nr_pages))
return false;- return true;
+} +EXPORT_SYMBOL_GPL(mem_cgroup_reserve_dmabuf);
+/**
- mem_cgroup_cancel_dmabuf - undo a dma-buf charge
- @memcg: memcg to uncharge (may be %NULL)
- @nr_pages: same value passed to mem_cgroup_reserve_dmabuf()
- Returns @nr_pages of charge to @memcg. Use when the allocation fails
- after mem_cgroup_reserve_dmabuf().
- */
+void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) +{
- if (!memcg)
return;- if (!mem_cgroup_is_root(memcg))
refill_stock(memcg, nr_pages);+} +EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf);
+/**
- mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
- @dmabuf: buffer to uncharge
- Reverses a finalized dma-buf charge. Drops the memcg
- reference stored in @dmabuf->memcg and clears the pointer.
- Safe to call when @dmabuf->memcg is %NULL.
- */
+void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf) +{
- unsigned int nr_pages;
- struct mem_cgroup *memcg = dmabuf->memcg;
- if (!memcg)
return;- nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
- mod_memcg_state(memcg, MEMCG_DMABUF, -nr_pages);
- mem_cgroup_cancel_dmabuf(memcg, nr_pages);
- mem_cgroup_put(memcg);
- dmabuf->memcg = NULL;
+}
[Severity: Medium] Can passing a 32-bit unsigned int for nr_pages in mem_cgroup_reserve_dmabuf(), mem_cgroup_cancel_dmabuf(), and mem_cgroup_uncharge_dmabuf() in mm/memcontrol.c (and include/linux/memcontrol.h) truncate 64-bit dma-buf sizes?
struct dma_buf stores size as a 64-bit size_t, and in the following commit ("dma-heap: charge dma-buf memory via explicit memcg"), dma_heap_ioctl_allocate() -> system_heap_allocate() passes a 64-bit PAGE_ALIGN(len) >> PAGE_SHIFT to mem_cgroup_reserve_dmabuf() before entering its while (size_remaining > 0) page allocation loop without __GFP_ACCOUNT.
If a userspace caller requests an allocation where len >= (1ULL << (32 + PAGE_SHIFT)) (for example, (1ULL << 44) + PAGE_SIZE on a 4 KiB page system), PAGE_ALIGN(len) >> PAGE_SHIFT wraps to 1 page when converted to unsigned int nr_pages. try_charge() will then succeed against a small memory.max limit while system_heap_allocate() proceeds to allocate unaccounted pages using the full 64-bit size_remaining.
Additionally, in mem_cgroup_uncharge_dmabuf(), passing -nr_pages to mod_memcg_state(memcg, MEMCG_DMABUF, -nr_pages) (which takes a signed int val) will overflow when nr_pages exceeds INT_MAX. Should nr_pages use unsigned long (or size_t) across these helpers?
On Wed, 2026-11-04 at 12:09 +0100, Albert Esteve wrote:
From: "T.J. Mercier" tjmercier@google.com
Introduce a MEMCG_DMABUF stat counter and helpers for charging, cancelling, and uncharging dma-buf memory against a memory cgroup.
The charge is tracked on struct dma_buf via a new memcg pointer. The memcg reference is stored into (or cleared from) dmabuf->memcg by the helpers, removing the need for callers to manage those details. The release path in dma_buf_release() is reduced to a single mem_cgroup_uncharge_dmabuf() call.
Rather than charging at the page level via __GFP_ACCOUNT, which would require exporters to expose their backing pages for cross-cgroup transfer via new dma_buf_ops, charges are tracked externally on struct dma_buf through a single accounting path.
The next patch wires up the charging policy in the dma-heap ioctl.
Signed-off-by: T.J. Mercier tjmercier@google.com Signed-off-by: Albert Esteve aesteve@redhat.com
Documentation/admin-guide/cgroup-v2.rst | 4 ++ drivers/dma-buf/dma-buf.c | 3 ++ include/linux/dma-buf.h | 4 ++ include/linux/memcontrol.h | 27 +++++++++++++ mm/memcontrol.c | 70 +++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index 86a2a0099178..a3265a4a953e 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1662,6 +1662,10 @@ The following nested keys are defined. Amount of memory used for storing in-kernel data structures.
- dmabuf (npn)
Amount of memory used for exported DMA buffersallocated by the cgroup.
Stays with the allocating cgroup regardless of howthe buffer is shared.
workingset_refault_anon Number of refaults of previously evicted anonymous pages. diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 4c9add51f9ef..1c6b9cee7ac2 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -14,6 +14,7 @@ #include <linux/fs.h> #include <linux/slab.h> #include <linux/dma-buf.h> +#include <linux/memcontrol.h> #include <linux/dma-fence.h> #include <linux/dma-fence-unwrap.h> #include <linux/anon_inodes.h> @@ -182,6 +183,8 @@ static void dma_buf_release(struct dentry *dentry) dmabuf->ops->release(dmabuf);
- mem_cgroup_uncharge_dmabuf(dmabuf);
if (dmabuf->resv == (struct dma_resv *)&dmabuf[1]) dma_resv_fini(dmabuf->resv); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index d15b2b31d3c9..23ec0ff23fba 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -27,6 +27,7 @@ struct device; struct dma_buf; struct dma_buf_attachment; +struct mem_cgroup; /** * struct dma_buf_ops - operations possible on struct dma_buf @@ -429,6 +430,9 @@ struct dma_buf { __poll_t active; } cb_in, cb_out;
- /** @memcg: the cgroup to which this buffer is currently
attributed */
- struct mem_cgroup *memcg;
}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 7d1c0ce189a8..4d1e82cf0833 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -10,6 +10,9 @@ #ifndef _LINUX_MEMCONTROL_H #define _LINUX_MEMCONTROL_H
+struct dma_buf;
#include <linux/cgroup.h> #include <linux/vm_event_item.h> #include <linux/hardirq.h> @@ -40,6 +43,7 @@ enum memcg_stat_item { MEMCG_ZSWAP_B, MEMCG_ZSWAPPED, MEMCG_ZSWAP_INCOMP,
- MEMCG_DMABUF,
MEMCG_NR_STAT, }; @@ -667,6 +671,9 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp); +bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp,
unsigned int nr_pages);void __mem_cgroup_uncharge(struct folio *folio); /** @@ -682,6 +689,10 @@ static inline void mem_cgroup_uncharge(struct folio *folio) __mem_cgroup_uncharge(folio); } +void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages);
+void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf);
void __mem_cgroup_uncharge_folios(struct folio_batch *folios); static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios) { @@ -1167,10 +1178,26 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; } +static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg,
gfp_t gfp,unsigned int nr_pages)+{
- return true;
+}
+static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg,
unsigned int nr_pages)+{ +}
static inline void mem_cgroup_uncharge(struct folio *folio) { } +static inline void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf) +{ +}
static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios) { } diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 856a7d07586c..e53dd7eebc3b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -69,6 +69,7 @@ #include <net/ip.h> #include "slab.h" #include "memcontrol-v1.h" +#include <linux/dma-buf.h> #include <linux/uaccess.h> @@ -442,6 +443,7 @@ static const unsigned int memcg_stat_items[] = { MEMCG_ZSWAP_B, MEMCG_ZSWAPPED, MEMCG_ZSWAP_INCOMP,
- MEMCG_DMABUF,
}; #define NR_MEMCG_NODE_STAT_ITEMS ARRAY_SIZE(memcg_node_stat_items) @@ -1625,6 +1627,7 @@ static const struct memory_stat memory_stats[] = { #ifdef CONFIG_HUGETLB_PAGE { "hugetlb", NR_HUGETLB }, #endif
- {
"dmabuf", MEMCG_DMABUF }, /* The memory events */ { "workingset_refault_anon", WORKINGSET_REFAULT_ANON }, @@ -5658,6 +5661,73 @@ void mem_cgroup_flush_workqueue(void) flush_workqueue(memcg_wq); } +/**
- mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf
allocation
- @memcg: memcg to charge (may be %NULL)
- @gfp_mask: reclaim mode
- @nr_pages: number of pages to charge
- Charges @nr_pages against @memcg. Call this before allocating
pages so
- the cgroup limit is enforced up front. On success the caller must
- finalize the charge with the MEMCG_DMABUF stat update, or call
- mem_cgroup_cancel_dmabuf() to undo it.
- Returns %true on success, %false if the charge would exceed the
limit.
- */
+bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp_mask,
unsigned int nr_pages)+{
- if (!memcg || mem_cgroup_disabled())
return true;- if (try_charge(memcg, gfp_mask, nr_pages))
return false;- return true;
+} +EXPORT_SYMBOL_GPL(mem_cgroup_reserve_dmabuf);
+/**
- mem_cgroup_cancel_dmabuf - undo a dma-buf charge
- @memcg: memcg to uncharge (may be %NULL)
- @nr_pages: same value passed to mem_cgroup_reserve_dmabuf()
- Returns @nr_pages of charge to @memcg. Use when the allocation
fails
- after mem_cgroup_reserve_dmabuf().
- */
+void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) +{
- if (!memcg)
return;- if (!mem_cgroup_is_root(memcg))
refill_stock(memcg, nr_pages);+} +EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf);
+/**
- mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
- @dmabuf: buffer to uncharge
- Reverses a finalized dma-buf charge. Drops the memcg
- reference stored in @dmabuf->memcg and clears the pointer.
- Safe to call when @dmabuf->memcg is %NULL.
- */
+void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf) +{
- unsigned int nr_pages;
- struct mem_cgroup *memcg = dmabuf->memcg;
- if (!memcg)
return;- nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
- mod_memcg_state(memcg, MEMCG_DMABUF, -nr_pages);
- mem_cgroup_cancel_dmabuf(memcg, nr_pages);
- mem_cgroup_put(memcg);
- dmabuf->memcg = NULL;
+}
Instead of keeping a reference on the memcg for the buffer's whole lifetime, shouldn't an obj_cgroup be used, which is reparented when the memcg goes offline? Like in Dave's series here:
https://lore.kernel.org/all/20260706052330.1110909-1-airlied@gmail.com/
Thanks, Thomas
static int __init cgroup_memory(char *s) { char *token;
On Thu, Nov 12, 2026 at 08:26:35AM +0100, Thomas Hellström wrote:
On Wed, 2026-11-04 at 12:09 +0100, Albert Esteve wrote:
From: "T.J. Mercier" tjmercier@google.com
Introduce a MEMCG_DMABUF stat counter and helpers for charging, cancelling, and uncharging dma-buf memory against a memory cgroup.
The charge is tracked on struct dma_buf via a new memcg pointer. The memcg reference is stored into (or cleared from) dmabuf->memcg by the helpers, removing the need for callers to manage those details. The release path in dma_buf_release() is reduced to a single mem_cgroup_uncharge_dmabuf() call.
Rather than charging at the page level via __GFP_ACCOUNT, which would require exporters to expose their backing pages for cross-cgroup transfer via new dma_buf_ops, charges are tracked externally on struct dma_buf through a single accounting path.
The next patch wires up the charging policy in the dma-heap ioctl.
Signed-off-by: T.J. Mercier tjmercier@google.com Signed-off-by: Albert Esteve aesteve@redhat.com
Documentation/admin-guide/cgroup-v2.rst | 4 ++ drivers/dma-buf/dma-buf.c | 3 ++ include/linux/dma-buf.h | 4 ++ include/linux/memcontrol.h | 27 +++++++++++++ mm/memcontrol.c | 70 +++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index 86a2a0099178..a3265a4a953e 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1662,6 +1662,10 @@ The following nested keys are defined. Amount of memory used for storing in-kernel data structures.
- dmabuf (npn)
Amount of memory used for exported DMA buffersallocated by the cgroup.
Stays with the allocating cgroup regardless of howthe buffer is shared.
workingset_refault_anon Number of refaults of previously evicted anonymous pages. diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 4c9add51f9ef..1c6b9cee7ac2 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -14,6 +14,7 @@ #include <linux/fs.h> #include <linux/slab.h> #include <linux/dma-buf.h> +#include <linux/memcontrol.h> #include <linux/dma-fence.h> #include <linux/dma-fence-unwrap.h> #include <linux/anon_inodes.h> @@ -182,6 +183,8 @@ static void dma_buf_release(struct dentry *dentry) dmabuf->ops->release(dmabuf);
- mem_cgroup_uncharge_dmabuf(dmabuf);
if (dmabuf->resv == (struct dma_resv *)&dmabuf[1]) dma_resv_fini(dmabuf->resv); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index d15b2b31d3c9..23ec0ff23fba 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -27,6 +27,7 @@ struct device; struct dma_buf; struct dma_buf_attachment; +struct mem_cgroup; /** * struct dma_buf_ops - operations possible on struct dma_buf @@ -429,6 +430,9 @@ struct dma_buf { __poll_t active; } cb_in, cb_out;
- /** @memcg: the cgroup to which this buffer is currently
attributed */
- struct mem_cgroup *memcg;
}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 7d1c0ce189a8..4d1e82cf0833 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -10,6 +10,9 @@ #ifndef _LINUX_MEMCONTROL_H #define _LINUX_MEMCONTROL_H
+struct dma_buf;
#include <linux/cgroup.h> #include <linux/vm_event_item.h> #include <linux/hardirq.h> @@ -40,6 +43,7 @@ enum memcg_stat_item { MEMCG_ZSWAP_B, MEMCG_ZSWAPPED, MEMCG_ZSWAP_INCOMP,
- MEMCG_DMABUF,
MEMCG_NR_STAT, }; @@ -667,6 +671,9 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp); +bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp,
unsigned int nr_pages);void __mem_cgroup_uncharge(struct folio *folio); /** @@ -682,6 +689,10 @@ static inline void mem_cgroup_uncharge(struct folio *folio) __mem_cgroup_uncharge(folio); } +void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages);
+void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf);
void __mem_cgroup_uncharge_folios(struct folio_batch *folios); static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios) { @@ -1167,10 +1178,26 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; } +static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg,
gfp_t gfp,unsigned int nr_pages)+{
- return true;
+}
+static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg,
unsigned int nr_pages)+{ +}
static inline void mem_cgroup_uncharge(struct folio *folio) { } +static inline void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf) +{ +}
static inline void mem_cgroup_uncharge_folios(struct folio_batch *folios) { } diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 856a7d07586c..e53dd7eebc3b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -69,6 +69,7 @@ #include <net/ip.h> #include "slab.h" #include "memcontrol-v1.h" +#include <linux/dma-buf.h> #include <linux/uaccess.h> @@ -442,6 +443,7 @@ static const unsigned int memcg_stat_items[] = { MEMCG_ZSWAP_B, MEMCG_ZSWAPPED, MEMCG_ZSWAP_INCOMP,
- MEMCG_DMABUF,
}; #define NR_MEMCG_NODE_STAT_ITEMS ARRAY_SIZE(memcg_node_stat_items) @@ -1625,6 +1627,7 @@ static const struct memory_stat memory_stats[] = { #ifdef CONFIG_HUGETLB_PAGE { "hugetlb", NR_HUGETLB }, #endif
- {
"dmabuf", MEMCG_DMABUF }, /* The memory events */ { "workingset_refault_anon", WORKINGSET_REFAULT_ANON }, @@ -5658,6 +5661,73 @@ void mem_cgroup_flush_workqueue(void) flush_workqueue(memcg_wq); } +/**
- mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf
allocation
- @memcg: memcg to charge (may be %NULL)
- @gfp_mask: reclaim mode
- @nr_pages: number of pages to charge
- Charges @nr_pages against @memcg. Call this before allocating
pages so
- the cgroup limit is enforced up front. On success the caller must
- finalize the charge with the MEMCG_DMABUF stat update, or call
- mem_cgroup_cancel_dmabuf() to undo it.
- Returns %true on success, %false if the charge would exceed the
limit.
- */
+bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp_mask,
unsigned int nr_pages)+{
- if (!memcg || mem_cgroup_disabled())
return true;- if (try_charge(memcg, gfp_mask, nr_pages))
return false;- return true;
+} +EXPORT_SYMBOL_GPL(mem_cgroup_reserve_dmabuf);
+/**
- mem_cgroup_cancel_dmabuf - undo a dma-buf charge
- @memcg: memcg to uncharge (may be %NULL)
- @nr_pages: same value passed to mem_cgroup_reserve_dmabuf()
- Returns @nr_pages of charge to @memcg. Use when the allocation
fails
- after mem_cgroup_reserve_dmabuf().
- */
+void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) +{
- if (!memcg)
return;- if (!mem_cgroup_is_root(memcg))
refill_stock(memcg, nr_pages);+} +EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf);
+/**
- mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
- @dmabuf: buffer to uncharge
- Reverses a finalized dma-buf charge. Drops the memcg
- reference stored in @dmabuf->memcg and clears the pointer.
- Safe to call when @dmabuf->memcg is %NULL.
- */
+void mem_cgroup_uncharge_dmabuf(struct dma_buf *dmabuf) +{
- unsigned int nr_pages;
- struct mem_cgroup *memcg = dmabuf->memcg;
- if (!memcg)
return;- nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
- mod_memcg_state(memcg, MEMCG_DMABUF, -nr_pages);
- mem_cgroup_cancel_dmabuf(memcg, nr_pages);
- mem_cgroup_put(memcg);
- dmabuf->memcg = NULL;
+}
Instead of keeping a reference on the memcg for the buffer's whole lifetime, shouldn't an obj_cgroup be used, which is reparented when the memcg goes offline?
Good point. I did not think of it, but after a brief check I think it'd be the right object to use. I'll switch for the next version.
Like in Dave's series here:
https://lore.kernel.org/all/20260706052330.1110909-1-airlied@gmail.com/
Thanks for the link! I'll use it as reference. I should mention it in the cover letter too probably, as that series is related (should be safe as far as it does not opt-in for dma-buf-level charging).
BR, Albert
Thanks, Thomas
static int __init cgroup_memory(char *s) { char *token;
On embedded platforms a central process often allocates dma-buf memory on behalf of client applications. Without a way to attribute the charge to the requesting client's cgroup, the cost lands on the allocator, making per-cgroup memory limits ineffective for the actual consumers.
Add charge_fd to struct dma_heap_allocation_data. When set to a valid pidfd or cgroupfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's or cgroup's memcg and charges the buffer there via mem_cgroup_reserve_dmabuf() inside dma_heap_ioctl_allocate(). The resolved memcg is passed through dma_buf_export_info so that dma_buf_export() stores it and updates the MEMCG_DMABUF stat. Without charge_fd, and with the mem_accounting module parameter enabled, the buffer is charged to the allocator's own cgroup. The __GPF_ACCOUNT flag is removed from system heap allocations to avoid double counting.
Usage examples:
1. Central allocator charging to a client at allocation time. The allocator knows the client's PID (e.g., from binder's sender_pid) and uses pidfd to attribute the charge:
pid_t client_pid = txn->sender_pid; int pidfd = pidfd_open(client_pid, 0);
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, .charge_fd = pidfd, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); close(pidfd); /* alloc.fd is now charged to client's cgroup */
2. Default allocation (no pidfd, mem_accounting=1). When charge_fd is not set and the mem_accounting module parameter is enabled, the buffer is charged to the allocator's own cgroup:
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); /* charged to current process's cgroup */
Current limitations:
- Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. Means only the first owner (and exporter) of the shared buffer bears the charge. - Only memcg accounting supported. While this makes sense for system heap buffers, other heaps (e.g., CMA heaps) will require selectively charging also for the dmem controller.
Signed-off-by: Albert Esteve aesteve@redhat.com --- Documentation/admin-guide/cgroup-v2.rst | 5 ++-- drivers/dma-buf/dma-buf.c | 5 ++++ drivers/dma-buf/dma-heap.c | 28 +++++++++++++++++--- drivers/dma-buf/heaps/cma_heap.c | 3 ++- drivers/dma-buf/heaps/system_heap.c | 21 +++++++++++---- drivers/tee/tee_heap.c | 3 ++- include/linux/dma-buf.h | 4 +++ include/linux/dma-heap.h | 3 ++- include/linux/memcontrol.h | 7 +++++ include/uapi/linux/dma-heap.h | 7 +++++ mm/memcontrol.c | 47 +++++++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index a3265a4a953e..66ddba678784 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1663,8 +1663,9 @@ The following nested keys are defined. structures.
dmabuf (npn) - Amount of memory used for exported DMA buffers allocated by the cgroup. - Stays with the allocating cgroup regardless of how the buffer is shared. + Amount of memory used for exported DMA buffers allocated by or on + behalf of the cgroup. Stays with the allocating cgroup regardless + of how the buffer is shared.
workingset_refault_anon Number of refaults of previously evicted anonymous pages. diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 1c6b9cee7ac2..837666273550 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -714,6 +714,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) struct dma_resv *resv = exp_info->resv; struct file *file; size_t alloc_size = sizeof(struct dma_buf); + unsigned int nr_pages = PAGE_ALIGN(exp_info->size) >> PAGE_SHIFT; int ret;
if (WARN_ON(!exp_info->priv || !exp_info->ops @@ -750,6 +751,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 = exp_info->memcg; spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll; @@ -771,6 +773,9 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
+ if (dmabuf->memcg) + mod_memcg_state(dmabuf->memcg, MEMCG_DMABUF, nr_pages); + return dmabuf;
err_file: diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 8a6c2b6e0cc6..7bd416ae2c23 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -10,6 +10,8 @@ #include <linux/device.h> #include <linux/dma-buf.h> #include <linux/dma-heap.h> +#include <linux/memcontrol.h> +#include <linux/sched/mm.h> #include <linux/err.h> #include <linux/export.h> #include <linux/list.h> @@ -78,6 +80,7 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) struct dma_heap_allocation_data *heap_allocation = data; struct dma_heap *heap = file->private_data; struct dma_buf *dmabuf; + struct mem_cgroup *memcg = NULL; int fd; size_t len;
@@ -90,15 +93,34 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS) return ERR_PTR(-EINVAL);
+ if (heap_allocation->__padding != 0) + return ERR_PTR(-EINVAL); + + if (heap_allocation->charge_fd) { + memcg = mem_cgroup_get_from_fd(heap_allocation->charge_fd); + if (IS_ERR(memcg)) + return ERR_CAST(memcg); + } + + if (!memcg && mem_accounting) + memcg = get_mem_cgroup_from_mm(current->mm); + len = PAGE_ALIGN(heap_allocation->len); - if (!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); + heap_allocation->heap_flags, memcg);
- if (IS_ERR(dmabuf)) + if (IS_ERR(dmabuf)) { + mem_cgroup_put(memcg); return dmabuf; + } + + if (memcg && !dmabuf->memcg) + mem_cgroup_put(memcg);
fd = get_unused_fd_flags(heap_allocation->fd_flags); if (fd < 0) { diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 3fb4b946c91a..04de88b468a8 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -281,7 +281,8 @@ static const struct dma_buf_ops cma_heap_buf_ops = { static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, unsigned long len, u32 fd_flags, - u64 heap_flags) + u64 heap_flags, + struct mem_cgroup *memcg) { struct cma_heap *cma_heap = dma_heap_get_drvdata(heap); struct cma_heap_buffer *buffer; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index c8959eadc71d..2bd245e1eab8 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; 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; @@ -473,11 +479,14 @@ 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; + if (memcg) + exp_info.memcg = memcg; dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf); goto free_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);
return ERR_PTR(ret); } diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c index 007c00156520..83e2fbd6960b 100644 --- a/drivers/tee/tee_heap.c +++ b/drivers/tee/tee_heap.c @@ -167,7 +167,8 @@ static const struct dma_buf_ops tee_heap_buf_ops = {
static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap, unsigned long len, u32 fd_flags, - u64 heap_flags) + u64 heap_flags, + struct mem_cgroup *memcg) { struct tee_dma_heap *h = dma_heap_get_drvdata(heap); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..85dd747c9b36 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -505,6 +505,9 @@ 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: memcg to charge for this buffer's memory. Set only for + * buffers backed by system memory where @size reflects the + * actual allocation in bytes. * * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. @@ -517,6 +520,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv; + struct mem_cgroup *memcg; };
/** diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h index 648328a64b27..2d69f8432bd2 100644 --- a/include/linux/dma-heap.h +++ b/include/linux/dma-heap.h @@ -23,7 +23,8 @@ struct dma_heap_ops { struct dma_buf *(*allocate)(struct dma_heap *heap, unsigned long len, u32 fd_flags, - u64 heap_flags); + u64 heap_flags, + struct mem_cgroup *memcg); };
/** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 4d1e82cf0833..f8ec751b7d7b 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -671,6 +671,8 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp);
+struct mem_cgroup *mem_cgroup_get_from_fd(int fd); + bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages);
@@ -1178,6 +1180,11 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; }
+static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) +{ + return ERR_PTR(-EOPNOTSUPP); +} + static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages) diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h index a4cf716a49fa..bfcd1c25bc7e 100644 --- a/include/uapi/linux/dma-heap.h +++ b/include/uapi/linux/dma-heap.h @@ -29,6 +29,11 @@ * handle to the allocated dma-buf * @fd_flags: file descriptor flags used when allocating * @heap_flags: flags passed to heap + * @charge_fd: optional pidfd or cgroupfd whose cgroup should be + * charged for this allocation; 0 means charge the calling + * process's cgroup when the mem_accounting module parameter + * is enabled, or skip charging otherwise + * @__padding: reserved, must be zero * * Provided by userspace as an argument to the ioctl */ @@ -37,6 +42,8 @@ struct dma_heap_allocation_data { __u32 fd; __u32 fd_flags; __u64 heap_flags; + __u32 charge_fd; + __u32 __padding; };
#define DMA_HEAP_IOC_MAGIC 'H' diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e53dd7eebc3b..17fdefc56b97 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 ?: ERR_PTR(-ENOENT); +} + /** * mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf allocation * @memcg: memcg to charge (may be %NULL)
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] mm: memcontrol: missing permission checks in mem_cgroup_get_from_fd() - [High] dma-buf: system_heap: 32-bit nr_pages truncation in system_heap_allocate() --
commit e390a33c5307a75778a81c2ca5beff0530617904 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 that DMA_HEAP_IOCTL_ALLOC can resolve a target pidfd or cgroupfd and charge the exported dma-buf memory to that memcg via mem_cgroup_reserve_dmabuf(). It also removes __GFP_ACCOUNT from system heap page allocations to avoid double counting.
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index c8959eadc71d1..2bd245e1eab8f 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c
[ ... ]
@@ -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) page = alloc_pages(flags, orders[i]); if (!page) continue;flags |= __GFP_ACCOUNT;@@ -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; 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);
[Severity: High] As reported by Sashiko on v3 [1], can truncating nr_pages to a 32-bit unsigned int in system_heap_allocate() allow a 64-bit allocation length to bypass memcg memory.max limits?
In dma_heap_ioctl_allocate(), heap_allocation->len (__u64) is page-aligned into a 64-bit size_t len and only checked for wrap-around to zero before being passed as an unsigned long len to system_heap_allocate():
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 retains the full 64-bit len while nr_pages is truncated to unsigned int. Because __GFP_ACCOUNT is removed from alloc_largest_available(), mem_cgroup_reserve_dmabuf() becomes the only memcg limit check.
On a 64-bit system with 4 KiB pages (PAGE_SHIFT == 12), if a user passes len = (1ULL << 44) (or (1ULL << 44) + PAGE_SIZE), PAGE_ALIGN(len) >> PAGE_SHIFT truncates in 32-bit unsigned int nr_pages to 0 (or 1). mem_cgroup_reserve_dmabuf() then charges 0 (or 1) pages and succeeds inside a restricted cgroup, while the while (size_remaining > 0) loop allocates uncharged physical pages for the full 64-bit size_remaining until host RAM is exhausted and the global OOM killer is triggered.
Similarly, in dma_buf_export() and mem_cgroup_uncharge_dmabuf(), PAGE_ALIGN(size) >> PAGE_SHIFT is stored in an unsigned int nr_pages and passed to mod_memcg_state(), which takes a signed int val and can overflow to a negative delta when bit 31 of nr_pages is set. Should nr_pages be unsigned long (or should len be bounds-checked before converting to page counts)?
[ ... ]
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e53dd7eebc3b5..17fdefc56b97f 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 ?: ERR_PTR(-ENOENT);
+}
[Severity: Critical] As reported by Sashiko on v3 [2], does mem_cgroup_get_from_fd() allow an unprivileged caller to charge dma-buf allocations to arbitrary cgroups without permission checks?
When dma_heap_ioctl_allocate() calls mem_cgroup_get_from_fd() with a user-supplied charge_fd, the target mem_cgroup is resolved either from a cgroup v2 directory fd (which can be opened with O_RDONLY | O_DIRECTORY or O_PATH on any world-readable /sys/fs/cgroup directory, including the root cgroup) or from a pidfd obtained via pidfd_open() for any visible process such as PID 1.
Neither path verifies DAC permissions, cgroup write access (such as cgroup_may_write() or cgroup_attach_permissions() used by CLONE_INTO_CGROUP in cgroup_css_set_fork()), ptrace_may_access(), or capabilities such as CAP_SYS_ADMIN / CAP_SYS_RESOURCE.
When system_heap_allocate() then calls mem_cgroup_reserve_dmabuf(memcg, GFP_KERNEL, nr_pages) -> try_charge(), targeting root_mem_cgroup returns 0 immediately and bypasses the caller's memory.max and memory.high limits, while targeting another cgroup can trigger reclaim and OOM kills in that victim cgroup. In addition, when a remote target memcg exceeds memory.high during try_charge_memcg(), the over-high penalty and reclaim in __mem_cgroup_handle_over_high() are applied against current->mm's memcg rather than the remotely charged memcg.
Later commits 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 policy checks, but still leave no baseline DAC, capability, cgroup write permission, or ptrace_may_access() checks when CONFIG_SECURITY is disabled, when SELinux is not the active LSM, or when a cgroup v2 directory uses default genfscon labeling without an explicit security.selinux kernfs xattr (where selinux_dma_buf_charge_cgroup() sees kernfs_xattr_get() == -ENODATA and returns 0).
Should baseline permission checks be enforced before allowing a process to charge memory to another task's or cgroup's memcg?
[1] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-2-78d... [2] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-2-78d...
On Wed, 2026-11-04 at 12:09 +0100, Albert Esteve wrote:
On embedded platforms a central process often allocates dma-buf memory on behalf of client applications. Without a way to attribute the charge to the requesting client's cgroup, the cost lands on the allocator, making per-cgroup memory limits ineffective for the actual consumers.
Add charge_fd to struct dma_heap_allocation_data. When set to a valid pidfd or cgroupfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's or cgroup's memcg and charges the buffer there via mem_cgroup_reserve_dmabuf() inside dma_heap_ioctl_allocate(). The resolved memcg is passed through dma_buf_export_info so that dma_buf_export() stores it and updates the MEMCG_DMABUF stat. Without charge_fd, and with the mem_accounting module parameter enabled, the buffer is charged to the allocator's own cgroup. The __GPF_ACCOUNT flag is removed from system heap allocations to avoid double counting.
Usage examples:
1. Central allocator charging to a client at allocation time. The allocator knows the client's PID (e.g., from binder's sender_pid) and uses pidfd to attribute the charge:
pid_t client_pid = txn->sender_pid; int pidfd = pidfd_open(client_pid, 0);
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, .charge_fd = pidfd, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); close(pidfd); /* alloc.fd is now charged to client's cgroup */
2. Default allocation (no pidfd, mem_accounting=1). When charge_fd is not set and the mem_accounting module parameter is enabled, the buffer is charged to the allocator's own cgroup:
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); /* charged to current process's cgroup */
Current limitations:
- Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. Means only the first owner (and exporter) of the shared buffer bears the charge. - Only memcg accounting supported. While this makes sense for system heap buffers, other heaps (e.g., CMA heaps) will require selectively charging also for the dmem controller.
Signed-off-by: Albert Esteve aesteve@redhat.com
Documentation/admin-guide/cgroup-v2.rst | 5 ++-- drivers/dma-buf/dma-buf.c | 5 ++++ drivers/dma-buf/dma-heap.c | 28 +++++++++++++++++--- drivers/dma-buf/heaps/cma_heap.c | 3 ++- drivers/dma-buf/heaps/system_heap.c | 21 +++++++++++---- drivers/tee/tee_heap.c | 3 ++- include/linux/dma-buf.h | 4 +++ include/linux/dma-heap.h | 3 ++- include/linux/memcontrol.h | 7 +++++ include/uapi/linux/dma-heap.h | 7 +++++ mm/memcontrol.c | 47 +++++++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index a3265a4a953e..66ddba678784 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1663,8 +1663,9 @@ The following nested keys are defined. structures. dmabuf (npn)
Amount of memory used for exported DMA buffersallocated by the cgroup.
Stays with the allocating cgroup regardless of howthe buffer is shared.
Amount of memory used for exported DMA buffersallocated by or on
behalf of the cgroup. Stays with the allocatingcgroup regardless
of how the buffer is shared.
This doesn't sound right. Graphics buffers exported as DMA buffers are typically charged elsewhere, like gpu_active, and a transfer of their charge would likely stay in that key? To be consistent, shouldn't this be related to dma-heap memory and called dmaheap or something similar. We should have a tighter definition of exactly what memory is to be registered here.
Also says "Stays with the allocating cgroup regardless of how the buffer is shared", which isn't correct after the series? Perhaps that's changed in a later patch.
Thanks, Thomas
workingset_refault_anon Number of refaults of previously evicted anonymous pages. diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 1c6b9cee7ac2..837666273550 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -714,6 +714,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) struct dma_resv *resv = exp_info->resv; struct file *file; size_t alloc_size = sizeof(struct dma_buf);
- unsigned int nr_pages = PAGE_ALIGN(exp_info->size) >>
PAGE_SHIFT; int ret; if (WARN_ON(!exp_info->priv || !exp_info->ops @@ -750,6 +751,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 = exp_info->memcg;
spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll; @@ -771,6 +773,9 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
- if (dmabuf->memcg)
mod_memcg_state(dmabuf->memcg, MEMCG_DMABUF,nr_pages);
return dmabuf; err_file: diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 8a6c2b6e0cc6..7bd416ae2c23 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -10,6 +10,8 @@ #include <linux/device.h> #include <linux/dma-buf.h> #include <linux/dma-heap.h> +#include <linux/memcontrol.h> +#include <linux/sched/mm.h> #include <linux/err.h> #include <linux/export.h> #include <linux/list.h> @@ -78,6 +80,7 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) struct dma_heap_allocation_data *heap_allocation = data; struct dma_heap *heap = file->private_data; struct dma_buf *dmabuf;
- struct mem_cgroup *memcg = NULL;
int fd; size_t len; @@ -90,15 +93,34 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS) return ERR_PTR(-EINVAL);
- if (heap_allocation->__padding != 0)
return ERR_PTR(-EINVAL);- if (heap_allocation->charge_fd) {
memcg = mem_cgroup_get_from_fd(heap_allocation-charge_fd);
if (IS_ERR(memcg))return ERR_CAST(memcg);- }
- if (!memcg && mem_accounting)
memcg = get_mem_cgroup_from_mm(current->mm);len = PAGE_ALIGN(heap_allocation->len);
- if (!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);
heap_allocation->heap_flags,memcg);
- if (IS_ERR(dmabuf))
- if (IS_ERR(dmabuf)) {
mem_cgroup_put(memcg);return dmabuf;
- }
- if (memcg && !dmabuf->memcg)
mem_cgroup_put(memcg);fd = get_unused_fd_flags(heap_allocation->fd_flags); if (fd < 0) { diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma- buf/heaps/cma_heap.c index 3fb4b946c91a..04de88b468a8 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -281,7 +281,8 @@ static const struct dma_buf_ops cma_heap_buf_ops = { static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup *memcg){ struct cma_heap *cma_heap = dma_heap_get_drvdata(heap); struct cma_heap_buffer *buffer; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma- buf/heaps/system_heap.c index c8959eadc71d..2bd245e1eab8 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;
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; @@ -473,11 +479,14 @@ 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;
- if (memcg)
exp_info.memcg = memcg;dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf); goto free_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);
return ERR_PTR(ret); } diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c index 007c00156520..83e2fbd6960b 100644 --- a/drivers/tee/tee_heap.c +++ b/drivers/tee/tee_heap.c @@ -167,7 +167,8 @@ static const struct dma_buf_ops tee_heap_buf_ops = { static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup *memcg){ struct tee_dma_heap *h = dma_heap_get_drvdata(heap); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..85dd747c9b36 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -505,6 +505,9 @@ 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: memcg to charge for this buffer's memory. Set only
for
buffers backed by system memory where @size reflectsthe
actual allocation in bytes.* * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. @@ -517,6 +520,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv;
- struct mem_cgroup *memcg;
}; /** diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h index 648328a64b27..2d69f8432bd2 100644 --- a/include/linux/dma-heap.h +++ b/include/linux/dma-heap.h @@ -23,7 +23,8 @@ struct dma_heap_ops { struct dma_buf *(*allocate)(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags);
u64 heap_flags,struct mem_cgroup *memcg);}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 4d1e82cf0833..f8ec751b7d7b 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -671,6 +671,8 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp); +struct mem_cgroup *mem_cgroup_get_from_fd(int fd);
bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages); @@ -1178,6 +1180,11 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; } +static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) +{
- return ERR_PTR(-EOPNOTSUPP);
+}
static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages) diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma- heap.h index a4cf716a49fa..bfcd1c25bc7e 100644 --- a/include/uapi/linux/dma-heap.h +++ b/include/uapi/linux/dma-heap.h @@ -29,6 +29,11 @@ * handle to the allocated dma-buf * @fd_flags: file descriptor flags used when allocating * @heap_flags: flags passed to heap
- @charge_fd: optional pidfd or cgroupfd whose cgroup
should be
charged for this allocation; 0 means chargethe calling
process's cgroup when the mem_accountingmodule parameter
is enabled, or skip charging otherwise
- @__padding: reserved, must be zero
* * Provided by userspace as an argument to the ioctl */ @@ -37,6 +42,8 @@ struct dma_heap_allocation_data { __u32 fd; __u32 fd_flags; __u64 heap_flags;
- __u32 charge_fd;
- __u32 __padding;
}; #define DMA_HEAP_IOC_MAGIC 'H' diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e53dd7eebc3b..17fdefc56b97 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 ?: ERR_PTR(-ENOENT);
+}
/** * mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf allocation * @memcg: memcg to charge (may be %NULL)
On Thu, Nov 12, 2026 at 08:42:18AM +0100, Thomas Hellström wrote:
On Wed, 2026-11-04 at 12:09 +0100, Albert Esteve wrote:
On embedded platforms a central process often allocates dma-buf memory on behalf of client applications. Without a way to attribute the charge to the requesting client's cgroup, the cost lands on the allocator, making per-cgroup memory limits ineffective for the actual consumers.
Add charge_fd to struct dma_heap_allocation_data. When set to a valid pidfd or cgroupfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's or cgroup's memcg and charges the buffer there via mem_cgroup_reserve_dmabuf() inside dma_heap_ioctl_allocate(). The resolved memcg is passed through dma_buf_export_info so that dma_buf_export() stores it and updates the MEMCG_DMABUF stat. Without charge_fd, and with the mem_accounting module parameter enabled, the buffer is charged to the allocator's own cgroup. The __GPF_ACCOUNT flag is removed from system heap allocations to avoid double counting.
Usage examples:
1. Central allocator charging to a client at allocation time. The allocator knows the client's PID (e.g., from binder's sender_pid) and uses pidfd to attribute the charge:
pid_t client_pid = txn->sender_pid; int pidfd = pidfd_open(client_pid, 0);
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, .charge_fd = pidfd, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); close(pidfd); /* alloc.fd is now charged to client's cgroup */
2. Default allocation (no pidfd, mem_accounting=1). When charge_fd is not set and the mem_accounting module parameter is enabled, the buffer is charged to the allocator's own cgroup:
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); /* charged to current process's cgroup */
Current limitations:
- Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. Means only the first owner (and exporter) of the shared buffer bears the charge. - Only memcg accounting supported. While this makes sense for system heap buffers, other heaps (e.g., CMA heaps) will require selectively charging also for the dmem controller.
Signed-off-by: Albert Esteve aesteve@redhat.com
Documentation/admin-guide/cgroup-v2.rst | 5 ++-- drivers/dma-buf/dma-buf.c | 5 ++++ drivers/dma-buf/dma-heap.c | 28 +++++++++++++++++--- drivers/dma-buf/heaps/cma_heap.c | 3 ++- drivers/dma-buf/heaps/system_heap.c | 21 +++++++++++---- drivers/tee/tee_heap.c | 3 ++- include/linux/dma-buf.h | 4 +++ include/linux/dma-heap.h | 3 ++- include/linux/memcontrol.h | 7 +++++ include/uapi/linux/dma-heap.h | 7 +++++ mm/memcontrol.c | 47 +++++++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index a3265a4a953e..66ddba678784 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1663,8 +1663,9 @@ The following nested keys are defined. structures. dmabuf (npn)
Amount of memory used for exported DMA buffersallocated by the cgroup.
Stays with the allocating cgroup regardless of howthe buffer is shared.
Amount of memory used for exported DMA buffersallocated by or on
behalf of the cgroup. Stays with the allocatingcgroup regardless
of how the buffer is shared.This doesn't sound right. Graphics buffers exported as DMA buffers are typically charged elsewhere, like gpu_active, and a transfer of their charge would likely stay in that key? To be consistent, shouldn't this be related to dma-heap memory and called dmaheap or something similar.
Although this work initially targeted dma heaps only, in its current form is meant to stay generic (while solving the central allocator problem). The memory exported as dma-bufs has no memory.stat of its own, and an exporter allocating plain pages (e.g., V4L2) has nowhere else to report it.
We should have a tighter definition of exactly what memory is to be registered here.
Yes, I will be more specific and clearly state that an exporter whose memory is already accounted must not opt in. That also naturally disallows transfers.
Also says "Stays with the allocating cgroup regardless of how the buffer is shared", which isn't correct after the series? Perhaps that's changed in a later patch.
I missed updating the text after having added XFER_CHARGE to the series. I'll fix the wording in this patch and mention the transfer in the next one.
BR, Albert
Thanks, Thomas
workingset_refault_anon Number of refaults of previously evicted anonymous pages. diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 1c6b9cee7ac2..837666273550 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -714,6 +714,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) struct dma_resv *resv = exp_info->resv; struct file *file; size_t alloc_size = sizeof(struct dma_buf);
- unsigned int nr_pages = PAGE_ALIGN(exp_info->size) >>
PAGE_SHIFT; int ret; if (WARN_ON(!exp_info->priv || !exp_info->ops @@ -750,6 +751,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 = exp_info->memcg;
spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll; @@ -771,6 +773,9 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
- if (dmabuf->memcg)
mod_memcg_state(dmabuf->memcg, MEMCG_DMABUF,nr_pages);
return dmabuf; err_file: diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 8a6c2b6e0cc6..7bd416ae2c23 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -10,6 +10,8 @@ #include <linux/device.h> #include <linux/dma-buf.h> #include <linux/dma-heap.h> +#include <linux/memcontrol.h> +#include <linux/sched/mm.h> #include <linux/err.h> #include <linux/export.h> #include <linux/list.h> @@ -78,6 +80,7 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) struct dma_heap_allocation_data *heap_allocation = data; struct dma_heap *heap = file->private_data; struct dma_buf *dmabuf;
- struct mem_cgroup *memcg = NULL;
int fd; size_t len; @@ -90,15 +93,34 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS) return ERR_PTR(-EINVAL);
- if (heap_allocation->__padding != 0)
return ERR_PTR(-EINVAL);- if (heap_allocation->charge_fd) {
memcg = mem_cgroup_get_from_fd(heap_allocation-charge_fd);
if (IS_ERR(memcg))return ERR_CAST(memcg);- }
- if (!memcg && mem_accounting)
memcg = get_mem_cgroup_from_mm(current->mm);len = PAGE_ALIGN(heap_allocation->len);
- if (!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);
heap_allocation->heap_flags,memcg);
- if (IS_ERR(dmabuf))
- if (IS_ERR(dmabuf)) {
mem_cgroup_put(memcg);return dmabuf;
- }
- if (memcg && !dmabuf->memcg)
mem_cgroup_put(memcg);fd = get_unused_fd_flags(heap_allocation->fd_flags); if (fd < 0) { diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma- buf/heaps/cma_heap.c index 3fb4b946c91a..04de88b468a8 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -281,7 +281,8 @@ static const struct dma_buf_ops cma_heap_buf_ops = { static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup *memcg){ struct cma_heap *cma_heap = dma_heap_get_drvdata(heap); struct cma_heap_buffer *buffer; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma- buf/heaps/system_heap.c index c8959eadc71d..2bd245e1eab8 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;
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; @@ -473,11 +479,14 @@ 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;
- if (memcg)
exp_info.memcg = memcg;dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf); goto free_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);
return ERR_PTR(ret); } diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c index 007c00156520..83e2fbd6960b 100644 --- a/drivers/tee/tee_heap.c +++ b/drivers/tee/tee_heap.c @@ -167,7 +167,8 @@ static const struct dma_buf_ops tee_heap_buf_ops = { static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup *memcg){ struct tee_dma_heap *h = dma_heap_get_drvdata(heap); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..85dd747c9b36 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -505,6 +505,9 @@ 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: memcg to charge for this buffer's memory. Set only
for
buffers backed by system memory where @size reflectsthe
actual allocation in bytes.* * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. @@ -517,6 +520,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv;
- struct mem_cgroup *memcg;
}; /** diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h index 648328a64b27..2d69f8432bd2 100644 --- a/include/linux/dma-heap.h +++ b/include/linux/dma-heap.h @@ -23,7 +23,8 @@ struct dma_heap_ops { struct dma_buf *(*allocate)(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags);
u64 heap_flags,struct mem_cgroup *memcg);}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 4d1e82cf0833..f8ec751b7d7b 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -671,6 +671,8 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp); +struct mem_cgroup *mem_cgroup_get_from_fd(int fd);
bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages); @@ -1178,6 +1180,11 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; } +static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) +{
- return ERR_PTR(-EOPNOTSUPP);
+}
static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages) diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma- heap.h index a4cf716a49fa..bfcd1c25bc7e 100644 --- a/include/uapi/linux/dma-heap.h +++ b/include/uapi/linux/dma-heap.h @@ -29,6 +29,11 @@ * handle to the allocated dma-buf * @fd_flags: file descriptor flags used when allocating * @heap_flags: flags passed to heap
- @charge_fd: optional pidfd or cgroupfd whose cgroup
should be
charged for this allocation; 0 means chargethe calling
process's cgroup when the mem_accountingmodule parameter
is enabled, or skip charging otherwise
- @__padding: reserved, must be zero
* * Provided by userspace as an argument to the ioctl */ @@ -37,6 +42,8 @@ struct dma_heap_allocation_data { __u32 fd; __u32 fd_flags; __u64 heap_flags;
- __u32 charge_fd;
- __u32 __padding;
}; #define DMA_HEAP_IOC_MAGIC 'H' diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e53dd7eebc3b..17fdefc56b97 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 ?: ERR_PTR(-ENOENT);
+}
/** * mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf allocation * @memcg: memcg to charge (may be %NULL)
On Thu, 2026-11-12 at 10:56 +0100, Albert Esteve wrote:
On Thu, Nov 12, 2026 at 08:42:18AM +0100, Thomas Hellström wrote:
On Wed, 2026-11-04 at 12:09 +0100, Albert Esteve wrote:
On embedded platforms a central process often allocates dma-buf memory on behalf of client applications. Without a way to attribute the charge to the requesting client's cgroup, the cost lands on the allocator, making per-cgroup memory limits ineffective for the actual consumers.
Add charge_fd to struct dma_heap_allocation_data. When set to a valid pidfd or cgroupfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's or cgroup's memcg and charges the buffer there via mem_cgroup_reserve_dmabuf() inside dma_heap_ioctl_allocate(). The resolved memcg is passed through dma_buf_export_info so that dma_buf_export() stores it and updates the MEMCG_DMABUF stat. Without charge_fd, and with the mem_accounting module parameter enabled, the buffer is charged to the allocator's own cgroup. The __GPF_ACCOUNT flag is removed from system heap allocations to avoid double counting.
Usage examples:
1. Central allocator charging to a client at allocation time. The allocator knows the client's PID (e.g., from binder's sender_pid) and uses pidfd to attribute the charge:
pid_t client_pid = txn->sender_pid; int pidfd = pidfd_open(client_pid, 0);
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, .charge_fd = pidfd, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); close(pidfd); /* alloc.fd is now charged to client's cgroup */
2. Default allocation (no pidfd, mem_accounting=1). When charge_fd is not set and the mem_accounting module parameter is enabled, the buffer is charged to the allocator's own cgroup:
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); /* charged to current process's cgroup */
Current limitations:
- Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. Means only the first owner (and exporter) of the shared buffer bears the charge. - Only memcg accounting supported. While this makes sense for system heap buffers, other heaps (e.g., CMA heaps) will require selectively charging also for the dmem controller.
Signed-off-by: Albert Esteve aesteve@redhat.com
Documentation/admin-guide/cgroup-v2.rst | 5 ++-- drivers/dma-buf/dma-buf.c | 5 ++++ drivers/dma-buf/dma-heap.c | 28 +++++++++++++++++--
drivers/dma-buf/heaps/cma_heap.c | 3 ++- drivers/dma-buf/heaps/system_heap.c | 21 +++++++++++---- drivers/tee/tee_heap.c | 3 ++- include/linux/dma-buf.h | 4 +++ include/linux/dma-heap.h | 3 ++- include/linux/memcontrol.h | 7 +++++ include/uapi/linux/dma-heap.h | 7 +++++ mm/memcontrol.c | 47 +++++++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index a3265a4a953e..66ddba678784 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1663,8 +1663,9 @@ The following nested keys are defined. structures. dmabuf (npn)
Amount of memory used for exported DMA buffersallocated by the cgroup.
Stays with the allocating cgroup regardless ofhow the buffer is shared.
Amount of memory used for exported DMA buffersallocated by or on
behalf of the cgroup. Stays with the allocatingcgroup regardless
of how the buffer is shared.This doesn't sound right. Graphics buffers exported as DMA buffers are typically charged elsewhere, like gpu_active, and a transfer of their charge would likely stay in that key? To be consistent, shouldn't this be related to dma-heap memory and called dmaheap or something similar.
Although this work initially targeted dma heaps only, in its current form is meant to stay generic (while solving the central allocator problem). The memory exported as dma-bufs has no memory.stat of its own, and an exporter allocating plain pages (e.g., V4L2) has nowhere else to report it.
We should have a tighter definition of exactly what memory is to be registered here.
Yes, I will be more specific and clearly state that an exporter whose memory is already accounted must not opt in. That also naturally disallows transfers.
I would suspect (although not sure TBH) a system that uses a centralized allocator for gpu memory (even if it's accounted as gpu_active) might want to be able to transfer the charge as well, so I think the transfer mechanism might want to be generic enough to handle that. At least the initial design should IMO allow for a future hook to handle such transfers even for !dma-buf memory?
Thanks, Thomas
Also says "Stays with the allocating cgroup regardless of how the buffer is shared", which isn't correct after the series? Perhaps that's changed in a later patch.
I missed updating the text after having added XFER_CHARGE to the series. I'll fix the wording in this patch and mention the transfer in the next one.
BR, Albert
Thanks, Thomas
workingset_refault_anon Number of refaults of previously evicted anonymous pages. diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma- buf.c index 1c6b9cee7ac2..837666273550 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -714,6 +714,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) struct dma_resv *resv = exp_info->resv; struct file *file; size_t alloc_size = sizeof(struct dma_buf);
- unsigned int nr_pages = PAGE_ALIGN(exp_info->size) >>
PAGE_SHIFT; int ret; if (WARN_ON(!exp_info->priv || !exp_info->ops @@ -750,6 +751,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 = exp_info->memcg;
spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf-
poll;
@@ -771,6 +773,9 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
- if (dmabuf->memcg)
mod_memcg_state(dmabuf->memcg, MEMCG_DMABUF,nr_pages);
return dmabuf; err_file: diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma- heap.c index 8a6c2b6e0cc6..7bd416ae2c23 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -10,6 +10,8 @@ #include <linux/device.h> #include <linux/dma-buf.h> #include <linux/dma-heap.h> +#include <linux/memcontrol.h> +#include <linux/sched/mm.h> #include <linux/err.h> #include <linux/export.h> #include <linux/list.h> @@ -78,6 +80,7 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) struct dma_heap_allocation_data *heap_allocation = data; struct dma_heap *heap = file->private_data; struct dma_buf *dmabuf;
- struct mem_cgroup *memcg = NULL;
int fd; size_t len; @@ -90,15 +93,34 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS) return ERR_PTR(-EINVAL);
- if (heap_allocation->__padding != 0)
return ERR_PTR(-EINVAL);- if (heap_allocation->charge_fd) {
memcg = mem_cgroup_get_from_fd(heap_allocation-charge_fd);
if (IS_ERR(memcg))return ERR_CAST(memcg);- }
- if (!memcg && mem_accounting)
memcg = get_mem_cgroup_from_mm(current->mm);len = PAGE_ALIGN(heap_allocation->len);
- if (!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);
heap_allocation-heap_flags,
memcg);
- if (IS_ERR(dmabuf))
- if (IS_ERR(dmabuf)) {
mem_cgroup_put(memcg);return dmabuf;
- }
- if (memcg && !dmabuf->memcg)
mem_cgroup_put(memcg);fd = get_unused_fd_flags(heap_allocation->fd_flags); if (fd < 0) { diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma- buf/heaps/cma_heap.c index 3fb4b946c91a..04de88b468a8 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -281,7 +281,8 @@ static const struct dma_buf_ops cma_heap_buf_ops = { static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup*memcg) { struct cma_heap *cma_heap = dma_heap_get_drvdata(heap); struct cma_heap_buffer *buffer; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma- buf/heaps/system_heap.c index c8959eadc71d..2bd245e1eab8 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;
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; @@ -473,11 +479,14 @@ 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;
- if (memcg)
exp_info.memcg = memcg;dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf); goto free_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);
return ERR_PTR(ret); } diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c index 007c00156520..83e2fbd6960b 100644 --- a/drivers/tee/tee_heap.c +++ b/drivers/tee/tee_heap.c @@ -167,7 +167,8 @@ static const struct dma_buf_ops tee_heap_buf_ops = { static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup*memcg) { struct tee_dma_heap *h = dma_heap_get_drvdata(heap); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..85dd747c9b36 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -505,6 +505,9 @@ 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: memcg to charge for this buffer's memory. Set
only for
buffers backed by system memory where @sizereflects the
actual allocation in bytes.* * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. @@ -517,6 +520,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv;
- struct mem_cgroup *memcg;
}; /** diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h index 648328a64b27..2d69f8432bd2 100644 --- a/include/linux/dma-heap.h +++ b/include/linux/dma-heap.h @@ -23,7 +23,8 @@ struct dma_heap_ops { struct dma_buf *(*allocate)(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags);
u64 heap_flags,struct mem_cgroup *memcg);}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 4d1e82cf0833..f8ec751b7d7b 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -671,6 +671,8 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp); +struct mem_cgroup *mem_cgroup_get_from_fd(int fd);
bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages); @@ -1178,6 +1180,11 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; } +static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) +{
- return ERR_PTR(-EOPNOTSUPP);
+}
static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages) diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma- heap.h index a4cf716a49fa..bfcd1c25bc7e 100644 --- a/include/uapi/linux/dma-heap.h +++ b/include/uapi/linux/dma-heap.h @@ -29,6 +29,11 @@ * handle to the allocated dma-buf * @fd_flags: file descriptor flags used when allocating * @heap_flags: flags passed to heap
- @charge_fd: optional pidfd or cgroupfd whose cgroup
should be
charged for this allocation; 0 meanscharge the calling
process's cgroup when the mem_accountingmodule parameter
is enabled, or skip charging otherwise
- @__padding: reserved, must be zero
* * Provided by userspace as an argument to the ioctl */ @@ -37,6 +42,8 @@ struct dma_heap_allocation_data { __u32 fd; __u32 fd_flags; __u64 heap_flags;
- __u32 charge_fd;
- __u32 __padding;
}; #define DMA_HEAP_IOC_MAGIC 'H' diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e53dd7eebc3b..17fdefc56b97 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 ?: ERR_PTR(-ENOENT);
+}
/** * mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf allocation * @memcg: memcg to charge (may be %NULL)
On Thu, Nov 12, 2026 at 11:04:54AM +0100, Thomas Hellström wrote:
On Thu, 2026-11-12 at 10:56 +0100, Albert Esteve wrote:
On Thu, Nov 12, 2026 at 08:42:18AM +0100, Thomas Hellström wrote:
On Wed, 2026-11-04 at 12:09 +0100, Albert Esteve wrote:
On embedded platforms a central process often allocates dma-buf memory on behalf of client applications. Without a way to attribute the charge to the requesting client's cgroup, the cost lands on the allocator, making per-cgroup memory limits ineffective for the actual consumers.
Add charge_fd to struct dma_heap_allocation_data. When set to a valid pidfd or cgroupfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's or cgroup's memcg and charges the buffer there via mem_cgroup_reserve_dmabuf() inside dma_heap_ioctl_allocate(). The resolved memcg is passed through dma_buf_export_info so that dma_buf_export() stores it and updates the MEMCG_DMABUF stat. Without charge_fd, and with the mem_accounting module parameter enabled, the buffer is charged to the allocator's own cgroup. The __GPF_ACCOUNT flag is removed from system heap allocations to avoid double counting.
Usage examples:
1. Central allocator charging to a client at allocation time. The allocator knows the client's PID (e.g., from binder's sender_pid) and uses pidfd to attribute the charge:
pid_t client_pid = txn->sender_pid; int pidfd = pidfd_open(client_pid, 0);
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, .charge_fd = pidfd, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); close(pidfd); /* alloc.fd is now charged to client's cgroup */
2. Default allocation (no pidfd, mem_accounting=1). When charge_fd is not set and the mem_accounting module parameter is enabled, the buffer is charged to the allocator's own cgroup:
struct dma_heap_allocation_data alloc = { .len = buffer_size, .fd_flags = O_RDWR | O_CLOEXEC, }; ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc); /* charged to current process's cgroup */
Current limitations:
- Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. Means only the first owner (and exporter) of the shared buffer bears the charge. - Only memcg accounting supported. While this makes sense for system heap buffers, other heaps (e.g., CMA heaps) will require selectively charging also for the dmem controller.
Signed-off-by: Albert Esteve aesteve@redhat.com
Documentation/admin-guide/cgroup-v2.rst | 5 ++-- drivers/dma-buf/dma-buf.c | 5 ++++ drivers/dma-buf/dma-heap.c | 28 +++++++++++++++++--
drivers/dma-buf/heaps/cma_heap.c | 3 ++- drivers/dma-buf/heaps/system_heap.c | 21 +++++++++++---- drivers/tee/tee_heap.c | 3 ++- include/linux/dma-buf.h | 4 +++ include/linux/dma-heap.h | 3 ++- include/linux/memcontrol.h | 7 +++++ include/uapi/linux/dma-heap.h | 7 +++++ mm/memcontrol.c | 47 +++++++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index a3265a4a953e..66ddba678784 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1663,8 +1663,9 @@ The following nested keys are defined. structures. dmabuf (npn)
Amount of memory used for exported DMA buffersallocated by the cgroup.
Stays with the allocating cgroup regardless ofhow the buffer is shared.
Amount of memory used for exported DMA buffersallocated by or on
behalf of the cgroup. Stays with the allocatingcgroup regardless
of how the buffer is shared.This doesn't sound right. Graphics buffers exported as DMA buffers are typically charged elsewhere, like gpu_active, and a transfer of their charge would likely stay in that key? To be consistent, shouldn't this be related to dma-heap memory and called dmaheap or something similar.
Although this work initially targeted dma heaps only, in its current form is meant to stay generic (while solving the central allocator problem). The memory exported as dma-bufs has no memory.stat of its own, and an exporter allocating plain pages (e.g., V4L2) has nowhere else to report it.
We should have a tighter definition of exactly what memory is to be registered here.
Yes, I will be more specific and clearly state that an exporter whose memory is already accounted must not opt in. That also naturally disallows transfers.
I would suspect (although not sure TBH) a system that uses a centralized allocator for gpu memory (even if it's accounted as gpu_active) might want to be able to transfer the charge as well, so I think the transfer mechanism might want to be generic enough to handle that.
That makes sense as a direction, but my aim for this series was to tackle the central allocator problem, that is, memory charge at dma-buf level to the cgroup an allocation is made for, and moved afterwards if needed.
For allocators that charge per page (as in Dave's series) a transfer would move each page's charges between objcgs, which is a different mechanism from the dma-buf level charge in this series.
But iiuc you feedback is in line with Christian's feedback that uAPI should not block later extension. To that end, XFER_CHARGE ioctl has a reserved flags field, so more buffer types can be potentially supported later, as they opt-in, without uAPI changes.
At least the initial design should IMO allow for a future hook to handle such transfers even for !dma-buf memory?
Not sure what you mean by hook here. A callback in the exporter? A charge op in dma_buf_ops was already objected before [1].
[1] https://lore.kernel.org/lkml/5cc27a05-8131-ce9b-dea1-5c75e994216d@amd.com/
Thanks, Thomas
Also says "Stays with the allocating cgroup regardless of how the buffer is shared", which isn't correct after the series? Perhaps that's changed in a later patch.
I missed updating the text after having added XFER_CHARGE to the series. I'll fix the wording in this patch and mention the transfer in the next one.
BR, Albert
Thanks, Thomas
workingset_refault_anon Number of refaults of previously evicted anonymous pages. diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma- buf.c index 1c6b9cee7ac2..837666273550 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -714,6 +714,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) struct dma_resv *resv = exp_info->resv; struct file *file; size_t alloc_size = sizeof(struct dma_buf);
- unsigned int nr_pages = PAGE_ALIGN(exp_info->size) >>
PAGE_SHIFT; int ret; if (WARN_ON(!exp_info->priv || !exp_info->ops @@ -750,6 +751,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 = exp_info->memcg;
spin_lock_init(&dmabuf->name_lock); init_waitqueue_head(&dmabuf->poll); dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf-
poll;
@@ -771,6 +773,9 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
- if (dmabuf->memcg)
mod_memcg_state(dmabuf->memcg, MEMCG_DMABUF,nr_pages);
return dmabuf; err_file: diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma- heap.c index 8a6c2b6e0cc6..7bd416ae2c23 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -10,6 +10,8 @@ #include <linux/device.h> #include <linux/dma-buf.h> #include <linux/dma-heap.h> +#include <linux/memcontrol.h> +#include <linux/sched/mm.h> #include <linux/err.h> #include <linux/export.h> #include <linux/list.h> @@ -78,6 +80,7 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) struct dma_heap_allocation_data *heap_allocation = data; struct dma_heap *heap = file->private_data; struct dma_buf *dmabuf;
- struct mem_cgroup *memcg = NULL;
int fd; size_t len; @@ -90,15 +93,34 @@ static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data) if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS) return ERR_PTR(-EINVAL);
- if (heap_allocation->__padding != 0)
return ERR_PTR(-EINVAL);- if (heap_allocation->charge_fd) {
memcg = mem_cgroup_get_from_fd(heap_allocation-charge_fd);
if (IS_ERR(memcg))return ERR_CAST(memcg);- }
- if (!memcg && mem_accounting)
memcg = get_mem_cgroup_from_mm(current->mm);len = PAGE_ALIGN(heap_allocation->len);
- if (!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);
heap_allocation-heap_flags,
memcg);
- if (IS_ERR(dmabuf))
- if (IS_ERR(dmabuf)) {
mem_cgroup_put(memcg);return dmabuf;
- }
- if (memcg && !dmabuf->memcg)
mem_cgroup_put(memcg);fd = get_unused_fd_flags(heap_allocation->fd_flags); if (fd < 0) { diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma- buf/heaps/cma_heap.c index 3fb4b946c91a..04de88b468a8 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -281,7 +281,8 @@ static const struct dma_buf_ops cma_heap_buf_ops = { static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup*memcg) { struct cma_heap *cma_heap = dma_heap_get_drvdata(heap); struct cma_heap_buffer *buffer; diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma- buf/heaps/system_heap.c index c8959eadc71d..2bd245e1eab8 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;
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; @@ -473,11 +479,14 @@ 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;
- if (memcg)
exp_info.memcg = memcg;dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf); goto free_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);
return ERR_PTR(ret); } diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c index 007c00156520..83e2fbd6960b 100644 --- a/drivers/tee/tee_heap.c +++ b/drivers/tee/tee_heap.c @@ -167,7 +167,8 @@ static const struct dma_buf_ops tee_heap_buf_ops = { static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags)
u64 heap_flags,struct mem_cgroup*memcg) { struct tee_dma_heap *h = dma_heap_get_drvdata(heap); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 23ec0ff23fba..85dd747c9b36 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -505,6 +505,9 @@ 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: memcg to charge for this buffer's memory. Set
only for
buffers backed by system memory where @sizereflects the
actual allocation in bytes.* * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. @@ -517,6 +520,7 @@ struct dma_buf_export_info { int flags; struct dma_resv *resv; void *priv;
- struct mem_cgroup *memcg;
}; /** diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h index 648328a64b27..2d69f8432bd2 100644 --- a/include/linux/dma-heap.h +++ b/include/linux/dma-heap.h @@ -23,7 +23,8 @@ struct dma_heap_ops { struct dma_buf *(*allocate)(struct dma_heap *heap, unsigned long len, u32 fd_flags,
u64 heap_flags);
u64 heap_flags,struct mem_cgroup *memcg);}; /** diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 4d1e82cf0833..f8ec751b7d7b 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -671,6 +671,8 @@ int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id, struct mm_struct *mm, gfp_t gfp); +struct mem_cgroup *mem_cgroup_get_from_fd(int fd);
bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages); @@ -1178,6 +1180,11 @@ static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, return 0; } +static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) +{
- return ERR_PTR(-EOPNOTSUPP);
+}
static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages) diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma- heap.h index a4cf716a49fa..bfcd1c25bc7e 100644 --- a/include/uapi/linux/dma-heap.h +++ b/include/uapi/linux/dma-heap.h @@ -29,6 +29,11 @@ * handle to the allocated dma-buf * @fd_flags: file descriptor flags used when allocating * @heap_flags: flags passed to heap
- @charge_fd: optional pidfd or cgroupfd whose cgroup
should be
charged for this allocation; 0 meanscharge the calling
process's cgroup when the mem_accountingmodule parameter
is enabled, or skip charging otherwise
- @__padding: reserved, must be zero
* * Provided by userspace as an argument to the ioctl */ @@ -37,6 +42,8 @@ struct dma_heap_allocation_data { __u32 fd; __u32 fd_flags; __u64 heap_flags;
- __u32 charge_fd;
- __u32 __padding;
}; #define DMA_HEAP_IOC_MAGIC 'H' diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e53dd7eebc3b..17fdefc56b97 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 ?: ERR_PTR(-ENOENT);
+}
/** * mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf allocation * @memcg: memcg to charge (may be %NULL)
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. Concurrent transfers are serialized by the dma-buf reservation lock, so lockdep covers the locking order of the charge path early. The transfer implementation is generic (mem_cgroup_transfer_charge) and not dma-buf specific, so it can be reused by other subsystems.
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. The xfer ioctl returns -EOPNOTSUPP for buffers whose memcg is not set. Exporters opt in by passing a memcg through dma_buf_export_info at export time. 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 | 42 ++++++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 3 ++- include/linux/memcontrol.h | 13 +++++++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++++++ mm/memcontrol.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 837666273550..3bd26fbb1109 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; + unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT; + 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; + + memcg = mem_cgroup_get_from_fd(arg.charge_fd); + if (IS_ERR(memcg)) + return PTR_ERR(memcg); + + dma_resv_lock(dmabuf->resv, NULL); + + if (!dmabuf->memcg) { + ret = -EOPNOTSUPP; + goto out; + } + + if (dmabuf->memcg != memcg && + !mem_cgroup_transfer_charge(&dmabuf->memcg, memcg, MEMCG_DMABUF, nr_pages)) + ret = -ENOMEM; + +out: + dma_resv_unlock(dmabuf->resv); + 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; } diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 85dd747c9b36..b01149d26b44 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -507,7 +507,8 @@ struct dma_buf_attachment { * @priv: Attach private data of allocator to this buffer * @memcg: memcg to charge for this buffer's memory. Set only for * buffers backed by system memory where @size reflects the - * actual allocation in bytes. + * actual allocation in bytes. Enables charge transfer via + * DMA_BUF_IOCTL_XFER_CHARGE. * * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f8ec751b7d7b..af02d9777fe9 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -676,6 +676,11 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd); bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages);
+bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg, + struct mem_cgroup *new_memcg, + enum memcg_stat_item item, + unsigned int nr_pages); + void __mem_cgroup_uncharge(struct folio *folio);
/** @@ -1197,6 +1202,14 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { }
+static inline bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg, + struct mem_cgroup *new_memcg, + enum memcg_stat_item item, + unsigned int nr_pages) +{ + 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..0aaf2e7d593b 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 memcg charge, the + * ioctl returns -EOPNOTSUPP. + */ +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 17fdefc56b97..3e717b5663c8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5752,6 +5752,48 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) } EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf);
+/** + * mem_cgroup_transfer_charge - move a charge between memcgs + * @ptr_memcg: pointer to the memcg to transfer from + * @new_memcg: target memcg; caller must hold a reference + * @item: stat counter to adjust + * @nr_pages: number of pages to transfer + * + * Transfers @nr_pages of charge from *@ptr_memcg to @new_memcg and, on + * success, stores @new_memcg in *@ptr_memcg. + * + * Updates to *@ptr_memcg must be serialized by the caller. + * Returns %true on success, %false if the charge against @new_memcg + * failed. + */ +bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg, + struct mem_cgroup *new_memcg, + enum memcg_stat_item item, + unsigned int nr_pages) +{ + struct mem_cgroup *old_memcg = *ptr_memcg; + + if (!new_memcg || mem_cgroup_disabled()) + return true; + + if (try_charge(new_memcg, GFP_KERNEL, nr_pages)) + return false; + + mod_memcg_state(new_memcg, item, nr_pages); + css_get(&new_memcg->css); + *ptr_memcg = new_memcg; + + if (old_memcg) { + /* Uncharge old cgroup */ + mod_memcg_state(old_memcg, item, -nr_pages); + if (!mem_cgroup_is_root(old_memcg)) + refill_stock(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 11/4/26 12:09, 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. Concurrent transfers are serialized by the dma-buf reservation lock, so lockdep covers the locking order of the charge path early. The transfer implementation is generic (mem_cgroup_transfer_charge) and not dma-buf specific, so it can be reused by other subsystems.
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. The xfer ioctl returns -EOPNOTSUPP for buffers whose memcg is not set. Exporters opt in by passing a memcg through dma_buf_export_info at export time. 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 | 42 ++++++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 3 ++- include/linux/memcontrol.h | 13 +++++++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++++++ mm/memcontrol.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 837666273550..3bd26fbb1109 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;
- unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
- 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;- memcg = mem_cgroup_get_from_fd(arg.charge_fd);
- if (IS_ERR(memcg))
return PTR_ERR(memcg);- dma_resv_lock(dmabuf->resv, NULL);
- if (!dmabuf->memcg) {
ret = -EOPNOTSUPP;goto out;- }
- if (dmabuf->memcg != memcg &&
!mem_cgroup_transfer_charge(&dmabuf->memcg, memcg, MEMCG_DMABUF, nr_pages))
Please move the calculation of nr_pages after taking the dma_resv_lock().
We currently don't have anybody dynamically changing DMA-buf sizes but just to be on the save side.
With that done Reviewed-by: Christian König christian.koenig@amd.com
Regards, Christian.
ret = -ENOMEM;+out:
- dma_resv_unlock(dmabuf->resv);
- 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; }
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 85dd747c9b36..b01149d26b44 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -507,7 +507,8 @@ struct dma_buf_attachment {
- @priv: Attach private data of allocator to this buffer
- @memcg: memcg to charge for this buffer's memory. Set only for
buffers backed by system memory where @size reflects the
actual allocation in bytes.
actual allocation in bytes. Enables charge transfer via
DMA_BUF_IOCTL_XFER_CHARGE.- This structure holds the information required to export the buffer. Used
- with dma_buf_export() only.
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f8ec751b7d7b..af02d9777fe9 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -676,6 +676,11 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd); bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages); +bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages);void __mem_cgroup_uncharge(struct folio *folio); /** @@ -1197,6 +1202,14 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { } +static inline bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages)+{
- 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..0aaf2e7d593b 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 memcg charge, the
- ioctl returns -EOPNOTSUPP.
- */
+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 17fdefc56b97..3e717b5663c8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5752,6 +5752,48 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) } EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf); +/**
- mem_cgroup_transfer_charge - move a charge between memcgs
- @ptr_memcg: pointer to the memcg to transfer from
- @new_memcg: target memcg; caller must hold a reference
- @item: stat counter to adjust
- @nr_pages: number of pages to transfer
- Transfers @nr_pages of charge from *@ptr_memcg to @new_memcg and, on
- success, stores @new_memcg in *@ptr_memcg.
- Updates to *@ptr_memcg must be serialized by the caller.
- Returns %true on success, %false if the charge against @new_memcg
- failed.
- */
+bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages)+{
- struct mem_cgroup *old_memcg = *ptr_memcg;
- if (!new_memcg || mem_cgroup_disabled())
return true;- if (try_charge(new_memcg, GFP_KERNEL, nr_pages))
return false;- mod_memcg_state(new_memcg, item, nr_pages);
- css_get(&new_memcg->css);
- *ptr_memcg = new_memcg;
- if (old_memcg) {
/* Uncharge old cgroup */mod_memcg_state(old_memcg, item, -nr_pages);if (!mem_cgroup_is_root(old_memcg))refill_stock(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, Nov 4, 2026 at 2:52 PM Christian König christian.koenig@amd.com wrote:
On 11/4/26 12:09, 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. Concurrent transfers are serialized by the dma-buf reservation lock, so lockdep covers the locking order of the charge path early. The transfer implementation is generic (mem_cgroup_transfer_charge) and not dma-buf specific, so it can be reused by other subsystems.
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. The xfer ioctl returns -EOPNOTSUPP for buffers whose memcg is not set. Exporters opt in by passing a memcg through dma_buf_export_info at export time. 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 | 42 ++++++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 3 ++- include/linux/memcontrol.h | 13 +++++++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++++++ mm/memcontrol.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 837666273550..3bd26fbb1109 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;unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;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;memcg = mem_cgroup_get_from_fd(arg.charge_fd);if (IS_ERR(memcg))return PTR_ERR(memcg);dma_resv_lock(dmabuf->resv, NULL);if (!dmabuf->memcg) {ret = -EOPNOTSUPP;goto out;}if (dmabuf->memcg != memcg &&!mem_cgroup_transfer_charge(&dmabuf->memcg, memcg, MEMCG_DMABUF, nr_pages))Please move the calculation of nr_pages after taking the dma_resv_lock().
We currently don't have anybody dynamically changing DMA-buf sizes but just to be on the save side.
Sounds like a good defensive option. I was also tempted to move it. Will do for v5.
With that done Reviewed-by: Christian König christian.koenig@amd.com
Thanks!
BR, Albert.
Regards, Christian.
ret = -ENOMEM;+out:
dma_resv_unlock(dmabuf->resv);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; }diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 85dd747c9b36..b01149d26b44 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -507,7 +507,8 @@ struct dma_buf_attachment {
- @priv: Attach private data of allocator to this buffer
- @memcg: memcg to charge for this buffer's memory. Set only for
buffers backed by system memory where @size reflects the
actual allocation in bytes.
actual allocation in bytes. Enables charge transfer via
DMA_BUF_IOCTL_XFER_CHARGE.- This structure holds the information required to export the buffer. Used
- with dma_buf_export() only.
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f8ec751b7d7b..af02d9777fe9 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -676,6 +676,11 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd); bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages);
+bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages);void __mem_cgroup_uncharge(struct folio *folio);
/** @@ -1197,6 +1202,14 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { }
+static inline bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages)+{
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..0aaf2e7d593b 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 memcg charge, the
- ioctl returns -EOPNOTSUPP.
- */
+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 17fdefc56b97..3e717b5663c8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5752,6 +5752,48 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) } EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf);
+/**
- mem_cgroup_transfer_charge - move a charge between memcgs
- @ptr_memcg: pointer to the memcg to transfer from
- @new_memcg: target memcg; caller must hold a reference
- @item: stat counter to adjust
- @nr_pages: number of pages to transfer
- Transfers @nr_pages of charge from *@ptr_memcg to @new_memcg and, on
- success, stores @new_memcg in *@ptr_memcg.
- Updates to *@ptr_memcg must be serialized by the caller.
- Returns %true on success, %false if the charge against @new_memcg
- failed.
- */
+bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages)+{
struct mem_cgroup *old_memcg = *ptr_memcg;if (!new_memcg || mem_cgroup_disabled())return true;if (try_charge(new_memcg, GFP_KERNEL, nr_pages))return false;mod_memcg_state(new_memcg, item, nr_pages);css_get(&new_memcg->css);*ptr_memcg = new_memcg;if (old_memcg) {/* Uncharge old cgroup */mod_memcg_state(old_memcg, item, -nr_pages);if (!mem_cgroup_is_root(old_memcg))refill_stock(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 2 potential issue(s) to consider: - [High] dma-buf: unprivileged cgroup escape and OOM via dma_buf_xfer_charge() - [Medium] dma-buf: broken !CONFIG_MEMCG and disabled-memcg handling in dma_buf_xfer_charge() --
commit 3f75f95a079fdfc2b4d2659ae4dbea6726be45ad Author: Albert Esteve aesteve@redhat.com
dma-buf: transfer memcg charges via DMA_BUF_IOCTL_XFER_CHARGE
This commit adds the DMA_BUF_IOCTL_XFER_CHARGE ioctl and a generic mem_cgroup_transfer_charge() helper to transfer a dma-buf's memory cgroup charge to a target cgroup identified by a cgroupfd or pidfd.
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 8376662735502..3bd26fbb11092 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;
- unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
- int ret = 0;
- if (copy_from_user(&arg, user_data, sizeof(arg)))
return -EFAULT;- if (!IS_ENABLED(CONFIG_MEMCG))
return 0;
[Severity: Medium] As reported by Sashiko on v3 [1], when CONFIG_MEMCG is disabled, dma_buf_xfer_charge() returns 0 before checking arg.flags, mem_cgroup_get_from_fd(arg.charge_fd), or dmabuf->memcg.
On a CONFIG_MEMCG=n kernel, won't this accept non-zero arg.flags instead of returning -EINVAL, and return 0 instead of -EOPNOTSUPP on buffers that have no existing memcg charge?
In include/uapi/linux/dma-buf.h, struct dma_buf_transfer_charge documents that flags must be zero and that the ioctl returns -EOPNOTSUPP if the buffer has no existing memcg charge. In include/linux/memcontrol.h, the !CONFIG_MEMCG stub for mem_cgroup_get_from_fd() already returns ERR_PTR(-EOPNOTSUPP):
include/linux/memcontrol.h: static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) { return ERR_PTR(-EOPNOTSUPP); }
Returning 0 early here bypasses that stub and leaves the !CONFIG_MEMCG stub for mem_cgroup_transfer_charge() in include/linux/memcontrol.h unreachable.
In addition, in mm/memcontrol.c, mem_cgroup_transfer_charge() has:
if (!new_memcg || mem_cgroup_disabled()) return true;
which returns true without updating *ptr_memcg or uncharging and dropping the reference to old_memcg, even though the function comment states that on success it stores new_memcg in *ptr_memcg. Should the early CONFIG_MEMCG check in dma_buf_xfer_charge() be dropped so it falls through to arg.flags validation and mem_cgroup_get_from_fd()?
- if (arg.flags != 0)
return -EINVAL;- memcg = mem_cgroup_get_from_fd(arg.charge_fd);
- if (IS_ERR(memcg))
return PTR_ERR(memcg);- dma_resv_lock(dmabuf->resv, NULL);
- if (!dmabuf->memcg) {
ret = -EOPNOTSUPP;goto out;- }
- if (dmabuf->memcg != memcg &&
!mem_cgroup_transfer_charge(&dmabuf->memcg, memcg, MEMCG_DMABUF, nr_pages))ret = -ENOMEM;
[Severity: High] As reported by Sashiko on v3 [2], can an unprivileged holder of a dma-buf file descriptor use dma_buf_xfer_charge() to escape its own memory cgroup limits or trigger reclaim and OOM kills in another cgroup?
In dma_buf_ioctl() -> dma_buf_xfer_charge() -> mem_cgroup_get_from_fd(), neither dma_buf_xfer_charge() nor mem_cgroup_get_from_fd() checks DAC permissions, capabilities (such as CAP_SYS_RESOURCE or CAP_SYS_ADMIN), cgroup v2 write/delegation permissions (such as cgroup_procs_write_permission() used by CLONE_INTO_CGROUP), or task access permissions (such as ptrace_may_access()) on either the source cgroup (dmabuf->memcg) or the target cgroup/task (memcg):
mm/memcontrol.c:mem_cgroup_get_from_fd() { cgrp = cgroup_get_from_fd(fd); if (!IS_ERR(cgrp)) { ... return memcg; }
task = pidfd_get_task(fd, &pidfd_flags); ... }
Because /sys/fs/cgroup directories are world-readable (0755) by default and pidfd_open() requires no privileges for visible PIDs, any unprivileged process can open /sys/fs/cgroup (root_mem_cgroup) or another cgroup's directory or pidfd.
Even with the subsequent LSM and SELinux patches in this series: 1. On systems without SELinux (such as AppArmor, Smack, or CONFIG_SECURITY=n), security_dma_buf_charge_cgroup() and security_dma_buf_charge_task() return 0 with no baseline DAC, cgroup permission, or capability checks. 2. On SELinux systems, selinux_dma_buf_charge_cgroup() checks kernfs_xattr_get(cgrp->kn, XATTR_NAME_SELINUX, NULL, 0) and returns 0 on -ENODATA whenever a cgroup directory lacks an explicit security.selinux xattr (including /sys/fs/cgroup and cgroups labeled via default genfscon policy rules). 3. Neither dma_buf_xfer_charge() nor the LSM hooks check whether the caller is authorized to uncharge the source cgroup (dmabuf->memcg).
When mem_cgroup_transfer_charge() runs:
mm/memcontrol.c:mem_cgroup_transfer_charge() { ... if (try_charge(new_memcg, GFP_KERNEL, nr_pages)) return false; ... if (old_memcg) { /* Uncharge old cgroup */ mod_memcg_state(old_memcg, item, -nr_pages); if (!mem_cgroup_is_root(old_memcg)) refill_stock(old_memcg, nr_pages); mem_cgroup_put(old_memcg); } }
A confined process can pass an fd for /sys/fs/cgroup (where try_charge() on root_mem_cgroup is a no-op) to uncharge its own cgroup and bypass memory.max, or pass an fd for another cgroup to trigger GFP_KERNEL direct reclaim and mem_cgroup_oom() in that victim cgroup. Should dma_buf_xfer_charge() or mem_cgroup_get_from_fd() enforce baseline cgroup write/task permissions and authorization to uncharge dmabuf->memcg?
[1] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-7-78d... [2] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-7-78d...
On Wed, 2026-11-04 at 12:09 +0100, 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. Concurrent transfers are serialized by the dma-buf reservation lock, so lockdep covers the locking order of the charge path early. The transfer implementation is generic (mem_cgroup_transfer_charge) and not dma-buf specific, so it can be reused by other subsystems.
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. The xfer ioctl returns -EOPNOTSUPP for buffers whose memcg is not set. Exporters opt in by passing a memcg through dma_buf_export_info at export time. 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 | 42 ++++++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 3 ++- include/linux/memcontrol.h | 13 +++++++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++++++ mm/memcontrol.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 837666273550..3bd26fbb1109 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;
- unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >>
PAGE_SHIFT;
- 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;- memcg = mem_cgroup_get_from_fd(arg.charge_fd);
- if (IS_ERR(memcg))
return PTR_ERR(memcg);- dma_resv_lock(dmabuf->resv, NULL);
- if (!dmabuf->memcg) {
ret = -EOPNOTSUPP;goto out;- }
- if (dmabuf->memcg != memcg &&
- !mem_cgroup_transfer_charge(&dmabuf->memcg, memcg,
MEMCG_DMABUF, nr_pages))
ret = -ENOMEM;+out:
- dma_resv_unlock(dmabuf->resv);
- 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; } diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 85dd747c9b36..b01149d26b44 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -507,7 +507,8 @@ struct dma_buf_attachment { * @priv: Attach private data of allocator to this buffer * @memcg: memcg to charge for this buffer's memory. Set only for * buffers backed by system memory where @size reflects the
actual allocation in bytes.
actual allocation in bytes. Enables charge transfervia
DMA_BUF_IOCTL_XFER_CHARGE.* * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f8ec751b7d7b..af02d9777fe9 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -676,6 +676,11 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd); bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages); +bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages);void __mem_cgroup_uncharge(struct folio *folio); /** @@ -1197,6 +1202,14 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { } +static inline bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup*new_memcg,
enum memcg_stat_itemitem,
unsigned int nr_pages)+{
- 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..0aaf2e7d593b 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 memcg charge, the
- ioctl returns -EOPNOTSUPP.
- */
+struct dma_buf_transfer_charge {
- /** @flags: Reserved, must be zero */
- __u32 flags; /* unused for now */
- /**
* @charge_fd: File descriptor identifying the targetcgroup.
** May be either a pidfd, in which case the charge moves tothe
* memory cgroup of the referenced process, or a cgroupfdobtained
* by opening a cgroup v2 directory, in which case thecharge 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 17fdefc56b97..3e717b5663c8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5752,6 +5752,48 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) } EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf); +/**
- mem_cgroup_transfer_charge - move a charge between memcgs
- @ptr_memcg: pointer to the memcg to transfer from
- @new_memcg: target memcg; caller must hold a reference
- @item: stat counter to adjust
- @nr_pages: number of pages to transfer
- Transfers @nr_pages of charge from *@ptr_memcg to @new_memcg and,
on
- success, stores @new_memcg in *@ptr_memcg.
- Updates to *@ptr_memcg must be serialized by the caller.
- Returns %true on success, %false if the charge against @new_memcg
- failed.
- */
+bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages)+{
- struct mem_cgroup *old_memcg = *ptr_memcg;
- if (!new_memcg || mem_cgroup_disabled())
return true;- if (try_charge(new_memcg, GFP_KERNEL, nr_pages))
return false;- mod_memcg_state(new_memcg, item, nr_pages);
- css_get(&new_memcg->css);
- *ptr_memcg = new_memcg;
- if (old_memcg) {
/* Uncharge old cgroup */mod_memcg_state(old_memcg, item, -nr_pages);if (!mem_cgroup_is_root(old_memcg))refill_stock(old_memcg, nr_pages);mem_cgroup_put(old_memcg);- }
- return true;
+}
This has the same lifetime issue as patch 1. After a transfer, the buffer pins new_memcg until it is released, and with cross-process transfers the target may be removed before the buffer is. Since this helper is meant to be reused by other subsystems, would it be better to have it take and store an obj_cgroup from the start?
Thanks, Thomas
/** * mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg * @dmabuf: buffer to uncharge
On Thu, Nov 12, 2026 at 08:46:02AM +0100, Thomas Hellström wrote:
On Wed, 2026-11-04 at 12:09 +0100, 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. Concurrent transfers are serialized by the dma-buf reservation lock, so lockdep covers the locking order of the charge path early. The transfer implementation is generic (mem_cgroup_transfer_charge) and not dma-buf specific, so it can be reused by other subsystems.
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. The xfer ioctl returns -EOPNOTSUPP for buffers whose memcg is not set. Exporters opt in by passing a memcg through dma_buf_export_info at export time. 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 | 42 ++++++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 3 ++- include/linux/memcontrol.h | 13 +++++++++++++ include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++++++ mm/memcontrol.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 837666273550..3bd26fbb1109 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;
- unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >>
PAGE_SHIFT;
- 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;- memcg = mem_cgroup_get_from_fd(arg.charge_fd);
- if (IS_ERR(memcg))
return PTR_ERR(memcg);- dma_resv_lock(dmabuf->resv, NULL);
- if (!dmabuf->memcg) {
ret = -EOPNOTSUPP;goto out;- }
- if (dmabuf->memcg != memcg &&
- !mem_cgroup_transfer_charge(&dmabuf->memcg, memcg,
MEMCG_DMABUF, nr_pages))
ret = -ENOMEM;+out:
- dma_resv_unlock(dmabuf->resv);
- 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; } diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 85dd747c9b36..b01149d26b44 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -507,7 +507,8 @@ struct dma_buf_attachment { * @priv: Attach private data of allocator to this buffer * @memcg: memcg to charge for this buffer's memory. Set only for * buffers backed by system memory where @size reflects the
actual allocation in bytes.
actual allocation in bytes. Enables charge transfervia
DMA_BUF_IOCTL_XFER_CHARGE.* * This structure holds the information required to export the buffer. Used * with dma_buf_export() only. diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f8ec751b7d7b..af02d9777fe9 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -676,6 +676,11 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd); bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp, unsigned int nr_pages); +bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages);void __mem_cgroup_uncharge(struct folio *folio); /** @@ -1197,6 +1202,14 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, { } +static inline bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup*new_memcg,
enum memcg_stat_itemitem,
unsigned int nr_pages)+{
- 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..0aaf2e7d593b 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 memcg charge, the
- ioctl returns -EOPNOTSUPP.
- */
+struct dma_buf_transfer_charge {
- /** @flags: Reserved, must be zero */
- __u32 flags; /* unused for now */
- /**
* @charge_fd: File descriptor identifying the targetcgroup.
** May be either a pidfd, in which case the charge moves tothe
* memory cgroup of the referenced process, or a cgroupfdobtained
* by opening a cgroup v2 directory, in which case thecharge 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 17fdefc56b97..3e717b5663c8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5752,6 +5752,48 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages) } EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf); +/**
- mem_cgroup_transfer_charge - move a charge between memcgs
- @ptr_memcg: pointer to the memcg to transfer from
- @new_memcg: target memcg; caller must hold a reference
- @item: stat counter to adjust
- @nr_pages: number of pages to transfer
- Transfers @nr_pages of charge from *@ptr_memcg to @new_memcg and,
on
- success, stores @new_memcg in *@ptr_memcg.
- Updates to *@ptr_memcg must be serialized by the caller.
- Returns %true on success, %false if the charge against @new_memcg
- failed.
- */
+bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
struct mem_cgroup *new_memcg,enum memcg_stat_item item,unsigned int nr_pages)+{
- struct mem_cgroup *old_memcg = *ptr_memcg;
- if (!new_memcg || mem_cgroup_disabled())
return true;- if (try_charge(new_memcg, GFP_KERNEL, nr_pages))
return false;- mod_memcg_state(new_memcg, item, nr_pages);
- css_get(&new_memcg->css);
- *ptr_memcg = new_memcg;
- if (old_memcg) {
/* Uncharge old cgroup */mod_memcg_state(old_memcg, item, -nr_pages);if (!mem_cgroup_is_root(old_memcg))refill_stock(old_memcg, nr_pages);mem_cgroup_put(old_memcg);- }
- return true;
+}
This has the same lifetime issue as patch 1. After a transfer, the buffer pins new_memcg until it is released, and with cross-process transfers the target may be removed before the buffer is. Since this helper is meant to be reused by other subsystems, would it be better to have it take and store an obj_cgroup from the start?
Agreed. I will switch to obj_cgroup through the series.
Thanks, Thomas
/** * mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg * @dmabuf: buffer to uncharge
DMA_HEAP_IOCTL_ALLOC accepts a charge_fd field that, when set, causes the allocation to be charged to an arbitrary process's cgroup rather than the caller's.
Without an access-control point, any process that holds a handle to a dma-heap device node can charge unlimited memory to any other process's cgroup, potentially exhausting that cgroup's limit and triggering OOM kills independent of the victim's own activity or privileges.
Add security_dma_buf_charge_task(), called in mem_cgroup_get_from_fd() when charge_fd refers to another process. The hook receives the credentials of the allocating process (from) and the credentials of the process whose cgroup will be charged (to), giving security modules a controlled enforcement point for cross-cgroup dma-buf attribution policy.
When CONFIG_SECURITY is not set the hook compiles to an inline returning 0, adding no overhead to the fast path.
Signed-off-by: Albert Esteve aesteve@redhat.com --- include/linux/lsm_hook_defs.h | 1 + include/linux/security.h | 7 +++++++ mm/memcontrol.c | 11 +++++++++++ security/security.c | 16 ++++++++++++++++ 4 files changed, 35 insertions(+)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 65c9609ec207..0f1138e16dbe 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -43,6 +43,7 @@ LSM_HOOK(int, 0, capset, struct cred *new, const struct cred *old, const kernel_cap_t *permitted) LSM_HOOK(int, 0, capable, const struct cred *cred, struct user_namespace *ns, int cap, unsigned int opts) +LSM_HOOK(int, 0, dma_buf_charge_task, const struct cred *from, const struct cred *to) LSM_HOOK(int, 0, quotactl, int cmds, int type, int id, const struct super_block *sb) LSM_HOOK(int, 0, quota_on, struct dentry *dentry) LSM_HOOK(int, 0, syslog, int type) diff --git a/include/linux/security.h b/include/linux/security.h index 153e9043058f..89a35eb07b8c 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -350,6 +350,7 @@ int security_capable(const struct cred *cred, struct user_namespace *ns, int cap, unsigned int opts); +int security_dma_buf_charge_task(const struct cred *from, const struct cred *to); int security_quotactl(int cmds, int type, int id, const struct super_block *sb); int security_quota_on(struct dentry *dentry); int security_syslog(int type); @@ -701,6 +702,12 @@ static inline int security_capable(const struct cred *cred, return cap_capable(cred, ns, cap, opts); }
+static inline int security_dma_buf_charge_task(const struct cred *from, + const struct cred *to) +{ + return 0; +} + static inline int security_quotactl(int cmds, int type, int id, const struct super_block *sb) { diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 3e717b5663c8..dd276e1230cc 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -59,6 +59,7 @@ #include <linux/lockdep.h> #include <linux/resume_user_mode.h> #include <linux/psi.h> +#include <linux/security.h> #include <linux/seq_buf.h> #include <linux/sched/isolation.h> #include <linux/kmemleak.h> @@ -5672,11 +5673,13 @@ void mem_cgroup_flush_workqueue(void) struct mem_cgroup *mem_cgroup_get_from_fd(int fd) { struct cgroup_subsys_state *css; + const struct cred *tcred; struct mem_cgroup *memcg; struct cgroup *cgrp; struct task_struct *task; struct mm_struct *mm; unsigned int pidfd_flags; + int ret;
cgrp = cgroup_get_from_fd(fd); if (!IS_ERR(cgrp)) { @@ -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) diff --git a/security/security.c b/security/security.c index 2ee276ab15c5..4a16e4076aab 100644 --- a/security/security.c +++ b/security/security.c @@ -660,6 +660,22 @@ int security_capable(const struct cred *cred, return call_int_hook(capable, cred, ns, cap, opts); }
+/** + * security_dma_buf_charge_task() - Check if cross-cgroup dma-heap charging is allowed + * @from: credentials of the allocating process + * @to: credentials of the process to charge + * + * Check whether the process with credentials @from is allowed to allocate + * dma-heap memory and charge it to the cgroup of the process with credentials + * @to. + * + * Return: Returns 0 if permission is granted. + */ +int security_dma_buf_charge_task(const struct cred *from, const struct cred *to) +{ + return call_int_hook(dma_buf_charge_task, from, to); +} + /** * security_quotactl() - Check if a quotactl() syscall is allowed for this fs * @cmds: commands
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] mm: memcontrol: task cred and mm->owner mismatch in mem_cgroup_get_from_fd() --
commit 8654d1fa67f56b32794f6b84163f6fcd4bd83d11 Author: Albert Esteve aesteve@redhat.com
security: dma-buf: Add dma_buf_charge_task LSM hook
This commit introduces the security_dma_buf_charge_task() LSM hook and calls it in mem_cgroup_get_from_fd() when resolving a target cgroup from a pidfd.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 3e717b5663c82..dd276e1230cc4 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] As reported by Sashiko on v3 [1], in mem_cgroup_get_from_fd(), security_dma_buf_charge_task(current_cred(), tcred) checks the credentials of task obtained via get_task_cred(task), while the charged mem_cgroup is then resolved indirectly via get_task_mm(task) and get_mem_cgroup_from_mm(mm):
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 between get_task_mm(task) and get_mem_cgroup_from_mm(mm), exit_mm() clears task->mm and calls mm_update_next_owner(mm), which sees mm_users > 1 with no live task using mm and sets mm->owner to NULL:
kernel/exit.c:mm_update_next_owner() { ... WRITE_ONCE(mm->owner, NULL); ... }
In that case, get_mem_cgroup_from_mm(mm) falls back to returning root_mem_cgroup, where try_charge() skips accounting because mem_cgroup_is_root(memcg) is true. Could a confined process fork a short-lived helper child with matching credentials to pass security_dma_buf_charge_task() and race its exit to charge or transfer dma-buf allocations to root_mem_cgroup, escaping its 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 may not be task. Could security_dma_buf_charge_task() validate task's credentials while get_mem_cgroup_from_mm(mm) charges the cgroup of a different mm->owner task?
[1] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-3-78d...
Similar to how security_dma_buf_charge_task adds an access-control point for pidfd for the charge_fd of a DMA_HEAP_IOCTL_ALLOC ioctl, we need a new access-control point for the case where charge_fd represents a cgroupfd.
Add security_dma_buf_charge_cgroup(), called in mem_cgroup_get_from_fd() when charge_fd refers to a cgroup. The hook receives the credentials of the requesting process (cred) and the target cgroup to charge (cgrp), giving security modules a controlled enforcement point for cross-cgroup dma-buf attribution policy.
When CONFIG_SECURITY is not set the hook compiles to an inline returning 0, adding no overhead to the fast path.
Signed-off-by: Albert Esteve aesteve@redhat.com --- include/linux/lsm_hook_defs.h | 3 +++ include/linux/security.h | 11 +++++++++++ mm/memcontrol.c | 6 ++++++ security/security.c | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 0f1138e16dbe..fbf7f17127db 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -44,6 +44,9 @@ LSM_HOOK(int, 0, capset, struct cred *new, const struct cred *old, LSM_HOOK(int, 0, capable, const struct cred *cred, struct user_namespace *ns, int cap, unsigned int opts) LSM_HOOK(int, 0, dma_buf_charge_task, const struct cred *from, const struct cred *to) +#ifdef CONFIG_CGROUPS +LSM_HOOK(int, 0, dma_buf_charge_cgroup, const struct cred *cred, struct cgroup *cgrp) +#endif LSM_HOOK(int, 0, quotactl, int cmds, int type, int id, const struct super_block *sb) LSM_HOOK(int, 0, quota_on, struct dentry *dentry) LSM_HOOK(int, 0, syslog, int type) diff --git a/include/linux/security.h b/include/linux/security.h index 89a35eb07b8c..8fb1539156b5 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -351,6 +351,9 @@ int security_capable(const struct cred *cred, int cap, unsigned int opts); int security_dma_buf_charge_task(const struct cred *from, const struct cred *to); +#ifdef CONFIG_CGROUPS +int security_dma_buf_charge_cgroup(const struct cred *cred, struct cgroup *cgrp); +#endif int security_quotactl(int cmds, int type, int id, const struct super_block *sb); int security_quota_on(struct dentry *dentry); int security_syslog(int type); @@ -708,6 +711,14 @@ static inline int security_dma_buf_charge_task(const struct cred *from, return 0; }
+#ifdef CONFIG_CGROUPS +static inline int security_dma_buf_charge_cgroup(const struct cred *cred, + struct cgroup *cgrp) +{ + return 0; +} +#endif + static inline int security_quotactl(int cmds, int type, int id, const struct super_block *sb) { diff --git a/mm/memcontrol.c b/mm/memcontrol.c index dd276e1230cc..4c718c06c122 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5693,7 +5693,13 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd)
memcg = mem_cgroup_from_css(css); rcu_read_unlock(); + + ret = security_dma_buf_charge_cgroup(current_cred(), cgrp); cgroup_put(cgrp); + if (ret) { + mem_cgroup_put(memcg); + return ERR_PTR(ret); + } return memcg; }
diff --git a/security/security.c b/security/security.c index 4a16e4076aab..a5dcf9dc4963 100644 --- a/security/security.c +++ b/security/security.c @@ -676,6 +676,24 @@ int security_dma_buf_charge_task(const struct cred *from, const struct cred *to) return call_int_hook(dma_buf_charge_task, from, to); }
+#ifdef CONFIG_CGROUPS +/** + * security_dma_buf_charge_cgroup() - Check if charging dma-buf memory to a cgroup is allowed + * @cred: credentials of the process requesting the charge + * @cgrp: target cgroup to charge + * + * Check whether the process with credentials @cred is allowed to charge + * dma-buf memory to @cgrp. Called when userspace passes a cgroupfd to + * identify the charge target instead of a pidfd. + * + * Return: Returns 0 if permission is granted. + */ +int security_dma_buf_charge_cgroup(const struct cred *cred, struct cgroup *cgrp) +{ + return call_int_hook(dma_buf_charge_cgroup, cred, cgrp); +} +#endif + /** * security_quotactl() - Check if a quotactl() syscall is allowed for this fs * @cmds: commands
The security_dma_buf_charge_task() hook allows security modules to control which processes may charge dma-buf allocations to another process's cgroup via the charge_fd field of DMA_HEAP_IOCTL_ALLOC. Without a policy implementation, the hook is a no-op and the restriction is not enforced.
On SELinux-managed systems any domain with access to a dma-heap device node can therefore exhaust another cgroup's memory budget without restriction.
Implement selinux_dma_buf_charge_task() using avc_has_perm() with a new dma_buf object class and a charge_task permission. Policy authors can then grant cross-cgroup charging selectively, for example:
allow allocator_app_t client_app_t:dma_buf charge_task;
Signed-off-by: Albert Esteve aesteve@redhat.com --- security/selinux/hooks.c | 7 +++++++ security/selinux/include/classmap.h | 1 + 2 files changed, 8 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 30776b0107f2..8f39d91eabc9 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(); @@ -7674,6 +7680,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(capget, selinux_capget), LSM_HOOK_INIT(capset, selinux_capset), LSM_HOOK_INIT(capable, selinux_capable), + LSM_HOOK_INIT(dma_buf_charge_task, selinux_dma_buf_charge_task), LSM_HOOK_INIT(quotactl, selinux_quotactl), LSM_HOOK_INIT(quota_on, selinux_quota_on), LSM_HOOK_INIT(syslog, selinux_syslog), diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 90cb61b16425..b4ebedc6e424 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 } }, /* last one */ { NULL, {} } };
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
On Thu, Nov 5, 2026 at 12:12 PM sashiko-bot@kernel.org wrote:
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
I held off on this so the core charging proposal could settle first, and then refine the LSM side later / in a separate series depending on the feedback.
For v5 I'm adding a capability baseline in mem_cgroup_get_from_fd() following a Sashiko report, so that charging a memcg outside the caller's own subtree requires CAP_SYS_RESOURCE. With that in place the LSM hooks no longer act as the only check, they add finer-grained policy on top of it. Also, unlike the RFC, they are now called from generic memcg code rather than dma-buf code, so I should have renamed them (and plan to) to security_memcg_charge_{task,cgroup}().
Regarding the broader cgroup model, as far as I can tell, existing cgroup control points are kernfs file operations, which SELinux already covers. The only non-file operations taking a cgroup fd (i.e., BPF attach and perf cgroup events) use their own subsystem classes. Since remote charging in this series consumes the target cgroup's resources, checking against the cgroup's label (as patch 7 already does) sounds like a good fit.
So to Sashiko remark and Paul's comment, rather than a dma-buf specific class, would something like a "cgroup" class with a "charge_to" permission for the cgroupfd path, and a process (process2 really) permission for the pidfd path, be an acceptable first step, with the class growing as other non-file control points come up? I'm happy to adjust the naming or structure to what works best.
BR, Albert
-- Sashiko AI review · https://sashiko.dev/#/patchset/20261104-v2_20230123_tjmercier_google_com-v4-...
The security_dma_buf_charge_cgroup() hook allows security modules to control which processes may charge dma-buf to an arbitrary cgroup via cgroupfd passed in charge_fd. Without a policy implementation, the hook is a no-op and any process that can open a cgroup directory (effectively any process) can charge memory to it.
Implement selinux_dma_buf_charge_cgroup() using avc_has_perm() with the dma_buf object class and a charge_cgroup permission. The target SID is derived from the SELinux label on the cgroup's kernfs directory, so access control follows the cgroup filesystem labeling. Policy authors can then grant cross-cgroup charging selectively, for example:
allow allocator_app_t cgroup_t:dma_buf charge_cgroup;
The hook matches the fail-open semantics of selinux_kernfs_init_security(). This means unlabeled cgroups are unrestricted by default.
Signed-off-by: Albert Esteve aesteve@redhat.com --- security/selinux/hooks.c | 37 +++++++++++++++++++++++++++++++++++++ security/selinux/include/classmap.h | 2 +- 2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 8f39d91eabc9..2f99eaa910af 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; + else if (rc < 0) + return rc; + + clen = (u32)rc; + context = kmalloc(clen, GFP_KERNEL); + if (!context) + return -ENOMEM; + + rc = kernfs_xattr_get(cgrp->kn, XATTR_NAME_SELINUX, context, clen); + if (rc < 0) { + kfree(context); + return rc; + } + + rc = security_context_to_sid(context, rc, &cgrp_sid, GFP_KERNEL); + kfree(context); + if (rc) + return rc; + + return avc_has_perm(cred_sid(cred), cgrp_sid, + SECCLASS_DMA_BUF, DMA_BUF__CHARGE_CGROUP, NULL); +} +#endif + static int selinux_quotactl(int cmds, int type, int id, const struct super_block *sb) { const struct cred *cred = current_cred(); @@ -7681,6 +7715,9 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(capset, selinux_capset), LSM_HOOK_INIT(capable, selinux_capable), LSM_HOOK_INIT(dma_buf_charge_task, selinux_dma_buf_charge_task), +#ifdef CONFIG_CGROUPS + LSM_HOOK_INIT(dma_buf_charge_cgroup, selinux_dma_buf_charge_cgroup), +#endif LSM_HOOK_INIT(quotactl, selinux_quotactl), LSM_HOOK_INIT(quota_on, selinux_quota_on), LSM_HOOK_INIT(syslog, selinux_syslog), diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index b4ebedc6e424..90d1e04c3d27 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -181,7 +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 } }, + { "dma_buf", { "charge_task", "charge_cgroup", NULL } }, /* last one */ { NULL, {} } };
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] selinux: selinux_dma_buf_charge_cgroup() bypasses check on genfscon cgroups - [Medium] selinux: one-off dma_buf charge_cgroup lacks broader cgroup control design --
commit d0dd0ed1dd84ec5c8f4de9a298a749d83668b53e Author: Albert Esteve aesteve@redhat.com
selinux: Restrict dma-buf direct cgroup charging
This commit implements the selinux_dma_buf_charge_cgroup() hook and adds the charge_cgroup permission to the dma_buf security class to restrict which processes can charge dma-buf memory to a target cgroup.
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] As reported by Sashiko on v3 [1], in security/selinux/hooks.c, does returning 0 in selinux_dma_buf_charge_cgroup() when kernfs_xattr_get() returns -ENODATA bypass the permission check for all default-labeled cgroups?
When a process calls ioctl(DMA_HEAP_IOCTL_ALLOC) or ioctl(DMA_BUF_IOCTL_XFER_CHARGE) with a charge_fd referencing a cgroup directory, mem_cgroup_get_from_fd() invokes security_dma_buf_charge_cgroup() -> selinux_dma_buf_charge_cgroup().
In SELinux, cgroup and cgroup2 filesystems are configured with SE_SBGENFS | SE_SBGENFS_XATTR in selinux_set_mnt_opts() and obtain their default label (such as cgroup_t) from policy genfscon rules. When a cgroupfs VFS inode is initialized in inode_doinit_with_dentry(), SELinux computes the default SID via selinux_genfs_get_sid() and only uses kernfs xattrs as an optional override:
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() gets -ENODATA and falls back to def_sid in memory without writing any 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; } ... }
Similarly, selinux_kernfs_init_security() sees -ENODATA on the parent node and returns 0 without setting an xattr on child cgroup nodes, and if the cgroup_seclabel policy capability is not enabled, security.selinux xattrs cannot be set on cgroup directories at all.
Because kernfs_xattr_get(cgrp->kn, XATTR_NAME_SELINUX, NULL, 0) returns -ENODATA for every cgroup that has not been manually relabeled via setxattr(), won't selinux_dma_buf_charge_cgroup() skip avc_has_perm() for all default-labeled cgroup_t cgroups and render policy rules like "allow allocator_app_t cgroup_t:dma_buf charge_cgroup;" ineffective?
[ ... ]
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index b4ebedc6e4240..90d1e04c3d27c 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -181,7 +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 } },
- { "dma_buf", { "charge_task", "charge_cgroup", NULL } },
[Severity: Medium] As reported by Paul Moore on RFC [2], in security/selinux/include/classmap.h (secclass_map) and security/selinux/hooks.c (selinux_dma_buf_charge_cgroup()), this adds a narrow dma_buf class permission for cgroup charging invoked from DMA_HEAP_IOCTL_ALLOC and DMA_BUF_IOCTL_XFER_CHARGE via mem_cgroup_get_from_fd() -> security_dma_buf_charge_cgroup().
Rather than introducing a one-off dma_buf cgroup charging permission, should this be part of a broader SELinux access control design covering existing cgroup control points so that the object class and permission model is consistent across cgroup operations?
[1] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-6-78d... [2] https://lore.kernel.org/r/16093a0278a6d7d1a0a8bc055c228bed@paul-moore.com
Add tests for the new charge_fd field in struct dma_heap_allocation_data.
When the charge_fd feature is absent (unpatched kernel), the probe in pidfd_alloc_supported() detects this and the tests are skipped gracefully.
Add vmtest.sh similar to other subsystem suites, to orchestrate building the selftests (optionally with a freshly compiled kernel) inside a virtme-ng VM, so the tests can be run without modifying the host system. Add a config fragment with required Kconfig symbols.
Also add test_memcg_dmabuf() to the existing test_memcontrol suite to verify end-to-end cross-cgroup accounting: a parent process opens a pidfd for a child in a separate cgroup, allocates a dma-buf via DMA_HEAP_IOCTL_ALLOC with that pidfd, and asserts that memory.stat dmabuf in the child's cgroup reflects the allocation. If the dmabuf key is missing (unpatched kernel) or /dev/dma_heap/system is absent, the test is skipped.
Assisted-by: LLM Signed-off-by: Albert Esteve aesteve@redhat.com --- tools/testing/selftests/cgroup/Makefile | 2 +- tools/testing/selftests/cgroup/test_memcontrol.c | 290 ++++++++++++++++++++- tools/testing/selftests/dmabuf-heaps/config | 1 + tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 227 +++++++++++++++- tools/testing/selftests/dmabuf-heaps/vmtest.sh | 226 ++++++++++++++++ 5 files changed, 742 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile index e01584c2189a..9edfc9f1de5c 100644 --- a/tools/testing/selftests/cgroup/Makefile +++ b/tools/testing/selftests/cgroup/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -Wall -pthread +CFLAGS += -Wall -pthread $(KHDR_INCLUDES)
all: ${HELPER_PROGS}
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c index 0ed82347044e..3c95bcabee2b 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 30
static bool has_localevents; static bool has_recursiveprot; @@ -1764,6 +1774,267 @@ static int test_memcg_inotify_delete_dir(const char *root) return ret; }
+static int memcg_dmabuf_child(const char *cgroup, void *arg) +{ + pause(); + return 0; +} + +/* + * This test allocates a dma-buf via DMA_HEAP_IOCTL_ALLOC with a pidfd + * pointing to a child process in a separate cgroup, then checks that + * memory.stat[dmabuf] in the child's cgroup rises by the allocation size + * and returns to zero after the buffer fd is closed. + */ +static int test_memcg_dmabuf(const char *root) +{ + char *parent = NULL, *child_cg = NULL; + int ret = KSFT_FAIL; + int heap_fd = -1, dmabuf_fd = -1, pidfd = -1; + pid_t child_pid; + int child_status; + long dmabuf_stat; + struct dma_heap_allocation_data alloc = { + .len = ONE_MEG, + .fd_flags = O_RDWR | O_CLOEXEC, + }; + + if (access(DMA_HEAP_SYSTEM, R_OK | W_OK)) { + ret = KSFT_SKIP; + goto cleanup; + } + + parent = cg_name(root, "dmabuf_memcg_test"); + if (!parent) + goto cleanup; + + if (cg_create(parent)) + goto cleanup_parent; + + if (cg_write(parent, "cgroup.subtree_control", "+memory")) + goto cleanup_parent; + + child_cg = cg_name(parent, "child"); + if (!child_cg) + goto cleanup_parent; + + if (cg_create(child_cg)) + goto cleanup_parent; + + child_pid = cg_run_nowait(child_cg, memcg_dmabuf_child, NULL); + if (child_pid < 0) + goto cleanup_child; + + if (cg_wait_for_proc_count(child_cg, 1)) + goto cleanup_kill; + + pidfd = sys_pidfd_open(child_pid, 0); + if (pidfd < 0) { + ret = KSFT_SKIP; + goto cleanup_kill; + } + + heap_fd = open(DMA_HEAP_SYSTEM, O_RDWR); + if (heap_fd < 0) { + ret = KSFT_SKIP; + goto cleanup_pidfd; + } + + alloc.charge_fd = (__u32)pidfd; + if (ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc) < 0) + goto cleanup_heap; + dmabuf_fd = (int)alloc.fd; + + dmabuf_stat = cg_read_key_long(child_cg, "memory.stat", "dmabuf "); + if (dmabuf_stat == -1) { + ret = KSFT_SKIP; + goto cleanup_dmabuf; + } + if (dmabuf_stat != ONE_MEG) + dmabuf_stat = cg_read_key_long_poll(child_cg, "memory.stat", + "dmabuf ", ONE_MEG, + MEMCG_DMABUF_WAIT_RETRIES, + DEFAULT_WAIT_INTERVAL_US); + if (dmabuf_stat != ONE_MEG) { + fprintf(stderr, "Expected dmabuf stat %d, got %ld\n", + ONE_MEG, dmabuf_stat); + goto cleanup_dmabuf; + } + + close(dmabuf_fd); + dmabuf_fd = -1; + + dmabuf_stat = cg_read_key_long_poll(child_cg, "memory.stat", + "dmabuf ", 0, + MEMCG_DMABUF_WAIT_RETRIES, + DEFAULT_WAIT_INTERVAL_US); + if (dmabuf_stat != 0) { + fprintf(stderr, "Expected dmabuf stat 0 after close, got %ld\n", + dmabuf_stat); + goto cleanup_heap; + } + + ret = KSFT_PASS; + +cleanup_dmabuf: + if (dmabuf_fd >= 0) + close(dmabuf_fd); +cleanup_heap: + close(heap_fd); +cleanup_pidfd: + close(pidfd); +cleanup_kill: + kill(child_pid, SIGTERM); + waitpid(child_pid, &child_status, 0); +cleanup_child: + cg_destroy(child_cg); + free(child_cg); +cleanup_parent: + cg_destroy(parent); + free(parent); +cleanup: + return ret; +} + +/* + * Allocate a dma-buf in cgroup A, set memory.max on cgroup B below the + * buffer size, then attempt DMA_BUF_IOCTL_XFER_CHARGE to B. The transfer + * must fail with ENOMEM and the charge must remain in A. + */ +static int test_memcg_dmabuf_xfer_at_limit(const char *root) +{ + char *parent = NULL, *cg_a = NULL, *cg_b = NULL; + int ret = KSFT_FAIL; + int heap_fd = -1, dmabuf_fd = -1, cg_b_fd = -1; + pid_t child_pid; + int child_status; + long stat_a; + struct dma_heap_allocation_data alloc = { + .len = ONE_MEG, + .fd_flags = O_RDWR | O_CLOEXEC, + }; + struct dma_buf_transfer_charge xfer = { 0 }; + + if (access(DMA_HEAP_SYSTEM, R_OK | W_OK)) { + ret = KSFT_SKIP; + goto cleanup; + } + + parent = cg_name(root, "dmabuf_xfer_limit_test"); + if (!parent) + goto cleanup; + + if (cg_create(parent)) + goto cleanup_parent; + + if (cg_write(parent, "cgroup.subtree_control", "+memory")) + goto cleanup_parent; + + cg_a = cg_name(parent, "cg_a"); + cg_b = cg_name(parent, "cg_b"); + if (!cg_a || !cg_b) + goto cleanup_cgs; + + if (cg_create(cg_a) || cg_create(cg_b)) + goto cleanup_cgs; + + if (cg_write(cg_b, "memory.max", "524288")) + goto cleanup_cgs; + + child_pid = cg_run_nowait(cg_a, memcg_dmabuf_child, NULL); + if (child_pid < 0) + goto cleanup_cgs; + + if (cg_wait_for_proc_count(cg_a, 1)) + goto cleanup_kill; + + heap_fd = open(DMA_HEAP_SYSTEM, O_RDWR); + if (heap_fd < 0) { + ret = KSFT_SKIP; + goto cleanup_kill; + } + + alloc.charge_fd = (__u32)sys_pidfd_open(child_pid, 0); + if ((int)alloc.charge_fd < 0) { + ret = KSFT_SKIP; + goto cleanup_heap; + } + + if (ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc) < 0) { + close(alloc.charge_fd); + goto cleanup_heap; + } + close(alloc.charge_fd); + dmabuf_fd = (int)alloc.fd; + + stat_a = cg_read_key_long(cg_a, "memory.stat", "dmabuf "); + if (stat_a == -1) { + ret = KSFT_SKIP; + goto cleanup_dmabuf; + } + if (stat_a != ONE_MEG) + stat_a = cg_read_key_long_poll(cg_a, "memory.stat", + "dmabuf ", ONE_MEG, + MEMCG_DMABUF_WAIT_RETRIES, + DEFAULT_WAIT_INTERVAL_US); + if (stat_a != ONE_MEG) { + fprintf(stderr, "Charge not in A after alloc: expected %d, got %ld\n", + ONE_MEG, stat_a); + goto cleanup_dmabuf; + } + + cg_b_fd = open(cg_b, O_RDONLY | O_DIRECTORY); + if (cg_b_fd < 0) + goto cleanup_dmabuf; + + xfer.charge_fd = (__u32)cg_b_fd; + if (ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer) == 0) { + fprintf(stderr, "xfer_charge should have failed at limit\n"); + goto cleanup_cgfd; + } + if (errno != ENOMEM) { + fprintf(stderr, "Expected ENOMEM, got %s\n", strerror(errno)); + goto cleanup_cgfd; + } + + stat_a = cg_read_key_long_poll(cg_a, "memory.stat", + "dmabuf ", ONE_MEG, + MEMCG_DMABUF_WAIT_RETRIES, + DEFAULT_WAIT_INTERVAL_US); + if (stat_a != ONE_MEG) { + fprintf(stderr, "Charge leaked from A: expected %d, got %ld\n", + ONE_MEG, stat_a); + goto cleanup_cgfd; + } + + ret = KSFT_PASS; + +cleanup_cgfd: + close(cg_b_fd); +cleanup_dmabuf: + if (dmabuf_fd >= 0) + close(dmabuf_fd); +cleanup_heap: + close(heap_fd); +cleanup_kill: + kill(child_pid, SIGTERM); + waitpid(child_pid, &child_status, 0); +cleanup_cgs: + if (cg_a) { + cg_destroy(cg_a); + free(cg_a); + } + if (cg_b) { + cg_destroy(cg_b); + free(cg_b); + } +cleanup_parent: + cg_destroy(parent); + free(parent); +cleanup: + return ret; +} + #define T(x) { x, #x } struct memcg_test { int (*fn)(const char *root); @@ -1785,13 +2056,19 @@ struct memcg_test { T(test_memcg_oom_group_score_events), T(test_memcg_inotify_delete_file), T(test_memcg_inotify_delete_dir), + T(test_memcg_dmabuf), + T(test_memcg_dmabuf_xfer_at_limit), }; #undef T
int main(int argc, char **argv) { char root[PATH_MAX]; - int i, proc_status; + int i, proc_status, count = ARRAY_SIZE(tests); + const char *filter = NULL; + + if (argc > 1) + filter = argv[1];
page_size = sysconf(_SC_PAGE_SIZE); if (page_size <= 0) @@ -1822,8 +2099,17 @@ int main(int argc, char **argv) ksft_exit_skip("Failed to query cgroup mount option\n"); has_localevents = proc_status;
- ksft_set_plan(ARRAY_SIZE(tests)); + if (filter) { + count = 0; + for (i = 0; i < ARRAY_SIZE(tests); i++) { + if (!strcmp(tests[i].name, filter)) + count++; + } + } + ksft_set_plan(count); for (i = 0; i < ARRAY_SIZE(tests); i++) { + if (filter && strcmp(tests[i].name, filter)) + continue; switch (tests[i].fn(root)) { case KSFT_PASS: ksft_test_result_pass("%s\n", tests[i].name); diff --git a/tools/testing/selftests/dmabuf-heaps/config b/tools/testing/selftests/dmabuf-heaps/config index be091f1cdfa0..94c8f33b71a2 100644 --- a/tools/testing/selftests/dmabuf-heaps/config +++ b/tools/testing/selftests/dmabuf-heaps/config @@ -1,3 +1,4 @@ +CONFIG_MEMCG=y CONFIG_DMABUF_HEAPS=y CONFIG_DMABUF_HEAPS_SYSTEM=y CONFIG_DRM_VGEM=y diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c index 1d49df671919..90ef14d2ecba 100644 --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c @@ -3,6 +3,7 @@ #include <dirent.h> #include <errno.h> #include <fcntl.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> #include <stdint.h> @@ -10,11 +11,15 @@ #include <unistd.h> #include <sys/ioctl.h> #include <sys/mman.h> +#include <sys/syscall.h> #include <sys/types.h> +#include <sys/wait.h>
#include <linux/dma-buf.h> #include <linux/dma-heap.h> +#include <linux/limits.h> #include <drm/drm.h> +#include "../pidfd/pidfd.h" #include "kselftest.h"
#define DEVPATH "/dev/dma_heap" @@ -320,6 +325,8 @@ static int dmabuf_heap_alloc_newer(int fd, size_t len, unsigned int flags, __u32 fd; __u32 fd_flags; __u64 heap_flags; + __u32 charge_fd; + __u32 __padding; __u64 garbage1; __u64 garbage2; __u64 garbage3; @@ -328,6 +335,8 @@ static int dmabuf_heap_alloc_newer(int fd, size_t len, unsigned int flags, .fd = 0, .fd_flags = O_RDWR | O_CLOEXEC, .heap_flags = flags, + .charge_fd = 0, + .__padding = 0, .garbage1 = 0xffffffff, .garbage2 = 0x88888888, .garbage3 = 0x11111111, @@ -496,6 +505,218 @@ static void test_alloc_no_fd_leak_on_efault(char *heap_name) ksft_test_result(fd_before == fd_after, "fd leak on EFAULT: before=%d after=%d\n", fd_before, fd_after); + +out: + close(heap_fd); +} + +static int dmabuf_heap_alloc_pidfd(int fd, size_t len, unsigned int heap_flags, + unsigned int charge_fd, int *dmabuf_fd) +{ + struct dma_heap_allocation_data data = { + .len = len, + .fd = 0, + .fd_flags = O_RDWR | O_CLOEXEC, + .heap_flags = heap_flags, + .charge_fd = charge_fd, + }; + int ret; + + if (!dmabuf_fd) + return -EINVAL; + + ret = ioctl(fd, DMA_HEAP_IOCTL_ALLOC, &data); + if (ret < 0) + return ret; + *dmabuf_fd = (int)data.fd; + return ret; +} + +/* + * 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; +} + +/* + * Test: allocate charging the calling process's own cgroup via a self pidfd. + */ +static void test_alloc_pidfd_self(char *heap_name) +{ + int heap_fd = -1, pidfd = -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; + } + + pidfd = sys_pidfd_open(getpid(), 0); + if (pidfd < 0) { + ksft_test_result_skip("pidfd_open not available\n"); + goto out; + } + + ret = dmabuf_heap_alloc_pidfd(heap_fd, ONE_MEG, 0, pidfd, &dmabuf_fd); + ksft_test_result(!ret, "Allocation with self pidfd %d\n", ret); + if (dmabuf_fd >= 0) + close(dmabuf_fd); + close(pidfd); +out: + close(heap_fd); +} + +/* + * Test: allocate charging a child process's cgroup via a child pidfd. + */ +static void test_alloc_pidfd_child(char *heap_name) +{ + int heap_fd = -1, pidfd = -1, dmabuf_fd = -1; + pid_t child_pid; + int status, 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; + } + + child_pid = fork(); + if (child_pid == 0) { + pause(); + _exit(0); + } + if (child_pid < 0) + ksft_exit_fail_msg("fork failed: %s\n", strerror(errno)); + + pidfd = sys_pidfd_open(child_pid, 0); + if (pidfd < 0) { + kill(child_pid, SIGTERM); + waitpid(child_pid, &status, 0); + ksft_test_result_skip("pidfd_open for child failed\n"); + goto out; + } + + ret = dmabuf_heap_alloc_pidfd(heap_fd, ONE_MEG, 0, pidfd, &dmabuf_fd); + ksft_test_result(!ret, "Allocation with child pidfd %d\n", ret); + if (dmabuf_fd >= 0) + close(dmabuf_fd); + close(pidfd); + kill(child_pid, SIGTERM); + waitpid(child_pid, &status, 0); +out: + close(heap_fd); +} + +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); +} + +/* + * 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); + + close(cgroup_fd); + close(dmabuf_fd); out: close(heap_fd); } @@ -530,7 +751,7 @@ int main(void) return KSFT_SKIP; }
- ksft_set_plan(12 * numer_of_heaps()); + ksft_set_plan(16 * numer_of_heaps());
while ((dir = readdir(d))) { if (!strncmp(dir->d_name, ".", 2)) @@ -546,6 +767,10 @@ int main(void) test_alloc_compat(dir->d_name); test_alloc_errors(dir->d_name); test_alloc_no_fd_leak_on_efault(dir->d_name); + test_alloc_pidfd_self(dir->d_name); + test_alloc_pidfd_child(dir->d_name); + test_alloc_cgroupfd_self(dir->d_name); + test_xfer_charge_cgroupfd_self(dir->d_name); } closedir(d);
diff --git a/tools/testing/selftests/dmabuf-heaps/vmtest.sh b/tools/testing/selftests/dmabuf-heaps/vmtest.sh new file mode 100755 index 000000000000..3574b71ca83e --- /dev/null +++ b/tools/testing/selftests/dmabuf-heaps/vmtest.sh @@ -0,0 +1,226 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2026 Red Hat +# +# Dependencies: +# * virtme-ng +# * qemu (used by virtme-ng) + +readonly SCRIPT_DIR="$(cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" +readonly KERNEL_CHECKOUT=$(realpath "${SCRIPT_DIR}"/../../../../) +readonly CGROUP_DIR="${KERNEL_CHECKOUT}/tools/testing/selftests/cgroup" + +source "${SCRIPT_DIR}"/../kselftest/ktap_helpers.sh + +readonly DMABUF_HEAP_TEST="${SCRIPT_DIR}"/dmabuf-heap +readonly MEMCONTROL_TEST="${CGROUP_DIR}"/test_memcontrol +readonly TMP_DIR=$(mktemp -d /tmp/dmabuf-vmtest.XXXXXXXX) + +VERBOSE=false +BUILD=false +BUILD_HOST="" +BUILD_HOST_PODMAN_CONTAINER_NAME="" +SHELL_MODE=false + +usage() { + echo + echo "$0 [OPTIONS]" + echo + echo "Options" + echo " -b: build the kernel from the current source tree and use it for the VM" + echo " -H: hostname for remote build host (used with -b)" + echo " -p: podman container name for remote build host (used with -b)" + echo " Example: -H beefyserver -p vng" + echo " -s: drop into an interactive shell in the VM instead of running tests" + echo " -v: enable verbose vng/qemu output" + echo + + exit 1 +} + +die() { + echo "$*" >&2 + exit "${KSFT_FAIL}" +} + +cleanup() { + rm -rf "${TMP_DIR}" +} + +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 +} + +check_vng() { + local tested_versions=("1.36" "1.37") + local version + local ok=0 + + version="$(vng --version)" + for tv in "${tested_versions[@]}"; do + if [[ "${version}" == *"${tv}"* ]]; then + ok=1 + break + fi + done + + if [[ "${ok}" -eq 0 ]]; then + printf "warning: vng version '%s' has not been tested and may " "${version}" >&2 + printf "not function properly.\n\tThe following versions have been tested: " >&2 + echo "${tested_versions[@]}" >&2 + fi +} + +build_selftests() { + make -C "${KERNEL_CHECKOUT}" headers_install \ + INSTALL_HDR_PATH="${TMP_DIR}/usr" -j"$(nproc)" + + local khdr="-isystem ${TMP_DIR}/usr/include" + + if ! make -C "${SCRIPT_DIR}" KHDR_INCLUDES="${khdr}" -j"$(nproc)"; then + die "failed to build dmabuf-heaps selftests" + fi + + if ! make -C "${CGROUP_DIR}" KHDR_INCLUDES="${khdr}" \ + "${MEMCONTROL_TEST}" -j"$(nproc)"; then + die "failed to build cgroup/test_memcontrol selftest" + fi +} + +handle_build() { + if ! ${BUILD}; then + return + fi + + if [[ ! -d "${KERNEL_CHECKOUT}" ]]; then + echo "-b requires vmtest.sh called from the kernel source tree" >&2 + exit 1 + fi + + pushd "${KERNEL_CHECKOUT}" &>/dev/null + + if ! vng --kconfig --config "${SCRIPT_DIR}/config"; then + die "failed to generate .config for kernel source tree (${KERNEL_CHECKOUT})" + fi + + local vng_args=("-v" "--config" "${SCRIPT_DIR}/config" "--build") + + if [[ -n "${BUILD_HOST}" ]]; then + vng_args+=("--build-host" "${BUILD_HOST}") + fi + + if [[ -n "${BUILD_HOST_PODMAN_CONTAINER_NAME}" ]]; then + vng_args+=("--build-host-exec-prefix" \ + "podman exec -ti ${BUILD_HOST_PODMAN_CONTAINER_NAME}") + fi + + if ! vng "${vng_args[@]}"; then + die "failed to build kernel from source tree (${KERNEL_CHECKOUT})" + fi + + build_selftests + + popd &>/dev/null +} + +make_runner() { + # virtme-ng shares the host filesystem, so TMP_DIR is accessible + # inside the VM at the same absolute path. + cat > "${TMP_DIR}/run_tests.sh" <<-EOF + #!/bin/sh + set -u + + # Enable memory controller for child cgroups + echo +memory > /sys/fs/cgroup/cgroup.subtree_control 2>/dev/null + + PASS=0; FAIL=0; SKIP=0; N=0 + + run() { + name="$1"; shift + N=$((N+1)) + "$@"; rc=$? + if [ $rc -eq 0 ]; then echo "ok $N $name"; PASS=$((PASS+1)) + elif [ $rc -eq 4 ]; then echo "ok $N $name # SKIP"; SKIP=$((SKIP+1)) + else echo "not ok $N $name"; FAIL=$((FAIL+1)) + fi + } + + run "dmabuf-heap charge_fd ioctl" ${DMABUF_HEAP_TEST} + run "memcontrol dma-buf memcg" ${MEMCONTROL_TEST} test_memcg_dmabuf + run "memcontrol dma-buf xfer limit" \ + ${MEMCONTROL_TEST} test_memcg_dmabuf_xfer_at_limit + echo "# PASS=$PASS SKIP=$SKIP FAIL=$FAIL" + [ $FAIL -eq 0 ] + EOF + chmod +x "${TMP_DIR}/run_tests.sh" +} + +run_vm() { + local verbose_opt="" + local kernel_opt="" + + ${VERBOSE} && verbose_opt="--verbose" + + # If we are running from within the kernel source tree, use the kernel + # source tree as the kernel to boot, otherwise use the running kernel. + if [[ "$(realpath "$(pwd)")" == "${KERNEL_CHECKOUT}"* ]]; then + kernel_opt="${KERNEL_CHECKOUT}" + fi + + if ${SHELL_MODE}; then + echo "Starting interactive shell in VM. Exit to stop VM." + vng --run ${kernel_opt} ${verbose_opt} --user root --memory 512M \ + --append "dma_heap.mem_accounting=1" \ + -- bash --noprofile --norc + exit "${KSFT_PASS}" + fi + + vng --run ${kernel_opt} ${verbose_opt} --user root --memory 512M \ + --append "dma_heap.mem_accounting=1" \ + --exec "${TMP_DIR}/run_tests.sh" +} + +while getopts :hvsbH:p: o +do + case $o in + v) VERBOSE=true;; + b) BUILD=true;; + s) SHELL_MODE=true;; + H) BUILD_HOST=$OPTARG;; + p) BUILD_HOST_PODMAN_CONTAINER_NAME=$OPTARG;; + h|*) usage;; + esac +done +shift $((OPTIND-1)) + +trap cleanup EXIT + +check_deps +check_vng +handle_build +make_runner + +echo "Booting VM and running tests..." +run_vm
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [Medium] selftests/dmabuf-heaps: charge_fd=0 and unskipped EOPNOTSUPP in XFER_CHARGE test - [Medium] selftests/dmabuf-heaps: pidfd_alloc_supported() treats EOPNOTSUPP as supported - [Medium] selftests/dmabuf-heaps: missing memory controller check in open_self_cgroupfd() - [Medium] selftests/dmabuf-heaps: check_deps() runs before handle_build() in vmtest.sh - [Low] selftests: cgroup: leak of child_cg on cg_create() failure in test_memcg_dmabuf() --
commit b3c8a8b74499074e19a368c4979733bfe47e9d56 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 and DMA_BUF_IOCTL_XFER_CHARGE in dmabuf-heaps and cgroup/test_memcontrol, along with a virtme-ng runner script and Kconfig fragment.
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c index 0ed82347044ed..3c95bcabee2bd 100644 --- a/tools/testing/selftests/cgroup/test_memcontrol.c +++ b/tools/testing/selftests/cgroup/test_memcontrol.c
[ ... ]
+static int test_memcg_dmabuf(const char *root) +{
[ ... ]
- parent = cg_name(root, "dmabuf_memcg_test");
- if (!parent)
goto cleanup;- if (cg_create(parent))
goto cleanup_parent;- if (cg_write(parent, "cgroup.subtree_control", "+memory"))
goto cleanup_parent;- child_cg = cg_name(parent, "child");
- if (!child_cg)
goto cleanup_parent;- if (cg_create(child_cg))
goto cleanup_parent;
[ ... ]
+cleanup_child:
- cg_destroy(child_cg);
- free(child_cg);
+cleanup_parent:
- cg_destroy(parent);
- free(parent);
+cleanup:
- return ret;
+}
[Severity: Low] Does test_memcg_dmabuf() leak the child_cg string buffer if cg_create(child_cg) fails?
Jumping to cleanup_parent bypasses free(child_cg) under cleanup_child. Should that error path free child_cg (or jump to cleanup_child) before cleaning up parent?
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
[ ... ]
+/*
- 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] As reported by Sashiko on v3 [1], won't pidfd_alloc_supported() return true when CONFIG_MEMCG is disabled?
When CONFIG_MEMCG=n, the inline stub in include/linux/memcontrol.h returns -EOPNOTSUPP for any file descriptor:
static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd) { return ERR_PTR(-EOPNOTSUPP); }
In dma_heap_ioctl_allocate(), that causes DMA_HEAP_IOCTL_ALLOC to fail with ret < 0 and errno == EOPNOTSUPP when passed devnull_fd, so pidfd_alloc_supported() treats the feature as supported and the subsequent tests fail with EOPNOTSUPP instead of skipping.
Should pidfd_alloc_supported() check ret < 0 && errno != EOPNOTSUPP?
[ ... ]
+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);
+}
[Severity: Medium] As reported by Sashiko on v3 [2], what happens in test_alloc_cgroupfd_self() and test_xfer_charge_cgroupfd_self() when the test process runs in a cgroup where the memory controller is not enabled in its parent's cgroup.subtree_control?
In mm/memcontrol.c:mem_cgroup_get_from_fd(), resolving a cgroup directory fd requires an active memory controller CSS on that cgroup and returns -ENOENT otherwise:
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); }
Should open_self_cgroupfd() verify that the memory controller is enabled on cgroup_path (or should the callers skip on ENOENT) so the tests skip rather than fail when run outside vmtest.sh in a non-root cgroup without +memory?
[ ... ]
+/*
- 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] As reported by Sashiko on v3 [3], won't test_xfer_charge_cgroupfd_self() fail with EOPNOTSUPP under default kernel parameters (dma_heap.mem_accounting=0) or on heaps that do not set exp_info.memcg (such as CMA heaps)?
Here the buffer is allocated with dmabuf_heap_alloc(), which passes charge_fd = 0. In 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); }
if (!memcg && mem_accounting) memcg = get_mem_cgroup_from_mm(current->mm);
When charge_fd is 0 and mem_accounting is false (the default), memcg is NULL, and drivers/dma-buf/dma-buf.c:dma_buf_xfer_charge() returns -EOPNOTSUPP when dmabuf->memcg is NULL:
if (!dmabuf->memcg) { ret = -EOPNOTSUPP; goto out; }
Similarly, pidfd_alloc_supported() passes for all heaps because charge_fd is validated in dma_heap_ioctl_allocate() before heap->ops->allocate(), so non-system heaps reach DMA_BUF_IOCTL_XFER_CHARGE with dmabuf->memcg == NULL and also return -EOPNOTSUPP.
Should test_xfer_charge_cgroupfd_self() allocate the buffer with a charge_fd (e.g. via dmabuf_heap_alloc_pidfd()) and skip when DMA_BUF_IOCTL_XFER_CHARGE fails with 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}") ]]; thenecho -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] As reported by Sashiko on v3 [4], 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 compile them?
Should handle_build() run before check_deps() (or should check_deps() skip the binary existence checks when BUILD is true)?
[1] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-8-78d... [2] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-8-78d... [3] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-8-78d... [4] https://lore.kernel.org/r/20261027-v2_20230123_tjmercier_google_com-v3-8-78d...
op-tee@lists.trustedfirmware.org