Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

129 participants
7046 discussions

by Olga

Hi! I'm curious to know if you pay attention to how your daily habits can impact your health. Nutrition, sleep, stress levels, and recovery are often interrelated. Do you think it's worth starting with lifestyle changes before looking for solutions to maintain hormonal balance?

4 days, 13 hours

[PATCH v2] udmabuf: serialize the sg_table cache under the reservation lock

by Bryam Vargas via B4 Relay

From: Bryam Vargas <hexlabsecurity(a)proton.me> begin_cpu_udmabuf() builds and caches ubuf->sg with an unserialised check-then-set, and end_cpu_udmabuf() reads the same field unlocked. The core invokes both cpu-access hooks without holding the reservation lock and DMA_BUF_IOCTL_SYNC is unlocked, so concurrent SYNC ioctls on a shared udmabuf fd race on ubuf->sg: two begins can both observe NULL and both call get_sg_table(), and the later store orphans the earlier table and its DMA mapping, which release_udmabuf() never frees. Each won race permanently leaks an sg_table and an unbalanced DMA mapping. Serialize both hooks under the buffer's reservation lock, as panfrost and panthor do. Take it interruptibly: the lock can be held across a wait for hardware to finish, so an uninterruptible acquire would park a SYNC ioctl in TASK_UNINTERRUPTIBLE. dma_buf_begin/end_cpu_access() already annotate might_lock() on that lock, so taking it here matches the documented contract. Single-threaded callers are unaffected. Fixes: 284562e1f348 ("udmabuf: implement begin_cpu_access/end_cpu_access hooks") Cc: stable(a)vger.kernel.org Signed-off-by: Bryam Vargas <hexlabsecurity(a)proton.me> --- v2: Take the reservation lock interruptibly (dma_resv_lock_interruptible()) in both hooks instead of the uninterruptible dma_resv_lock(), and return the error; the lock can be held across a wait for hardware to finish, so an uninterruptible acquire could park a SYNC ioctl in TASK_UNINTERRUPTIBLE. With a NULL ww_acquire_ctx the call returns only 0 or -EINTR, so a single error check is enough. (Christian König) v1: https://lore.kernel.org/all/20260625-b4-disp-67d1f3db-v1-1-a47fb9edab9e@pro… Same leak-with-dangling-pointer class as CVE-2024-56712 (export_udmabuf() error path) -- a distinct site the 2024 fix does not cover. udmabuf is the only exporter that lazily builds its sg_table cache inside the cpu-access hook without serialising the check-then-set. The exporters that do comparable in-hook cache work all take a lock first: panfrost and panthor dma_resv_lock() (both hooks), omapdrm omap_obj->lock around its lazy page-get, the dma-heaps buffer->lock, and the TTM/GEM exporters (amdgpu, i915, xe) their object's reservation lock. tegra and videobuf2 take no lock here because they only sync an sg_table built earlier, so there is nothing to serialise. Confirmed with an out-of-tree A/B exercising the begin/begin race: this driver built as a module with get_sg_table()/put_sg_table() counting allocations against frees, driven by a userspace racer that creates 3000 udmabufs and fires DMA_BUF_IOCTL_SYNC(SYNC_START) from N threads on each shared fd. The lock serialises the check-then-set identically whether it is taken interruptibly or not; the run below used the reservation lock: arm leaked sg_tables (of 3000 buffers) vulnerable, 4 threads 4761 control, 1 thread 0 patched (resv lock), 4 threads 0 One sg_table and its DMA mapping leak per won race; the single-thread control does not leak, isolating the race; with the lock the lazy-init runs once per buffer (3000 allocations, zero leaked). end_cpu_udmabuf() is locked for the same field too: an unlocked end could otherwise observe the transient IS_ERR store begin makes before resetting ubuf->sg to NULL, and dereference it. In a tighter 5000-iteration loop the unpatched leak runs around 15-20 MB/s of slab. --- drivers/dma-buf/udmabuf.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index bced421c0d65..d6a137f0de1f 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -226,6 +226,10 @@ static int begin_cpu_udmabuf(struct dma_buf *buf, struct device *dev = ubuf->device->this_device; int ret = 0; + ret = dma_resv_lock_interruptible(buf->resv, NULL); + if (ret) + return ret; + if (!ubuf->sg) { ubuf->sg = get_sg_table(dev, buf, direction); if (IS_ERR(ubuf->sg)) { @@ -238,6 +242,8 @@ static int begin_cpu_udmabuf(struct dma_buf *buf, dma_sync_sgtable_for_cpu(dev, ubuf->sg, direction); } + dma_resv_unlock(buf->resv); + return ret; } @@ -246,12 +252,20 @@ static int end_cpu_udmabuf(struct dma_buf *buf, { struct udmabuf *ubuf = buf->priv; struct device *dev = ubuf->device->this_device; + int ret = 0; + + ret = dma_resv_lock_interruptible(buf->resv, NULL); + if (ret) + return ret; if (!ubuf->sg) - return -EINVAL; + ret = -EINVAL; + else + dma_sync_sgtable_for_device(dev, ubuf->sg, direction); - dma_sync_sgtable_for_device(dev, ubuf->sg, direction); - return 0; + dma_resv_unlock(buf->resv); + + return ret; } static const struct dma_buf_ops udmabuf_ops = { --- base-commit: 7eed1fb17959e721031555e5b5654083fe6a7d02 change-id: 20260625-b4-disp-a9216ef0-d068373aff05 Best regards, -- Bryam Vargas <hexlabsecurity(a)proton.me>

4 days, 13 hours

[PATCH] dma-buf: udmabuf: make list limit unsigned

by Yousef Alhouseen

UDMABUF_CREATE_LIST uses list_limit only as an upper bound for the unsigned entry count supplied by userspace. Negative values have no useful meaning and complicate the bounds check. Make the module parameter unsigned and keep the checked array copy so large counts cannot wrap the allocation size before udmabuf_create() walks the copied list. Signed-off-by: Yousef Alhouseen <alhouseenyousef(a)gmail.com> --- drivers/dma-buf/udmabuf.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index b4078ec84..620113df3 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -16,8 +16,8 @@ #include <linux/vmalloc.h> #include <linux/iosys-map.h> -static int list_limit = 1024; -module_param(list_limit, int, 0644); +static uint list_limit = 1024; +module_param(list_limit, uint, 0644); MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is 1024."); static int size_limit_mb = 64; @@ -469,12 +469,10 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - int limit; if (copy_from_user(&head, (void __user *)arg, sizeof(head))) return -EFAULT; - limit = READ_ONCE(list_limit); - if (!head.count || limit <= 0 || head.count > limit) + if (!head.count || head.count > READ_ONCE(list_limit)) return -EINVAL; list = memdup_array_user((void __user *)(arg + sizeof(head)), head.count, sizeof(*list)); -- 2.54.0

4 days, 13 hours

[PATCH] udmabuf: serialize the sg_table cache under the reservation lock

by Bryam Vargas via B4 Relay

From: Bryam Vargas <hexlabsecurity(a)proton.me> begin_cpu_udmabuf() builds and caches ubuf->sg with an unserialised check-then-set, and end_cpu_udmabuf() reads the same field unlocked. The core invokes both cpu-access hooks without holding the reservation lock and DMA_BUF_IOCTL_SYNC is unlocked, so concurrent SYNC ioctls on a shared udmabuf fd race on ubuf->sg: two begins can both observe NULL and both call get_sg_table(), and the later store orphans the earlier table and its DMA mapping, which release_udmabuf() never frees. Each won race permanently leaks an sg_table and an unbalanced DMA mapping. Serialize both hooks under the buffer's reservation lock, as panfrost and panthor do. dma_buf_begin/end_cpu_access() already annotate might_lock() on that lock, so taking it here matches the documented contract. Single-threaded callers are unaffected. Fixes: 284562e1f348 ("udmabuf: implement begin_cpu_access/end_cpu_access hooks") Cc: stable(a)vger.kernel.org Signed-off-by: Bryam Vargas <hexlabsecurity(a)proton.me> --- Same leak-with-dangling-pointer class as CVE-2024-56712 (export_udmabuf() error path) -- a distinct site the 2024 fix does not cover. udmabuf is the only exporter that lazily builds its sg_table cache inside the cpu-access hook without serialising the check-then-set. The exporters that do comparable in-hook cache work all take a lock first: panfrost and panthor dma_resv_lock() (both hooks), omapdrm omap_obj->lock around its lazy page-get, the dma-heaps buffer->lock, and the TTM/GEM exporters (amdgpu, i915, xe) their object's reservation lock. tegra and videobuf2 take no lock here because they only sync an sg_table built earlier, so there is nothing to serialise. Confirmed with an out-of-tree A/B exercising the begin/begin race: this driver built as a module with get_sg_table()/put_sg_table() counting allocations against frees, driven by a userspace racer that creates 3000 udmabufs and fires DMA_BUF_IOCTL_SYNC(SYNC_START) from N threads on each shared fd. arm leaked sg_tables (of 3000 buffers) vulnerable, 4 threads 4761 control, 1 thread 0 patched (resv lock), 4 threads 0 One sg_table and its DMA mapping leak per won race; the single-thread control does not leak, isolating the race; with the lock the lazy-init runs once per buffer (3000 allocations, zero leaked). end_cpu_udmabuf() is locked for the same field too: an unlocked end could otherwise observe the transient IS_ERR store begin makes before resetting ubuf->sg to NULL, and dereference it. In a tighter 5000-iteration loop the unpatched leak runs around 15-20 MB/s of slab. --- drivers/dma-buf/udmabuf.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index bced421c0d65..702ae13b97d1 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -226,6 +226,8 @@ static int begin_cpu_udmabuf(struct dma_buf *buf, struct device *dev = ubuf->device->this_device; int ret = 0; + dma_resv_lock(buf->resv, NULL); + if (!ubuf->sg) { ubuf->sg = get_sg_table(dev, buf, direction); if (IS_ERR(ubuf->sg)) { @@ -238,6 +240,8 @@ static int begin_cpu_udmabuf(struct dma_buf *buf, dma_sync_sgtable_for_cpu(dev, ubuf->sg, direction); } + dma_resv_unlock(buf->resv); + return ret; } @@ -246,12 +250,18 @@ static int end_cpu_udmabuf(struct dma_buf *buf, { struct udmabuf *ubuf = buf->priv; struct device *dev = ubuf->device->this_device; + int ret = 0; + + dma_resv_lock(buf->resv, NULL); if (!ubuf->sg) - return -EINVAL; + ret = -EINVAL; + else + dma_sync_sgtable_for_device(dev, ubuf->sg, direction); - dma_sync_sgtable_for_device(dev, ubuf->sg, direction); - return 0; + dma_resv_unlock(buf->resv); + + return ret; } static const struct dma_buf_ops udmabuf_ops = { --- base-commit: 7eed1fb17959e721031555e5b5654083fe6a7d02 change-id: 20260625-b4-disp-67d1f3db-0082918fdcb5 Best regards, -- Bryam Vargas <hexlabsecurity(a)proton.me>

4 days, 15 hours

Unlocking Fun, One Block at a Time: A Dive into Block Blast

by loyal.kingfisher.ghlj＠hidingmail.net

In the vast and ever-growing world of mobile gaming, sometimes the simplest concepts deliver the most satisfying experiences. One such gem is Block Blast, a captivating puzzle game that combines the familiar mechanics of Tetris with a fresh, strategic twist. If you're looking for a relaxing yet engaging way to challenge your mind, then Block Blast might just be your next addiction. https://blockblasts.io/ What is Block Blast? Imagine a 10x10 grid, initially empty. Your goal is to fill this grid with various-shaped blocks that appear at the bottom of the screen, one set of three at a time. The catch? You can't rotate the blocks, and you must place all three given blocks before new ones appear. The objective is to clear lines and columns by filling them completely. Once a line or column is full, it disappears, freeing up space for more blocks and earning you points. The game ends when you can no longer place any of the current blocks on the grid. Gameplay: Simple to Grasp, Challenging to Master The beauty of Block Blast lies in its straightforward mechanics. You simply drag and drop the blocks from the bottom onto the grid. There's no time limit, no frantic swiping – just thoughtful placement. However, don't let the simplicity fool you. As the grid fills up, strategic thinking becomes paramount. You'll quickly learn the importance of anticipating future block shapes and planning your placements to create more clearing opportunities. Should you save that long straight piece for a full column clear, or use it now to open up a crucial corner? These are the delightful dilemmas you'll face. The game encourages a flow state, where you’re constantly evaluating and adapting to the evolving grid. Tips for Becoming a Block Blast Master While the game is easy to pick up, a few strategies can significantly boost your scores and enjoyment: Prioritize Clears: Don't be afraid to clear lines or columns, even if it means using a block in a less-than-ideal spot. Clearing space is crucial for longevity. Think Ahead: Always glance at the next set of blocks. This allows you to plan your current placements with future pieces in mind, creating combos and larger clears. Corners are King: Filling corners and edges can be tricky, so try to tackle them early when you have more space. Don't leave isolated blocks in odd spots. The "L" and "T" Block Dilemma: These oddly shaped blocks can be your best friends or worst enemies. Learn how to integrate them into your clears effectively, often by leaving gaps for them. Practice Makes Perfect: Like any puzzle game, consistent play will improve your spatial reasoning and pattern recognition, leading to higher scores. Conclusion Block Blast offers a delightful blend of relaxation and mental stimulation. Its intuitive design makes it accessible to everyone, while its strategic depth keeps players engaged for countless hours. Whether you're looking for a quick brain break or a long, meditative puzzle session, Block Blast provides a satisfying and rewarding experience. Give it a try, and you might just find your new favorite way to unwind and sharpen your mind, one block at a time.

4 days, 18 hours

Legitimate Recovery Support in 2026 - Primatz Guard

by edwardsjosepht33＠gmail.com

Primatz Guard helps victims of crypto, romance, and investment scams trace funds, investigate fraud, and seek recovery with transparent consultation support.

4 days, 23 hours

[PATCH v2] dma-buf: udmabuf: avoid list copy size overflow

by Yousef Alhouseen

UDMABUF_CREATE_LIST copies an array whose element count comes from userspace. The count is bounded by the list_limit module parameter, but that parameter does not need negative values. Make list_limit unsigned so its type matches the u32 count field. Also use memdup_array_user() for the list copy so the element-count multiplication is checked before allocation and copying. Suggested-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Yousef Alhouseen <alhouseenyousef(a)gmail.com> --- Changes in v2: - Make list_limit unsigned as suggested by Christian. - Keep the checked array copy and drop the local u32 byte-count temporary. drivers/dma-buf/udmabuf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index bced421c0..e34a3b135 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -21,8 +21,8 @@ #include <linux/udmabuf.h> #include <linux/vmalloc.h> -static int list_limit = 1024; -module_param(list_limit, int, 0644); +static unsigned int list_limit = 1024; +module_param(list_limit, uint, 0644); MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is 1024."); static int size_limit_mb = 64; module_param(size_limit_mb, int, 0644); @@ -471,12 +471,11 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - u32 lsize; if (copy_from_user(&head, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL; - lsize = sizeof(struct udmabuf_create_item) * head.count; - list = memdup_user((void __user *)(arg + sizeof(head)), lsize); + list = memdup_array_user((void __user *)(arg + sizeof(head)), + head.count, sizeof(*list)); if (IS_ERR(list)) return PTR_ERR(list); -- 2.54.0

5 days, 4 hours

[PATCH] dma-buf: Split sgl by largest page-aligned chunk

by David Hu

Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`. This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the first entry, resulting in non-page-aligned DMA addresses for all subsequent entries. While the underlying IOMMU mapping may be contiguous, hardware DMA engines often require explicit address alignment (e.g., page, cacheline, or storage sector boundaries). Passing unaligned addresses and lengths can cause explicit failures in DMA descriptor creation or silent data corruption if lower unaligned bits are truncated. Fix this by splitting the scatterlist by the largest possible page aligned chunk within `UINT_MAX` (`ALIGN_DOWN(UINT_MAX, PAGE_SIZE)`). This ensures all scatterlist DMA addresses and lengths remain page aligned and satisfy hardware constraints. Page-aligned entries allow the system to cleanly chunk payloads into PCIe MaxPayloadSize (MPS) (e.g., 128 bytes, 256 bytes, 512 bytes). As a result, this may help reduce TLP fragmentation in P2P transfers and alleviate potential congestion within a logical PCIe switch partition, especially when Relaxed Ordering is not possible due to hardware constraints. Reported-by: sashiko-bot <sashiko-bot(a)kernel.org> Closes: https://lore.kernel.org/all/20260609165431.778061F00893@smtp.kernel.org/ Fixes: 3aa31a8bb11e ("dma-buf: provide phys_vec to scatter-gather mapping routine") Cc: stable(a)vger.kernel.org Signed-off-by: David Hu <xuehaohu(a)google.com> --- drivers/dma-buf/dma-buf-mapping.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c index 794acff2546a..f2bde38fdb1f 100644 --- a/drivers/dma-buf/dma-buf-mapping.c +++ b/drivers/dma-buf/dma-buf-mapping.c @@ -5,6 +5,9 @@ */ #include <linux/dma-buf-mapping.h> #include <linux/dma-resv.h> +#include <linux/align.h> + +#define MAX_ENT_SZ ALIGN_DOWN(UINT_MAX, PAGE_SIZE) static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length, dma_addr_t addr) @@ -12,9 +15,9 @@ static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length, unsigned int len, nents; int i; - nents = DIV_ROUND_UP(length, UINT_MAX); + nents = DIV_ROUND_UP(length, MAX_ENT_SZ); for (i = 0; i < nents; i++) { - len = min_t(size_t, length, UINT_MAX); + len = min_t(size_t, length, MAX_ENT_SZ); length -= len; /* * DMABUF abuses scatterlist to create a scatterlist @@ -24,7 +27,7 @@ static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length, * does not require the CPU list for mapping or unmapping. */ sg_set_page(sgl, NULL, 0, 0); - sg_dma_address(sgl) = addr + (dma_addr_t)i * UINT_MAX; + sg_dma_address(sgl) = addr + (dma_addr_t)i * MAX_ENT_SZ; sg_dma_len(sgl) = len; sgl = sg_next(sgl); } @@ -41,14 +44,14 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state, if (!state || !dma_use_iova(state)) { for (i = 0; i < nr_ranges; i++) - nents += DIV_ROUND_UP(phys_vec[i].len, UINT_MAX); + nents += DIV_ROUND_UP(phys_vec[i].len, MAX_ENT_SZ); } else { /* * In IOVA case, there is only one SG entry which spans * for whole IOVA address space, but we need to make sure * that it fits sg->length, maybe we need more. */ - nents = DIV_ROUND_UP(size, UINT_MAX); + nents = DIV_ROUND_UP(size, MAX_ENT_SZ); } return nents; -- 2.55.0.rc0.738.g0c8ab3ebcc-goog

5 days, 12 hours

[PATCH] drm/sched: Protect entity->last_scheduled with spinlock

by Philipp Stanner

The entity->last_scheduled field has always been set and read with special RCU functions in addition to memory barriers. There is no obvious reason for that, since the entity lock is available and taken at all places that evaluate the last_scheduled field. The only exception is drm_sched_entity_error(), which is not performance critical in any way. Improve robustness, readability and maintainability by replacing RCU and barriers with the lock. As a preparational step, while at it, also guard spsc_queue_pop() with the lock, since spsc_queue is deprecated and supposed to be replaced with a locked list. Signed-off-by: Philipp Stanner <phasta(a)kernel.org> --- Tested with drm_sched unit tests, which all ran fine. --- drivers/gpu/drm/scheduler/sched_entity.c | 49 +++++++++++------------- include/drm/gpu_scheduler.h | 9 ++--- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index c51101ec70c1..95b2c48a604a 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -135,7 +135,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, entity->num_sched_list = num_sched_list; entity->sched_list = num_sched_list > 1 ? sched_list : NULL; entity->rq = &sched_list[0]->rq; - RCU_INIT_POINTER(entity->last_scheduled, NULL); RB_CLEAR_NODE(&entity->rb_tree_node); init_completion(&entity->entity_idle); @@ -201,10 +200,10 @@ int drm_sched_entity_error(struct drm_sched_entity *entity) struct dma_fence *fence; int r; - rcu_read_lock(); - fence = rcu_dereference(entity->last_scheduled); + spin_lock(&entity->lock); + fence = entity->last_scheduled; r = fence ? fence->error : 0; - rcu_read_unlock(); + spin_unlock(&entity->lock); return r; } @@ -288,8 +287,10 @@ void drm_sched_entity_kill(struct drm_sched_entity *entity) wait_for_completion(&entity->entity_idle); /* The entity is guaranteed to not be used by the scheduler */ - prev = rcu_dereference_check(entity->last_scheduled, true); + spin_lock(&entity->lock); + prev = entity->last_scheduled; dma_fence_get(prev); + spin_unlock(&entity->lock); while ((job = drm_sched_entity_queue_pop(entity))) { struct drm_sched_fence *s_fence = job->s_fence; @@ -381,8 +382,8 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity) entity->dependency = NULL; } - dma_fence_put(rcu_dereference_check(entity->last_scheduled, true)); - RCU_INIT_POINTER(entity->last_scheduled, NULL); + dma_fence_put(entity->last_scheduled); + WRITE_ONCE(entity->last_scheduled, NULL); drm_sched_entity_stats_put(entity->stats); } EXPORT_SYMBOL(drm_sched_entity_fini); @@ -523,18 +524,18 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity) if (entity->guilty && atomic_read(entity->guilty)) dma_fence_set_error(&sched_job->s_fence->finished, -ECANCELED); - dma_fence_put(rcu_dereference_check(entity->last_scheduled, true)); - rcu_assign_pointer(entity->last_scheduled, - dma_fence_get(&sched_job->s_fence->finished)); + spin_lock(&entity->lock); + dma_fence_put(entity->last_scheduled); + entity->last_scheduled = dma_fence_get(&sched_job->s_fence->finished); - /* - * If the queue is empty we allow drm_sched_entity_select_rq() to - * locklessly access ->last_scheduled. This only works if we set the - * pointer before we dequeue and if we a write barrier here. + /* A recent rework required taking the spinlock above. Since spsc_queue + * is scheduled for removal as per the DRM-TODO-list, we access it here + * locked already to prepare for that cleanup. + * + * TODO: Fully replace spsc_queue with a locked (h)list. */ - smp_wmb(); - spsc_queue_pop(&entity->job_queue); + spin_unlock(&entity->lock); drm_sched_rq_pop_entity(entity); @@ -561,21 +562,15 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) if (spsc_queue_count(&entity->job_queue)) return; - /* - * Only when the queue is empty are we guaranteed that - * drm_sched_run_job_work() cannot change entity->last_scheduled. To - * enforce ordering we need a read barrier here. See - * drm_sched_entity_pop_job() for the other side. - */ - smp_rmb(); - - fence = rcu_dereference_check(entity->last_scheduled, true); + spin_lock(&entity->lock); + fence = entity->last_scheduled; /* stay on the same engine if the previous job hasn't finished */ - if (fence && !dma_fence_is_signaled(fence)) + if (fence && !dma_fence_is_signaled(fence)) { + spin_unlock(&entity->lock); return; + } - spin_lock(&entity->lock); sched = drm_sched_pick_best(entity->sched_list, entity->num_sched_list); rq = sched ? &sched->rq : NULL; if (rq != entity->rq) { diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index d61c19e78182..176ff1f936cd 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -100,7 +100,8 @@ struct drm_sched_entity { * @lock: * * Lock protecting the run-queue (@rq) to which this entity belongs, - * @priority and the list of schedulers (@sched_list, @num_sched_list). + * @priority, @last_scheduled and the list of schedulers (@sched_list, + * @num_sched_list). */ spinlock_t lock; @@ -202,11 +203,9 @@ struct drm_sched_entity { /** * @last_scheduled: * - * Points to the finished fence of the last scheduled job. Only written - * by drm_sched_entity_pop_job(). Can be accessed locklessly from - * drm_sched_job_arm() if the queue is empty. + * Points to the finished fence of the last scheduled job. */ - struct dma_fence __rcu *last_scheduled; + struct dma_fence *last_scheduled; /** * @last_user: last group leader pushing a job into the entity. base-commit: 60b5fa6edfef867322fce7c8306e5c4b46211be7 -- 2.54.0

5 days, 13 hours

[PATCH] drm/drm_crtc: fix race with dma_fence_signal() in ::get_driver_name()

by André Draszik

Since commit 541c8f2468b9 ("dma-buf: detach fence ops on signal v3"), I'm seeing the BUG_ON() triggering in drm_crtc's fence_to_crtc() via drm_crtc_fence_get_driver_name() regularly: Call trace: panic+0x58/0x5c die+0x160/0x178 bug_brk_handler+0x70/0xa4 call_el1_break_hook+0x3c/0x1a0 do_el1_brk64+0x24/0x74 el1_brk64+0x34/0x54 el1h_64_sync_handler+0x80/0xfc el1h_64_sync+0x84/0x88 drm_crtc_fence_get_driver_name+0x60/0x68 (P) sync_file_get_name+0x184/0x45c sync_file_ioctl+0x404/0xf70 __arm64_sys_ioctl+0x124/0x1dc This looks to be caused by a code flow similar to the following: +++ snip +++ thread A thread B ioctl(SYNC_IOC_FILE_INFO) sync_file_ioctl() sync_file_get_name() dma_fence_signal_timestamp_locked() dma_fence_driver_name() ops = rcu_dereference(fence->ops) if (!dma_fence_test_signaled_flag()) ops->get_driver_name(fence) i.e. drm_crtc_fence_get_driver_name() test_and_set_bit(SIGNALED) RCU_INIT_POINTER(fence->ops, NULL) drm_crtc_fence_get_driver_name() BUG_ON(rcu_access_pointer(fence->ops) != &drm_crtc_fence_ops) +++ snap +++ I see two ways to resolve this: a) simply drop the BUG_ON(). It can not work anymore since above commit, as it is racy now. b) pass the original 'ops' pointer obtained in dma_fence_driver_name() to all callees. This patch implements option a), as because: * I don't see much benefit in passing the extra pointer just for this BUG_ON() to work. * Requiring the dma_fence_ops in those callbacks is an implementation detail of the drm_crtc driver, and therefore upper layers shouldn't have to care about that. * The existence of the BUG_ON() doesn't appear to be consistent with implementations of ::get_driver_name() or ::get_timeline_name() in the majority of other DRM drivers in the first place. Those that do have a similar BUG_ON() (i915, xe) probably also need an update similar to this patch here but I'm not in a position to test those. Note that the adjacent drm_crtc_fence_get_timeline_name() has the same problem and is fixed by this patch as well. Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3") Signed-off-by: André Draszik <andre.draszik(a)linaro.org> --- drivers/gpu/drm/drm_crtc.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 63ead8ba6756..31c8636e7467 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -73,6 +73,9 @@ * &drm_mode_config_funcs.atomic_check. */ +#define fence_to_crtc(f) container_of((f)->extern_lock, \ + struct drm_crtc, fence_lock) + /** * drm_crtc_from_index - find the registered CRTC at an index * @dev: DRM device @@ -154,14 +157,6 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc) #endif } -static const struct dma_fence_ops drm_crtc_fence_ops; - -static struct drm_crtc *fence_to_crtc(struct dma_fence *fence) -{ - BUG_ON(rcu_access_pointer(fence->ops) != &drm_crtc_fence_ops); - return container_of(fence->extern_lock, struct drm_crtc, fence_lock); -} - static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence) { struct drm_crtc *crtc = fence_to_crtc(fence); --- base-commit: e2cae00c05d196491c318196792297f2dfbaa02c change-id: 20260618-linux-drm_crtc_fix2-23a7c354a412 Best regards, -- André Draszik <andre.draszik(a)linaro.org>

5 days, 19 hours

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig