Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

59 participants
6157 discussions

[PATCH v3 0/8] dma-buf: heaps: Turn heaps into modules

by Maxime Ripard

Hi, The recent introduction of heaps in the optee driver [1] made possible the creation of heaps as modules. It's generally a good idea if possible, including for the already existing system and CMA heaps. The system one is pretty trivial, the CMA one is a bit more involved, especially since we have a call from kernel/dma/contiguous.c to the CMA heap code. This was solved by turning the logic around and making the CMA heap call into the contiguous DMA code. Let me know what you think, Maxime 1: https://lore.kernel.org/dri-devel/20250911135007.1275833-4-jens.wiklander@l… Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Changes in v3: - Squashed cma_get_name and cma_alloc/release patches - Fixed typo in Export dev_get_cma_area commit title - Fixed compilation failure with DMA_CMA but not OF_RESERVED_MEM - Link to v2: https://lore.kernel.org/r/20260227-dma-buf-heaps-as-modules-v2-0-454aee7e06… Changes in v2: - Collect tags - Don't export dma_contiguous_default_area anymore, but export dev_get_cma_area instead - Mentioned that heap modules can't be removed - Link to v1: https://lore.kernel.org/r/20260225-dma-buf-heaps-as-modules-v1-0-2109225a09… --- Maxime Ripard (8): dma: contiguous: Turn heap registration logic around dma: contiguous: Make dev_get_cma_area() a proper function dma: contiguous: Make dma_contiguous_default_area static dma: contiguous: Export dev_get_cma_area() mm: cma: Export cma_alloc(), cma_release() and cma_get_name() dma-buf: heaps: Export mem_accounting parameter dma-buf: heaps: cma: Turn the heap into a module dma-buf: heaps: system: Turn the heap into a module drivers/dma-buf/dma-heap.c | 1 + drivers/dma-buf/heaps/Kconfig | 4 ++-- drivers/dma-buf/heaps/cma_heap.c | 21 +++++---------------- drivers/dma-buf/heaps/system_heap.c | 5 +++++ include/linux/dma-map-ops.h | 18 ++++++++++-------- kernel/dma/contiguous.c | 37 ++++++++++++++++++++++++++++++++++--- mm/cma.c | 3 +++ 7 files changed, 60 insertions(+), 29 deletions(-) --- base-commit: 499a718536dc0e1c1d1b6211847207d58acd9916 change-id: 20260225-dma-buf-heaps-as-modules-1034b3ec9f2a Best regards, -- Maxime Ripard <mripard(a)kernel.org>

1 day, 9 hours

bolt action hunting rifles for sale usa

by Chong Criston

2 days, 4 hours

best deer hunting rifles usa 2026

by Chong Criston

2 days, 4 hours

affordable hunting rifles under $500 usa

by Chong Criston

2 days, 4 hours

best hunting rifles for beginners usa

by Chong Criston

2 days, 4 hours

Re: [PATCH] dma-buf/dma_fence_array: remove unused functionality

by Christian König

On 3/18/26 10:51, Tvrtko Ursulin wrote: > > On 17/03/2026 14:50, Christian König wrote: >> Amdgpu was the only user of the signal on any feature and we dropped >> that use case recently, so we can remove that functionality again. >> >> This allows to simplfy the dma_fence_array code a lot and saves us from > > simplify > >> the need to install a callback on all fences at the same time. >> >> Signed-off-by: Christian König <christian.koenig(a)amd.com> >> --- >> drivers/dma-buf/dma-fence-array.c | 133 +++++++----------- >> drivers/dma-buf/dma-fence-unwrap.c | 3 +- >> drivers/dma-buf/dma-resv.c | 3 +- >> drivers/dma-buf/st-dma-fence-unwrap.c | 2 +- >> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 3 +- >> drivers/gpu/drm/xe/xe_sync.c | 2 +- >> drivers/gpu/drm/xe/xe_vm.c | 4 +- >> include/linux/dma-fence-array.h | 28 +--- >> 8 files changed, 63 insertions(+), 115 deletions(-) >> >> diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c >> index 089f69469524..0d54bf2e47ba 100644 >> --- a/drivers/dma-buf/dma-fence-array.c >> +++ b/drivers/dma-buf/dma-fence-array.c >> @@ -42,97 +42,71 @@ static void dma_fence_array_clear_pending_error(struct dma_fence_array *array) >> cmpxchg(&array->base.error, PENDING_ERROR, 0); >> } >> -static void irq_dma_fence_array_work(struct irq_work *wrk) >> +static void dma_fence_array_cb_func(struct dma_fence *f, >> + struct dma_fence_cb *cb) >> { >> - struct dma_fence_array *array = container_of(wrk, typeof(*array), work); >> + struct dma_fence_array *array = >> + container_of(cb, struct dma_fence_array, callback); >> - dma_fence_array_clear_pending_error(array); >> - >> - dma_fence_signal(&array->base); >> - dma_fence_put(&array->base); >> + irq_work_queue(&array->work); >> } >> -static void dma_fence_array_cb_func(struct dma_fence *f, >> - struct dma_fence_cb *cb) >> +static void irq_dma_fence_array_work(struct irq_work *wrk) >> { >> - struct dma_fence_array_cb *array_cb = >> - container_of(cb, struct dma_fence_array_cb, cb); >> - struct dma_fence_array *array = array_cb->array; >> + struct dma_fence_array *array = container_of(wrk, typeof(*array), work); >> - dma_fence_array_set_pending_error(array, f->error); >> + while (array->num_pending--) { >> + struct dma_fence *f = array->fences[array->num_pending]; >> - if (atomic_dec_and_test(&array->num_pending)) >> - irq_work_queue(&array->work); >> - else >> - dma_fence_put(&array->base); >> + if (!dma_fence_add_callback(f, &array->callback, >> + dma_fence_array_cb_func)) >> + return; >> + >> + dma_fence_array_set_pending_error(array, f->error); >> + } >> + >> + dma_fence_signal(&array->base); >> + dma_fence_put(&array->base); >> } >> static bool dma_fence_array_enable_signaling(struct dma_fence *fence) >> { >> struct dma_fence_array *array = to_dma_fence_array(fence); >> - struct dma_fence_array_cb *cb = array->callbacks; >> - unsigned i; >> - >> - for (i = 0; i < array->num_fences; ++i) { >> - cb[i].array = array; >> - /* >> - * As we may report that the fence is signaled before all >> - * callbacks are complete, we need to take an additional >> - * reference count on the array so that we do not free it too >> - * early. The core fence handling will only hold the reference >> - * until we signal the array as complete (but that is now >> - * insufficient). >> - */ >> - dma_fence_get(&array->base); >> - if (dma_fence_add_callback(array->fences[i], &cb[i].cb, >> - dma_fence_array_cb_func)) { >> - int error = array->fences[i]->error; >> - >> - dma_fence_array_set_pending_error(array, error); >> - dma_fence_put(&array->base); >> - if (atomic_dec_and_test(&array->num_pending)) { >> - dma_fence_array_clear_pending_error(array); >> - return false; >> - } >> - } >> - } >> + /* >> + * As we may report that the fence is signaled before all >> + * callbacks are complete, we need to take an additional >> + * reference count on the array so that we do not free it too >> + * early. The core fence handling will only hold the reference >> + * until we signal the array as complete (but that is now >> + * insufficient). >> + */ >> + dma_fence_get(&array->base); >> + irq_dma_fence_array_work(&array->work); >> return true; >> } >> static bool dma_fence_array_signaled(struct dma_fence *fence) >> { >> struct dma_fence_array *array = to_dma_fence_array(fence); >> - int num_pending; >> + int num_pending, error = 0; >> unsigned int i; >> /* >> - * We need to read num_pending before checking the enable_signal bit >> - * to avoid racing with the enable_signaling() implementation, which >> - * might decrement the counter, and cause a partial check. >> - * atomic_read_acquire() pairs with atomic_dec_and_test() in >> - * dma_fence_array_enable_signaling() >> - * >> - * The !--num_pending check is here to account for the any_signaled case >> - * if we race with enable_signaling(), that means the !num_pending check >> - * in the is_signalling_enabled branch might be outdated (num_pending ( >> - * might have been decremented), but that's fine. The user will get the >> - * right value when testing again later. >> + * Reading num_pending is just an optimization, it is perfectly >> + * acceptable to have a stale value for it. > > Bear with me please. I decided to look at this in the morning and maybe it is too early after all. At first I thought "is this true". Because for example: > > num_fences = 2 > num_pending = 1 > > fences = [ signaled, unsignaled ] > > Below loop exits and function returns array signaled status. > > Then I realised array->num_pending is not the number of unsignaled fences. Instead it seems to be number of fences on which callbacks have not been installed, regardless of the signaled status. Or from a different angle, the next fence index to install the callback on after the current one signals. > > But still the false positive seems possible. After installing the callback on the 2nd fence num_pending will be 1, so the signaled check at a point where the first fence has been signaled, but the second one still has not will return true. Dma_fence_array_signaled() cannot know since it doesn't look at the second fence. Good point, I will rework the handling to take that into account. Thanks for the review, Christian. > > Regards, > > Tvrtko > >> */ >> - num_pending = atomic_read_acquire(&array->num_pending); >> - if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &array->base.flags)) { >> - if (num_pending <= 0) >> - goto signal; >> - return false; >> - } >> + num_pending = READ_ONCE(array->num_pending); >> + for (i = 0; i < num_pending; ++i) { >> + struct dma_fence *f = array->fences[i]; >> - for (i = 0; i < array->num_fences; ++i) { >> - if (dma_fence_is_signaled(array->fences[i]) && !--num_pending) >> - goto signal; >> - } >> - return false; >> + if (!dma_fence_is_signaled(f)) >> + return false; >> -signal: >> + if (!error) >> + error = f->error; >> + } >> + dma_fence_array_set_pending_error(array, error); >> dma_fence_array_clear_pending_error(array); >> return true; >> } >> @@ -171,15 +145,12 @@ EXPORT_SYMBOL(dma_fence_array_ops); >> /** >> * dma_fence_array_alloc - Allocate a custom fence array >> - * @num_fences: [in] number of fences to add in the array >> * >> * Return dma fence array on success, NULL on failure >> */ >> -struct dma_fence_array *dma_fence_array_alloc(int num_fences) >> +struct dma_fence_array *dma_fence_array_alloc(void) >> { >> - struct dma_fence_array *array; >> - >> - return kzalloc_flex(*array, callbacks, num_fences); >> + return kzalloc_obj(struct dma_fence_array); >> } >> EXPORT_SYMBOL(dma_fence_array_alloc); >> @@ -190,21 +161,22 @@ EXPORT_SYMBOL(dma_fence_array_alloc); >> * @fences: [in] array containing the fences >> * @context: [in] fence context to use >> * @seqno: [in] sequence number to use >> - * @signal_on_any: [in] signal on any fence in the array >> * >> * Implementation of @dma_fence_array_create without allocation. Useful to init >> * a preallocated dma fence array in the path of reclaim or dma fence signaling. >> */ >> void dma_fence_array_init(struct dma_fence_array *array, >> int num_fences, struct dma_fence **fences, >> - u64 context, unsigned seqno, >> - bool signal_on_any) >> + u64 context, unsigned seqno) >> { >> static struct lock_class_key dma_fence_array_lock_key; >> WARN_ON(!num_fences || !fences); >> array->num_fences = num_fences; >> + array->num_pending = num_fences; >> + array->fences = fences; >> + array->base.error = PENDING_ERROR; >> dma_fence_init(&array->base, &dma_fence_array_ops, NULL, context, >> seqno); >> @@ -222,11 +194,6 @@ void dma_fence_array_init(struct dma_fence_array *array, >> */ >> lockdep_set_class(&array->base.inline_lock, &dma_fence_array_lock_key); >> - atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences); >> - array->fences = fences; >> - >> - array->base.error = PENDING_ERROR; >> - >> /* >> * dma_fence_array objects should never contain any other fence >> * containers or otherwise we run into recursion and potential kernel >> @@ -249,7 +216,6 @@ EXPORT_SYMBOL(dma_fence_array_init); >> * @fences: [in] array containing the fences >> * @context: [in] fence context to use >> * @seqno: [in] sequence number to use >> - * @signal_on_any: [in] signal on any fence in the array >> * >> * Allocate a dma_fence_array object and initialize the base fence with >> * dma_fence_init(). >> @@ -264,17 +230,16 @@ EXPORT_SYMBOL(dma_fence_array_init); >> */ >> struct dma_fence_array *dma_fence_array_create(int num_fences, >> struct dma_fence **fences, >> - u64 context, unsigned seqno, >> - bool signal_on_any) >> + u64 context, unsigned seqno) >> { >> struct dma_fence_array *array; >> - array = dma_fence_array_alloc(num_fences); >> + array = dma_fence_array_alloc(); >> if (!array) >> return NULL; >> dma_fence_array_init(array, num_fences, fences, >> - context, seqno, signal_on_any); >> + context, seqno); >> return array; >> } >> diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c >> index 07fe9bf45aea..53bb40e70b27 100644 >> --- a/drivers/dma-buf/dma-fence-unwrap.c >> +++ b/drivers/dma-buf/dma-fence-unwrap.c >> @@ -180,8 +180,7 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, >> if (count > 1) { >> result = dma_fence_array_create(count, array, >> - dma_fence_context_alloc(1), >> - 1, false); >> + dma_fence_context_alloc(1), 1); >> if (!result) { >> for (i = 0; i < count; i++) >> dma_fence_put(array[i]); >> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c >> index ce9e6c04897f..39a92d9f2413 100644 >> --- a/drivers/dma-buf/dma-resv.c >> +++ b/drivers/dma-buf/dma-resv.c >> @@ -648,8 +648,7 @@ int dma_resv_get_singleton(struct dma_resv *obj, enum dma_resv_usage usage, >> } >> array = dma_fence_array_create(count, fences, >> - dma_fence_context_alloc(1), >> - 1, false); >> + dma_fence_context_alloc(1), 1); >> if (!array) { >> while (count--) >> dma_fence_put(fences[count]); >> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c >> index 9c74195f47fd..45413666970e 100644 >> --- a/drivers/dma-buf/st-dma-fence-unwrap.c >> +++ b/drivers/dma-buf/st-dma-fence-unwrap.c >> @@ -65,7 +65,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...) >> array = dma_fence_array_create(num_fences, fences, >> dma_fence_context_alloc(1), >> - 1, false); >> + 1); >> if (!array) >> goto error_free; >> return &array->base; >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c >> index e7918f896a26..1ac91a46d87f 100644 >> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c >> @@ -3203,8 +3203,7 @@ eb_composite_fence_create(struct i915_execbuffer *eb, int out_fence_fd) >> fence_array = dma_fence_array_create(eb->num_batches, >> fences, >> eb->context->parallel.fence_context, >> - eb->context->parallel.seqno++, >> - false); >> + eb->context->parallel.seqno++); >> if (!fence_array) { >> kfree(fences); >> return ERR_PTR(-ENOMEM); >> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c >> index 24d6d9af20d6..37866768d64c 100644 >> --- a/drivers/gpu/drm/xe/xe_sync.c >> +++ b/drivers/gpu/drm/xe/xe_sync.c >> @@ -376,7 +376,7 @@ xe_sync_in_fence_get(struct xe_sync_entry *sync, int num_sync, >> xe_assert(vm->xe, current_fence == num_fence); >> cf = dma_fence_array_create(num_fence, fences, >> dma_fence_context_alloc(1), >> - 1, false); >> + 1); >> if (!cf) >> goto err_out; >> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c >> index 548b0769b3ef..b916a9d90104 100644 >> --- a/drivers/gpu/drm/xe/xe_vm.c >> +++ b/drivers/gpu/drm/xe/xe_vm.c >> @@ -3196,7 +3196,7 @@ static struct dma_fence *ops_execute(struct xe_vm *vm, >> goto err_trace; >> } >> - cf = dma_fence_array_alloc(n_fence); >> + cf = dma_fence_array_alloc(); >> if (!cf) { >> fence = ERR_PTR(-ENOMEM); >> goto err_out; >> @@ -3240,7 +3240,7 @@ static struct dma_fence *ops_execute(struct xe_vm *vm, >> xe_assert(vm->xe, current_fence == n_fence); >> dma_fence_array_init(cf, n_fence, fences, dma_fence_context_alloc(1), >> - 1, false); >> + 1); >> fence = &cf->base; >> for_each_tile(tile, vm->xe, id) { >> diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h >> index 370b3d2bba37..3ee55c0e2fa4 100644 >> --- a/include/linux/dma-fence-array.h >> +++ b/include/linux/dma-fence-array.h >> @@ -15,16 +15,6 @@ >> #include <linux/dma-fence.h> >> #include <linux/irq_work.h> >> -/** >> - * struct dma_fence_array_cb - callback helper for fence array >> - * @cb: fence callback structure for signaling >> - * @array: reference to the parent fence array object >> - */ >> -struct dma_fence_array_cb { >> - struct dma_fence_cb cb; >> - struct dma_fence_array *array; >> -}; >> - >> /** >> * struct dma_fence_array - fence to represent an array of fences >> * @base: fence base class >> @@ -33,18 +23,17 @@ struct dma_fence_array_cb { >> * @num_pending: fences in the array still pending >> * @fences: array of the fences >> * @work: internal irq_work function >> - * @callbacks: array of callback helpers >> + * @callback: callback structure for signaling >> */ >> struct dma_fence_array { >> struct dma_fence base; >> - unsigned num_fences; >> - atomic_t num_pending; >> + unsigned int num_fences; >> + unsigned int num_pending; >> struct dma_fence **fences; >> struct irq_work work; >> - >> - struct dma_fence_array_cb callbacks[] __counted_by(num_fences); >> + struct dma_fence_cb callback; >> }; >> /** >> @@ -78,16 +67,13 @@ to_dma_fence_array(struct dma_fence *fence) >> for (index = 0, fence = dma_fence_array_first(head); fence; \ >> ++(index), fence = dma_fence_array_next(head, index)) >> -struct dma_fence_array *dma_fence_array_alloc(int num_fences); >> +struct dma_fence_array *dma_fence_array_alloc(void); >> void dma_fence_array_init(struct dma_fence_array *array, >> int num_fences, struct dma_fence **fences, >> - u64 context, unsigned seqno, >> - bool signal_on_any); >> - >> + u64 context, unsigned seqno); >> struct dma_fence_array *dma_fence_array_create(int num_fences, >> struct dma_fence **fences, >> - u64 context, unsigned seqno, >> - bool signal_on_any); >> + u64 context, unsigned seqno); >> bool dma_fence_match_context(struct dma_fence *fence, u64 context); >> >

2 days, 9 hours

Re: [PATCH] dma-buf/dma_fence: be more defensive in dma_fence_release

by Christian König

Hi Boris, On 3/18/26 10:18, Boris Brezillon wrote: > Hi Christian, > > On Wed, 18 Mar 2026 09:21:34 +0100 > Christian König <christian.koenig(a)amd.com> wrote: > >> On 3/17/26 16:21, Boris Brezillon wrote: >>> On Tue, 17 Mar 2026 15:48:25 +0100 >>> "Christian König" <ckoenig.leichtzumerken(a)gmail.com> wrote: >>> >>>> In case of a refcounting bug dma_fence_release() can be called >>>> before the fence was even signaled. >>>> >>>> Previously the dma_fence framework then force signaled the fence >>>> to make sure to unblock waiters, but that can potentially lead to >>>> random memory corruption when the DMA operation continues. So be >>>> more defensive here and pick the lesser evil. >>>> >>>> Instead of force signaling the fence set an error code on the >>>> fence, re-initialize the refcount to something large and taint the >>>> kernel. >>>> >>>> This will leak memory and eventually can cause a deadlock when the >>>> fence is never signaled, but at least we won't run into an use >>>> after free or random memory corruption. >>>> >>>> Signed-off-by: Christian König <christian.koenig(a)amd.com> >>>> --- >>>> drivers/dma-buf/dma-fence.c | 18 ++++++++++++++---- >>>> 1 file changed, 14 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/dma-buf/dma-fence.c >>>> b/drivers/dma-buf/dma-fence.c index 1826ba73094c..8bf07685a053 >>>> 100644 --- a/drivers/dma-buf/dma-fence.c >>>> +++ b/drivers/dma-buf/dma-fence.c >>>> @@ -593,14 +593,24 @@ void dma_fence_release(struct kref *kref) >>>> /* >>>> * Failed to signal before release, likely a >>>> refcounting issue. * >>>> - * This should never happen, but if it does make >>>> sure that we >>>> - * don't leave chains dangling. We set the error >>>> flag first >>>> - * so that the callbacks know this signal is due >>>> to an error. >>>> + * This should never happen, but if try to be >>>> defensive and take >>>> + * the lesser evil. Initialize the refcount to >>>> something large, >>>> + * but not so large that it can overflow. >>>> + * >>>> + * That will leak memory and could deadlock if >>>> the fence never >>>> + * signals, but at least it doesn't cause an use >>>> after free or >>>> + * random memory corruption. >>>> + * >>>> + * Also taint the kernel to note that it is >>>> rather unreliable to >>>> + * continue. >>>> */ >>>> dma_fence_lock_irqsave(fence, flags); >>>> fence->error = -EDEADLK; >>>> - dma_fence_signal_locked(fence); >>>> + refcount_set(&fence->refcount.refcount, INT_MAX); >>>> >>> >>> I'm not convinced this is useful. If we leak the object, no one >>> should have a ref to release anyway. This does raise a question >>> though. The case we're trying to protect against is fence_callback >>> being registered to this fence and waiting for an event to signal >>> another proxy fence. >> >> Not quite. The real problematic case is that it is necessary to wait >> for a fence to signal with tons of memory management locks held. >> >> So it can be that a simple memory allocation cycles back and depends >> on the fence to signal. >> >>> How can the refcnt drop to zero in that case? Isn't the proxy >>> supposed to own a ref on the fence. Before we go further, I'd like >>> to understand what we're trying to do. >> >> Well we are in C here, so its simply coding errors. An unecessary >> dma_fence_put() in an error path is enough to trigger this. >> >>> The original discussion that led you to write this patch was about >>> detecting when a fence emitter/producer would leave unsignalled >>> fences behind, and the problem we have is when such unsignalled >>> fences have observers waiting for a "signalled" event. If the >>> refcnt drops to zero and the fence is released, we're already >>> passed that point, unfortunately. >> >> Well that is not quite correct. >> >> The most common problem is that we have unbalanced >> dma_fence_get()/dma_fence_put() and we end up in dma_fence_release() >> before the issuer of the dma_fence has a chance to signal it. > > Okay, so that's clearly not solving the problem we were discussing on > [1], I thought it was related. Yeah, correct. The situation on the Rust side is clearly different, you simply doesn't have incorrect refcounting issues there. > Also, I'm still skeptical that we should > try and harden security for a situation that's already covered by > refcount overflow detection. Refcount overflow detection is unfortunately not enabled everywhere and even if it is enabled it doesn't protect against such issues here, it only points them out when it is already to late. > I get why you want to do that, but it > feels like the wrong tool to me. I mean, we wouldn't even see it as > an unbalanced dma_fence_get/put() now that you manually set the refcount > to INT_MAX, which is the bug you're trying to cover for in the first > place. > >> >> See the main purpose of DMA fences is to prevent releasing memory >> back into the core memory management before the DMA operation is >> completed. > > That's a UAF, just a differnt kind (device UAF instead of CPU UAF). Yeah agree completely. The problem is that SW UAF issues are preventable by using something like Rust while HW UAF issues can only be found by an IOMMU and that in turn is disabled more often than not. Especially GPUs and accelerators usually use pass through mode for IOMMU because of both HW bugs as well as performance overhead. > Anyway, my point remains, the root of the issue you're covering for is > a dma_fence UAF (more put()s than get()s, and the CPU still has a ref > on a released dma_fence object). The outcome of this might be device > UAF because of the auto-signalling, but that's still just another > symptom of the dma_fence UAF (with wider consequences, admittedly). > >> >> So when a DMA fence signals to early it means that the HW is still >> writing to that memory but we already potentially re-using the memory >> ending in random memory corruption. > > Yep, I'm well aware of that. > >> >> UAF issues are harmless compared to that. > > That's not what I'm arguing against. What I'm saying is that you just > paper over an issue by messing up with the refcount, and now it's hard > to tell what the root cause is. Completely agree as well. It's not a real solution, but only the lesser evil. Regards, Christian. > > Regards, > > Boris > > [1]https://yhbt.net/lore/all/8bac1559-e139-4a74-a6e8-c2846093db72@amd.com/

3 days, 13 hours

Re: [PATCH] dma-buf/dma_fence: be more defensive in dma_fence_release

by Christian König

On 3/17/26 16:21, Boris Brezillon wrote: > On Tue, 17 Mar 2026 15:48:25 +0100 > "Christian König" <ckoenig.leichtzumerken(a)gmail.com> wrote: > >> In case of a refcounting bug dma_fence_release() can be called before the >> fence was even signaled. >> >> Previously the dma_fence framework then force signaled the fence to make >> sure to unblock waiters, but that can potentially lead to random memory >> corruption when the DMA operation continues. So be more defensive here and >> pick the lesser evil. >> >> Instead of force signaling the fence set an error code on the fence, >> re-initialize the refcount to something large and taint the kernel. >> >> This will leak memory and eventually can cause a deadlock when the fence >> is never signaled, but at least we won't run into an use after free or >> random memory corruption. >> >> Signed-off-by: Christian König <christian.koenig(a)amd.com> >> --- >> drivers/dma-buf/dma-fence.c | 18 ++++++++++++++---- >> 1 file changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c >> index 1826ba73094c..8bf07685a053 100644 >> --- a/drivers/dma-buf/dma-fence.c >> +++ b/drivers/dma-buf/dma-fence.c >> @@ -593,14 +593,24 @@ void dma_fence_release(struct kref *kref) >> /* >> * Failed to signal before release, likely a refcounting issue. >> * >> - * This should never happen, but if it does make sure that we >> - * don't leave chains dangling. We set the error flag first >> - * so that the callbacks know this signal is due to an error. >> + * This should never happen, but if try to be defensive and take >> + * the lesser evil. Initialize the refcount to something large, >> + * but not so large that it can overflow. >> + * >> + * That will leak memory and could deadlock if the fence never >> + * signals, but at least it doesn't cause an use after free or >> + * random memory corruption. >> + * >> + * Also taint the kernel to note that it is rather unreliable to >> + * continue. >> */ >> dma_fence_lock_irqsave(fence, flags); >> fence->error = -EDEADLK; >> - dma_fence_signal_locked(fence); >> + refcount_set(&fence->refcount.refcount, INT_MAX); > > I'm not convinced this is useful. If we leak the object, no one should > have a ref to release anyway. This does raise a question though. The > case we're trying to protect against is fence_callback being registered > to this fence and waiting for an event to signal another proxy fence. Not quite. The real problematic case is that it is necessary to wait for a fence to signal with tons of memory management locks held. So it can be that a simple memory allocation cycles back and depends on the fence to signal. > How can the refcnt drop to zero in that case? Isn't the proxy supposed > to own a ref on the fence. Before we go further, I'd like to understand > what we're trying to do. Well we are in C here, so its simply coding errors. An unecessary dma_fence_put() in an error path is enough to trigger this. > The original discussion that led you to write this patch was about > detecting when a fence emitter/producer would leave unsignalled fences > behind, and the problem we have is when such unsignalled fences have > observers waiting for a "signalled" event. If the refcnt drops to zero > and the fence is released, we're already passed that point, > unfortunately. Well that is not quite correct. The most common problem is that we have unbalanced dma_fence_get()/dma_fence_put() and we end up in dma_fence_release() before the issuer of the dma_fence has a chance to signal it. See the main purpose of DMA fences is to prevent releasing memory back into the core memory management before the DMA operation is completed. So when a DMA fence signals to early it means that the HW is still writing to that memory but we already potentially re-using the memory ending in random memory corruption. UAF issues are harmless compared to that. Regards, Christian. > It can be that: > > - the fence was never exposed -> this is fine > - the fence was exposed but never observed -> this is broken, because if > it had been observed it would have led to a deadlock > - the fence was exposed, observed for some time, but the observer got > bored, stopped waiting and: > * decided to go and execute its stuff anyway -> use-before-ready > situation > * gave up -> kinda okay, but we should still consider the fence > emitter broken > - the fence observer registered a callback but didn't take a ref on the > object -> this is potential UAF on the dma_fence, which can also lead > to a VRAM/system-mem UAF if the emitter drops the dma_fence without > signalling, because of the auto-signal you're getting rid of in this > patch. But the latter is just a side effect of the dma_fence UAF, > which I'm not convinced we should try to protect against. > >> dma_fence_unlock_irqrestore(fence, flags); >> + rcu_read_unlock(); >> + add_taint(TAINT_SOFTLOCKUP, LOCKDEP_STILL_OK); >> + return; >> } >> >> ops = rcu_dereference(fence->ops); >

3 days, 14 hours

Re: [PATCH] dma-buf/udmabuf: skip redundant cpu sync to fix cacheline EEXIST warning

by Christian König

On 3/18/26 06:40, Kasireddy, Vivek wrote: > Hi Mikhail, > >> Subject: [PATCH] dma-buf/udmabuf: skip redundant cpu sync to fix >> cacheline EEXIST warning >> >> When CONFIG_DMA_API_DEBUG_SG is enabled, importing a udmabuf >> into a DRM >> driver (e.g. amdgpu for video playback in GNOME Videos / Showtime) >> triggers a spurious warning: >> >> DMA-API: amdgpu 0000:03:00.0: cacheline tracking EEXIST, \ >> overlapping mappings aren't supported >> WARNING: kernel/dma/debug.c:619 at add_dma_entry+0x473/0x5f0 >> >> The call chain is: >> >> amdgpu_cs_ioctl >> -> amdgpu_ttm_backend_bind >> -> dma_buf_map_attachment >> -> [udmabuf] map_udmabuf -> get_sg_table >> -> dma_map_sgtable(dev, sg, direction, 0) // attrs=0 >> -> debug_dma_map_sg -> add_dma_entry -> EEXIST >> >> This happens because udmabuf builds a per-page scatter-gather list via >> sg_set_folio(). When begin_cpu_udmabuf() has already created an sg >> table mapped for the misc device, and an importer such as amdgpu >> maps >> the same pages for its own device via map_udmabuf(), the DMA debug >> infrastructure sees two active mappings whose physical addresses share >> cacheline boundaries and warns about the overlap. >> >> The DMA_ATTR_SKIP_CPU_SYNC flag suppresses this check in >> add_dma_entry() because it signals that no CPU cache maintenance is >> performed at map/unmap time, making the cacheline overlap harmless. >> >> All other major dma-buf exporters already pass this flag: >> - drm_gem_map_dma_buf() passes DMA_ATTR_SKIP_CPU_SYNC >> - amdgpu_dma_buf_map() passes DMA_ATTR_SKIP_CPU_SYNC >> >> The CPU sync at map/unmap time is also redundant for udmabuf: >> begin_cpu_udmabuf() and end_cpu_udmabuf() already perform explicit >> cache synchronization via dma_sync_sgtable_for_cpu/device() when >> CPU >> access is requested through the dma-buf interface. >> >> Pass DMA_ATTR_SKIP_CPU_SYNC to dma_map_sgtable() and >> dma_unmap_sgtable() in udmabuf to suppress the spurious warning >> and >> skip the redundant sync. >> >> Fixes: 284562e1f348 ("udmabuf: implement >> begin_cpu_access/end_cpu_access hooks") >> Cc: stable(a)vger.kernel.org >> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov(a)gmail.com> >> --- >> drivers/dma-buf/udmabuf.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >> index 94b8ecb892bb..9c6f8785a28a 100644 >> --- a/drivers/dma-buf/udmabuf.c >> +++ b/drivers/dma-buf/udmabuf.c >> @@ -162,7 +162,7 @@ static struct sg_table *get_sg_table(struct device >> *dev, struct dma_buf *buf, >> sg_set_folio(sgl, ubuf->folios[i], PAGE_SIZE, >> ubuf->offsets[i]); >> >> - ret = dma_map_sgtable(dev, sg, direction, 0); >> + ret = dma_map_sgtable(dev, sg, direction, >> DMA_ATTR_SKIP_CPU_SYNC); >> if (ret < 0) >> goto err_map; >> return sg; >> @@ -177,7 +177,7 @@ static struct sg_table *get_sg_table(struct device >> *dev, struct dma_buf *buf, >> static void put_sg_table(struct device *dev, struct sg_table *sg, >> enum dma_data_direction direction) >> { >> - dma_unmap_sgtable(dev, sg, direction, 0); >> + dma_unmap_sgtable(dev, sg, direction, >> DMA_ATTR_SKIP_CPU_SYNC); > Looks OK to me but it would be nice if Christian or someone else can > provide an Ack for this patch. The details of the udmabuf handling is absolutely not my field of expertise. Feel free to add my Acked-by since it obviously seems to fix a bug, but it would be nice if somebody could do an in deep review as well. Regards, Christian. > > Thanks, > Vivek > >> sg_free_table(sg); >> kfree(sg); >> } >> -- >> 2.53.0 >

3 days, 15 hours

Professional blockchain investigation services

by schoumachermathieu8＠gmail.com

When someone loses funds to a fake crypto investment platform, recovery can be difficult but not always impossible. Some people contact services like Vault Traces Recovery experts, which specialize in tracking stolen digital assets. The more information you can provide (transaction hashes, platform details, emails, etc.), the better they can analyze the situation. For assistance, Contact them for more info HomePage > Vaulttrades . c o m WhatApp: +1 (570) 291-8211 Mail: info @ vaulttraces . C o m

3 days, 23 hours

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig