On 11/6/26 14:18, Tao Cui wrote:
> From: Tao Cui <cuitao(a)kylinos.cn>
>
> udmabuf pins the pages of a memfd and hands them out as a dma-buf. The
> data pages keep the accounting of the memfd they were instantiated in,
> but the bookkeeping around them does not: the page pointer array and
> the pinned_folios array, both sized by the buffer page count, plus the
> transient folios array sized by the memfd folio count, are plain
> GFP_KERNEL. A 1GB buffer costs about 4MB of uncharged SUnreclaim
> memory, and size_limit_mb defaults to INT_MAX.
>
> Charge the three kvmalloc allocations so the creator pays for the
> metadata it induces. The system heap in the same subsystem already
> accounts its buffer allocation; the data pages here stay with the
> memfd.
T.J. needs to take a look here, but I fear that this might break uAPI.
A lot of the DMA-buf resources are intentionally not charged against memcg.
Regards,
Christian.
>
> Signed-off-by: Tao Cui <cuitao(a)kylinos.cn>
> ---
> drivers/dma-buf/udmabuf.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index df6dd0046242..40182c82df6b 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -190,11 +190,12 @@ static void unpin_all_folios(struct udmabuf *ubuf)
>
> static __always_inline int init_udmabuf(struct udmabuf *ubuf, pgoff_t pgcnt)
> {
> - ubuf->pages = kvmalloc_objs(*ubuf->pages, pgcnt);
> + ubuf->pages = kvmalloc_objs(*ubuf->pages, pgcnt, GFP_KERNEL_ACCOUNT);
> if (!ubuf->pages)
> return -ENOMEM;
>
> - ubuf->pinned_folios = kvmalloc_objs(*ubuf->pinned_folios, pgcnt);
> + ubuf->pinned_folios = kvmalloc_objs(*ubuf->pinned_folios, pgcnt,
> + GFP_KERNEL_ACCOUNT);
> if (!ubuf->pinned_folios)
> return -ENOMEM;
>
> @@ -387,7 +388,7 @@ static long udmabuf_create(struct miscdevice *device,
> if (ret)
> goto err;
>
> - folios = kvmalloc_objs(*folios, max_nr_folios);
> + folios = kvmalloc_objs(*folios, max_nr_folios, GFP_KERNEL_ACCOUNT);
> if (!folios) {
> ret = -ENOMEM;
> goto err;
> --
> 2.43.0
>
Write permissions on the /dev/dma_heap/* device files are not required
to issue ioctls and allocate dmabufs. Applications should be opening
these file as O_RDONLY. The BPF dmabuf_iter selftest already does
this. [1]
Users are pointing to these selftests as examples of how use dmabuf,
and encountering permission errors on systems where write permissions
are not available on /dev/dma_heap/*. Apply the principle of least
privilege to selftests which open dmabuf heaps by removing the write
access mode and using O_RDONLY for the open() instead.
The same is true for the vgem test using /dev/dri/card.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/too…
Signed-off-by: T.J. Mercier <tjmercier(a)google.com>
---
tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
index fc9694fc4e89..45b420e37c97 100644
--- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
+++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
@@ -48,7 +48,7 @@ static int open_vgem(void)
snprintf(name, 80, "%s%u", drmstr, i);
- fd = open(name, O_RDWR);
+ fd = open(name, O_RDONLY);
if (fd < 0)
continue;
@@ -96,7 +96,7 @@ static int dmabuf_heap_open(char *name)
if (ret < 0)
ksft_exit_fail_msg("snprintf failed! %d\n", ret);
- fd = open(buf, O_RDWR);
+ fd = open(buf, O_RDONLY);
if (fd < 0)
ksft_exit_fail_msg("open %s failed: %s\n", buf, strerror(errno));
base-commit: 9a11db68872055e6ead919bad04d6330851c522d
--
2.55.0.679.g6767b8d81c-goog
On 11/9/26 03:58, Alistair Popple wrote:
> On 2026-11-07 at 19:57 +1100, Karl Mehltretter <kmehltretter(a)gmail.com> wrote...
>> On Thu, Nov 05, 2026 at 05:40:41PM +0100, Alistair Popple wrote:
>>> + * A pin or map that fails after dma_buf_mapping_select() may reopen the
>>> + * choice only if it was the one that made it. A selection that predates the
>>> + * call belongs to a live pin or mapping and must survive, otherwise the next
>>> + * pin or map would reselect under it without an invalidate.
>>> + */
>>> +void dma_buf_mapping_unselect(struct dma_buf_attachment *attach,
>>> + bool was_selected)
>>> +{
>>> + dma_resv_assert_held(attach->dmabuf->resv);
>>> +
>>> + if (was_selected)
>>> + return;
>>> +
>>> + attach->selected = false;
>>> + attach->selected_idx = 0;
>>> +}
>>> +
>>
>> After dma_buf_invalidate_mappings(), attach->selected is false while
>> attach->selected_idx still holds that mapping's entry for a pending
>> unmap.
>>
>> If a pin/map fails before that unmap runs, was_selected is false and
>> dma_buf_mapping_unselect() resets attach->selected_idx to 0.
>>
>> A subsequent dma_buf_unmap_sgt() then uses matches[0].
>
> Actually there's a bigger issue here. The contract at the moment states that
> mapping type may change after invalidation. However that doesn't account for the
> fact mappings can outlive the invalidation.
>
> This could be dealt with by switching everything over to the generic
> dma_buf_mapping_map/unmap() path, but that requires an allocation per mapping
> just to track the mapping type.
You need an allocation per mapping anyway to identify the mapping anyway.
We currently abuse the sg_table for that, but essentially the idea of having a structure/object to represent the mapping needs to stay around as far as I can see.
Regards,
Christian.
>
> The alternate is we update the contract to say mapping type cannot change until
> after all importers have unmapped. Enforcing this would require tracking the
> number of live mappings/pins per-attachment, but I think this is the better
> solution so will add that for the next version.
>
>>> int dma_buf_pin(struct dma_buf_attachment *attach)
>>> {
>>> struct dma_buf *dmabuf = attach->dmabuf;
>>> + bool selected = attach->selected;
>>> int ret = 0;
>>>
>>> WARN_ON(!attach->importer_ops);
>>>
>>> dma_resv_assert_held(dmabuf->resv);
>>>
>>> - if (dmabuf->ops->pin)
>>> + ret = dma_buf_mapping_select(attach);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (dmabuf->ops->pin) {
>>> ret = dmabuf->ops->pin(attach);
>>> + if (ret)
>>> + dma_buf_mapping_unselect(attach, selected);
>>> + }
>>
>> dma_buf_mapping_select() sets attach->selected = true before ops->pin()
>> and the map callbacks run.
>>
>> In patches 14/15, xe/amdgpu migrate a VRAM buffer to system memory
>> during pin and map when the attachment does not accept P2P.
>>
>> That migration calls dma_buf_invalidate_mappings() and clears
>> attach->selected on every attachment, also the one being pinned/mapped.
>>
>> When the callback succeeds, the attachment stays pinned or mapped with
>> attach->selected == false.
>>
>> A later map/pin on the same attachment can then reselect without an
>> invalidation.
>
> Argh. Thanks. Will fix.
>
> - Alistair
>
>> Thanks,
>> Karl
Hi Karl,
We discussed in v1 how this is not the right approach. Why are you
posting a v2 without addressing that ? You need to go back to the design
table.
On Sat, Nov 07, 2026 at 11:47:00AM +0100, Karl Mehltretter wrote:
> uvcvideo and vivid (when using vb2-vmalloc) access imported DMA-BUFs
> through a kernel vmap without cache synchronization. When a non-coherent
> importer such as V3D maps the same DMA-BUF persistently, captured data
> remains in the CPU cache while physical RAM holds stale bytes.
>
> Add an opt-in uses_cpu_access queue flag for vb2-vmalloc queues to
> bracket the interval from buf_prepare() through buf_finish() with
> dma_buf_begin_cpu_access() and dma_buf_end_cpu_access(). Keep this
> opt-in because some vb2-vmalloc drivers pass imported buffers to
> hardware, and end CPU access in process context because DMA-BUF
> callbacks may sleep.
>
> This follows the per-queue flag proposed here:
> https://lore.kernel.org/r/CAAFQd5Am5jBev5P1HmsdeHoJfROZat3bi1W=UsN7wpVqw-XU…
>
> Based on Linux 7.4-rc1 with prerequisite patch "media: videobuf2: finish
> prepared buffers in VIDIOC_REMOVE_BUFS":
> https://lore.kernel.org/r/20261026071735.37220-1-kmehltretter@gmail.com/
>
> Changes in v2:
> - Split patch 1 of v1 into the standalone VIDIOC_REMOVE_BUFS patch above
> and use its __vb2_buf_cancel() helper so queue cancellation and
> VIDIOC_REMOVE_BUFS share the finish path (Tomasz).
> - Scope uses_cpu_access to vb2-vmalloc queues and drop the attach_dmabuf
> dma_dir parameter from vb2_mem_ops, videobuf2-dma-contig,
> videobuf2-dma-sg and videobuf2-vmalloc (Tomasz).
> - Enable uses_cpu_access in vivid only when vivid uses
> vb2_vmalloc_memops (Tomasz).
>
> Link to v1:
> https://lore.kernel.org/r/cover.1792392288.git.kmehltretter@gmail.com/
>
> Testing:
>
> - Raspberry Pi 500+ (6.18.34+rpt-rpi-2712, aarch64, 16 KiB pages,
> Sunplus UVC camera 1bcf:2701 and Broadcom V3D 7.1 GPU), backport of
> the prerequisite patch plus patches 1 and 2: eight 640x480 YUYV frames
> (614,400 B) captured into DMA-BUFs pre-seeded with 0xA5 and imported
> into persistent EGL_LINUX_DMA_BUF_EXT textures before VIDIOC_STREAMON,
> then copied by V3D 7.1 immediately after VIDIOC_DQBUF:
>
> Heap Baseline stale Patched v2 stale
> /dev/dma_heap/linux,cma 8/8 (329,890 B) 0/8 (0 B)
> /dev/dma_heap/system 8/8 (509,818 B) 0/8 (0 B)
>
> - QEMU virt (arm64, Linux 7.4-rc1 with the prerequisite patch and
> patches 1-3, VIDEO_ADV_DEBUG=y, DEBUG_ATOMIC_SLEEP=y): six vivid
> lifecycle cases (stream, streamoff-queued, prepare-reqbufs0,
> prepare-remove, close-streaming, replace-dmabuf) across single-planar
> and multi-planar queues with kprobe counters on
> dma_buf_begin_cpu_access() and dma_buf_end_cpu_access():
>
> Queue begin/end calls Unbalanced Warnings
> vivid vb2-vmalloc (SP) 34 / 34 0 0
> vivid vb2-vmalloc (MP) 34 / 34 0 0
> vivid vb2-dma-contig (SP) 0 / 0 0 0
> vivid vb2-dma-contig (MP) 0 / 0 0 0
> 20x prepare-remove (vmalloc) 20 / 20 0 0
>
> Assisted by an LLM for code changes, test harnesses and commit message
> drafting.
>
> Karl Mehltretter (3):
> media: videobuf2: add opt-in DMA-BUF CPU access
> media: uvcvideo: synchronize imported DMA-BUFs for CPU access
> media: vivid: synchronize imported DMA-BUFs for CPU access
>
> .../media/common/videobuf2/videobuf2-core.c | 76 +++++++++++++++++++
> drivers/media/test-drivers/vivid/vivid-core.c | 4 +-
> drivers/media/usb/uvc/uvc_queue.c | 1 +
> include/media/videobuf2-core.h | 5 ++
> 4 files changed, 85 insertions(+), 1 deletion(-)
>
>
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> prerequisite-patch-id: aa99d5c3c27082cbe003ed94a195e3de415777c9
> --
> 2.39.5 (Apple Git-154)
--
Regards,
Laurent Pinchart
Marc Zyngier <maz(a)kernel.org> writes:
> On Wed, 04 Nov 2026 16:45:55 +0000,
> "Aneesh Kumar K.V (Arm)" <aneesh.kumar(a)kernel.org> wrote:
>>
>> VPE L1 tables are allocated from the CPU-starting callback, where sleeping is
>> not allowed. Remove the page allocation from this atomic path.
>>
>> Use a blocking allocation for the boot CPU and record the resulting table
>> order. Allocate a page of that order from the sleepable CPU hotplug prepare
>> stage for each secondary CPU. Consume the preallocated page only when the CPU
>> cannot inherit an existing redistributor or ITS table.
>>
>> Reclaim unused preallocations after the CPU reaches the online state, and from
>> the prepare-state teardown when CPU bring-up aborts.
>
> No. The way to do it is to move the ITS probing out of the
> non-sleepable CPU hotplug path.
>
> I proposed a potential hack at [1] (which I find far more palatable
> than this proposal), but the real fix is to probe the ITSs as
> devices.
>
Could you share a bit more detail on what you have in mind?
Are you suggesting that we create a platform device for each ITS and
initialize it through the driver's probe callback, while the rest of the
GIC initialization stays in the existing early boot path?
Would that mean the main GIC is operational during early boot, while
ITS-dependent features, such as MSI delivery and direct injection, become
available after the ITS devices probe?
Is that the split you have in mind?
-aneesh
Suzuki K Poulose <suzuki.poulose(a)arm.com> writes:
> On 04/11/2026 16:45, Aneesh Kumar K.V (Arm) wrote:
>> Confidential-computing guests may require memory shared with the host to
>> be aligned and transitioned in units larger than PAGE_SIZE. Several DMA
>> users need struct page-backed allocations satisfying these requirements.
>> Provide a common allocator instead of requiring each user to open-code
>> this sequence.
>>
>> Add alloc_cc_shared_pages() and its node-aware variant. The allocator
>> rounds the requested size to the architecture's shared granule,
>> allocates suitably aligned contiguous pages and transitions the
>> complete range to shared state. It also preserves the caller's GFP
>> policy.
>>
>> A private-to-shared transition may alter memory contents. Mask
>> __GFP_ZERO from the underlying allocation and when requested, clear the
>> complete transitioned range after cc_make_shared() succeeds.
>
> This is not true for pKVM though. It is the other way around. i.e,
> pages must be cleared before transition (e.g., from private to shared)
> to make sure the contents don't leak into the host.
>
> Do we need to handle such cases with a pre_conversion and
> post_conversion hook ?
>
Patch 2 does something similar.
-aneesh
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.co…
[2] https://lpc.events/event/20/contributions/2416/
Signed-off-by: Albert Esteve <aesteve(a)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-78…
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-ef…
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-63…
---
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,
--
Albert Esteve <aesteve(a)redhat.com>
On 11/5/26 15:26, Kirtikumar Anandrao Ramchandani wrote:
> The last dma_fence_put() can drop a sync point's reference count to zero
> before timeline_fence_release() acquires the timeline lock. If
> sync_timeline_signal() takes the lock in that window, dma_fence_get()
> attempts to increment a zero reference count.
>
> Use dma_fence_get_rcu() for the temporary signaling reference and skip a
> point whose reference count has reached zero. The timeline lock serializes
> with timeline_fence_release(), keeping the point allocated while its
> reference count is examined. The release callback removes the point from
> the list and tree after acquiring the lock.
>
> For a live point, retain the temporary reference until after dropping the
> lock, avoiding recursive locking through signaling callbacks.
>
> Reported-by: syzbot+cba1a1f459230395a076(a)syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076
> Signed-off-by: Kirtikumar Anandrao Ramchandani <kirtiar15502(a)gmail.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
I will push this to drm-misc-next soon.
Thanks,
Christian.
> ---
> Changes in v4:
> - Document why the timeline lock makes the conditional reference
> acquisition safe without an RCU read-side critical section.
> - Leave list and rb-tree removal to timeline_fence_release().
> - Drop the Fixes tag because SW_SYNC fixes are not intended for stable.
>
> Changes in v3:
> - Unlink zero-reference points from both indexes while holding the
> timeline lock.
>
> drivers/dma-buf/sw_sync.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 8df20b021..8aa3ed777 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -221,7 +221,12 @@ static void sync_timeline_signal(struct
> sync_timeline *obj, unsigned int inc)
> if (!timeline_fence_signaled(&pt->base))
> break;
>
> - dma_fence_get(&pt->base);
> + /*
> + * The timeline lock prevents timeline_fence_release() from
> + * removing and freeing pt while its reference count is examined.
> + */
> + if (!dma_fence_get_rcu(&pt->base))
> + continue;
>
> list_move_tail(&pt->link, &signalled);
> rb_erase(&pt->node, &obj->pt_tree);
>
> base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
On 11/4/26 23:18, Kirtikumar Anandrao Ramchandani wrote:
> The last dma_fence_put() can drop a sync point's reference count to
> zero before timeline_fence_release() acquires the timeline lock to
> remove it from pt_list. If sync_timeline_signal() acquires the lock in
> that window, dma_fence_get() attempts to increment a zero reference
> count and triggers a refcount warning.
>
> Use dma_fence_get_rcu() to acquire the temporary signaling reference
> only if the fence is still referenced, as sync_pt_create() already
> does when looking up an existing point. Skip a point whose reference
> count has reached zero: its release callback will remove it from the
> list and tree once the timeline lock is released.
>
> The timeline lock prevents the release callback from removing or
> freeing a listed point during this check. For a live point, retain the
> temporary reference until after dropping the lock, preserving the
> protection against recursive locking from signaling callbacks.
Please add this explanation as code comment to the patch.
>
> Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")
Drop that here. SW_SYNC is a unit testing functionality which can intentionally crash the kernel.
You get literally tons of warnings and a tainted kernel when it is enabled/used. So bugs like those are generally not backported.
Apart from that the patch looks good to me,
Christian.
> Reported-by: syzbot+cba1a1f459230395a076(a)syzkaller.appspotmail.com <mailto:syzbot%2Bcba1a1f459230395a076@syzkaller.appspotmail.com>
> Closes: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076 <https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076>
> Signed-off-by: Kirtikumar Anandrao Ramchandani <kirtiar15502(a)gmail.com <mailto:kirtiar15502@gmail.com>>
> ---
>
> drivers/dma-buf/sw_sync.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 8df20b0218a9..7f1eadee4cdf 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -221,7 +221,8 @@
> if (!timeline_fence_signaled(&pt->base))
> break;
>
> - dma_fence_get(&pt->base);
> + if (!dma_fence_get_rcu(&pt->base))
> + continue;
>
> list_move_tail(&pt->link, &signalled);
> rb_erase(&pt->node, &obj->pt_tree);
>
> base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d