All of lore.kernel.org
 help / color / mirror / Atom feed
* [syzbot] [media?] [dri?] kernel BUG in sync_timeline_signal
@ 2026-11-04  6:38 syzbot
  2026-11-05  4:14 ` [PATCH] dma-buf/sw_sync: Optimize the retrieval of fence reference counts Edward Adam Davis
  0 siblings, 1 reply; 12+ messages in thread
From: syzbot @ 2026-11-04  6:38 UTC (permalink / raw)
  To: dri-devel, linux-kernel, linux-media, syzkaller-bugs

Hello,

syzbot found the following issue on:

HEAD commit:    6628e7d4c6b2 Linux 7.4-rc1
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=13fa63f1580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=1d2c97eac7d33533
dashboard link: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=134d2959580000

AI-generated root cause summary:
In drivers/dma-buf/sw_sync.c, a race condition exists between dma_fence_put()
and sync_timeline_signal(). When the last reference to a sync_pt fence is
dropped, its refcount reaches 0 before dma_fence_release() ->
timeline_fence_release() acquires obj->lock to remove the sync_pt from
obj->pt_list. If sync_timeline_signal() runs concurrently under obj->lock, it
iterates obj->pt_list and calls dma_fence_get() on the fence whose refcount is
already 0, violating refcount invariants and causing a use-after-free / refcount
addition-on-0 kernel crash.

full analysis:  https://syzkaller.appspot.com/x/ai_root_cause?x=cba1a1f459230395a076

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+cba1a1f459230395a076@syzkaller.appspotmail.com

refcount_t: addition on 0; use-after-free.
------------[ cut here ]------------
kernel BUG at lib/refcount.c:34!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
CPU: 1 UID: 0 PID: 6018 Comm: syz-executor369 Tainted: G             L      syzkaller #0 PREEMPT(full) 
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/15/2026
RIP: 0010:refcount_warn_saturate.cold+0x12/0x14 lib/refcount.c:34
Code: 89 ea be 01 00 00 00 48 c7 c7 40 c1 c3 90 e8 0a 5a 01 00 e9 72 e3 02 04 e8 c0 54 d7 00 48 c7 c7 a0 4a 81 8c e8 04 85 eb ff 90 <0f> 0b e8 ac 54 d7 00 48 c7 c7 40 4b 81 8c e8 f0 84 eb ff 90 0f 0b
RSP: 0018:ffffc90003e57cb8 EFLAGS: 00010086
RAX: 000000000000002a RBX: ffff88802d271570 RCX: 0000000000000000
RDX: 000000000000002a RSI: ffffffff81ebf219 RDI: fffff520007caf88
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
R10: ffffc90003e57a7f R11: 0000000000000001 R12: 0000000000000000
R13: dffffc0000000000 R14: ffff88802d271300 R15: ffffc90003e57d18
FS:  00007f83e74d26c0(0000) GS:ffff88812257d000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f83e75a93c8 CR3: 00000000357c8000 CR4: 00000000003526f0
Call Trace:
 <TASK>
 __refcount_add include/linux/refcount.h:289 [inline]
 __refcount_inc include/linux/refcount.h:366 [inline]
 refcount_inc include/linux/refcount.h:383 [inline]
 kref_get include/linux/kref.h:45 [inline]
 dma_fence_get include/linux/dma-fence.h:323 [inline]
 sync_timeline_signal+0x6d9/0x8a0 drivers/dma-buf/sw_sync.c:224
 sw_sync_ioctl_inc drivers/dma-buf/sw_sync.c:405 [inline]
 sw_sync_ioctl+0x7b4/0x13f0 drivers/dma-buf/sw_sync.c:470
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:597 [inline]
 __se_sys_ioctl fs/ioctl.c:583 [inline]
 __x64_sys_ioctl+0x18e/0x210 fs/ioctl.c:583
 do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
 do_syscall_64+0x123/0x790 arch/x86/entry/syscall_64.c:84
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f83e7507d5b
Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
RSP: 002b:00007f83e74d2100 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f83e7507d5b
RDX: 00007f83e74d2170 RSI: 0000000040045701 RDI: 0000000000000003
RBP: 00000000000003d0 R08: 0000000000000000 R09: 00007f83e74d26c0
R10: 0000000000000008 R11: 0000000000000246 R12: 00007f83e74d2170
R13: 00000000000000f3 R14: 00007ffcce841440 R15: 00007ffcce841528
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:refcount_warn_saturate.cold+0x12/0x14 lib/refcount.c:34
Code: 89 ea be 01 00 00 00 48 c7 c7 40 c1 c3 90 e8 0a 5a 01 00 e9 72 e3 02 04 e8 c0 54 d7 00 48 c7 c7 a0 4a 81 8c e8 04 85 eb ff 90 <0f> 0b e8 ac 54 d7 00 48 c7 c7 40 4b 81 8c e8 f0 84 eb ff 90 0f 0b
RSP: 0018:ffffc90003e57cb8 EFLAGS: 00010086
RAX: 000000000000002a RBX: ffff88802d271570 RCX: 0000000000000000
RDX: 000000000000002a RSI: ffffffff81ebf219 RDI: fffff520007caf88
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
R10: ffffc90003e57a7f R11: 0000000000000001 R12: 0000000000000000
R13: dffffc0000000000 R14: ffff88802d271300 R15: ffffc90003e57d18
FS:  00007f83e74d26c0(0000) GS:ffff88812257d000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f83e75a93c8 CR3: 00000000357c8000 CR4: 00000000003526f0


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-04  6:38 [syzbot] [media?] [dri?] kernel BUG in sync_timeline_signal syzbot
@ 2026-11-05  4:14 ` Edward Adam Davis
  2026-11-05  4:30   ` sashiko-bot
  2026-11-05  9:53   ` [PATCH] " Christian König
  0 siblings, 2 replies; 12+ messages in thread
From: Edward Adam Davis @ 2026-11-05  4:14 UTC (permalink / raw)
  To: syzbot+cba1a1f459230395a076
  Cc: dri-devel, linux-kernel, linux-media, syzkaller-bugs,
	sumit.semwal, christian.koenig, phasta, robin.clark

There is a race condition between closing a sync file and the SW_SYNC_IOC_INC
operation on a sync obj. When a fence is created via SW_SYNC_IOC_CREATE_FENCE,
its reference count is initialized to 1. When the sync file is closed, the
fence's reference count drops to 0. A race occurs if SW_SYNC_IOC_INC accesses
the fence whose ref count has already reached 0 before dma_fence_release()
calls timeline_fence_release() to erase the corresponding pt_tree node from
the sync object, potentially hitting:

refcount_t: addition on 0; use-after-free.
------------[ cut here ]------------
kernel BUG at lib/refcount.c:34!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
Call Trace:
 kref_get include/linux/kref.h:45 [inline]
 dma_fence_get include/linux/dma-fence.h:323 [inline]
 sync_timeline_signal+0x6d9/0x8a0 drivers/dma-buf/sw_sync.c:224
 sw_sync_ioctl_inc drivers/dma-buf/sw_sync.c:405 [inline]
 sw_sync_ioctl+0x7b4/0x13f0 drivers/dma-buf/sw_sync.c:470

Use kref_get_unless_zero() to optimize dma_get_fence(), in order to avoid
acquiring an object with a reference count of 0.

Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")
Reported-by: syzbot+cba1a1f459230395a076@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076
Signed-off-by: Edward Adam Davis <eadavis@sina.com>
---
 drivers/dma-buf/sw_sync.c | 3 ++-
 include/linux/dma-fence.h | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 8df20b0218a9..ad60ad5e5cd9 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -221,7 +221,8 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
 		if (!timeline_fence_signaled(&pt->base))
 			break;
 
-		dma_fence_get(&pt->base);
+		if (!dma_fence_get(&pt->base))
+			continue;
 
 		list_move_tail(&pt->link, &signalled);
 		rb_erase(&pt->node, &obj->pt_tree);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index dd07d128adc3..4d885c8e1d4f 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -320,8 +320,9 @@ static inline void dma_fence_put(struct dma_fence *fence)
 static inline struct dma_fence *dma_fence_get(struct dma_fence *fence)
 {
 	if (fence)
-		kref_get(&fence->refcount);
-	return fence;
+		if (kref_get_unless_zero(&fence->refcount))
+			return fence;
+	return NULL;
 }
 
 /**
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05  4:14 ` [PATCH] dma-buf/sw_sync: Optimize the retrieval of fence reference counts Edward Adam Davis
@ 2026-11-05  4:30   ` sashiko-bot
  2026-11-05  5:27     ` [PATCH v2] " Edward Adam Davis
  2026-11-05  9:53   ` [PATCH] " Christian König
  1 sibling, 1 reply; 12+ messages in thread
From: sashiko-bot @ 2026-11-05  4:30 UTC (permalink / raw)
  To: Edward Adam Davis; +Cc: dri-devel, media-ci

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dma-buf: dma_fence_get() uses kref_get_unless_zero() breaking API contract
- [High] dma-buf/sw_sync: sync_timeline_signal() leaves zero-refcount pt linked
--

commit 6ff0958843bbbad778cdc3d38a038557ac00bd49
Author: Edward Adam Davis <eadavis@sina.com>

dma-buf/sw_sync: Optimize the retrieval of fence reference counts

This commit changes dma_fence_get() to use kref_get_unless_zero() and updates
sync_timeline_signal() to skip fences when dma_fence_get() returns NULL. This
is intended to avoid a refcount addition-on-zero when racing with fence release
in sw_sync.

> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 8df20b0218a9e..ad60ad5e5cd9e 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -221,7 +221,8 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
>  		if (!timeline_fence_signaled(&pt->base))
>  			break;
>  
> -		dma_fence_get(&pt->base);
> +		if (!dma_fence_get(&pt->base))
> +			continue;

[Severity: High]
When acquiring a reference to pt->base fails because its refcount has already
dropped to zero on another CPU, is it safe to continue without unlinking pt
from obj->pt_list and obj->pt_tree?

Continuing here leaves a fence with pt->base.seqno <= obj->value linked at the
head of obj->pt_list and in obj->pt_tree until timeline_fence_release()
acquires obj->lock:

drivers/dma-buf/sw_sync.c:timeline_fence_release() {
	...
	dma_fence_lock_irqsave(fence, flags);
	if (!list_empty(&pt->link)) {
		list_del(&pt->link);
		rb_erase(&pt->node, &parent->pt_tree);
	}
	dma_fence_unlock_irqrestore(fence, flags);
	...
}

Before timeline_fence_release() takes obj->lock, leaving this stale entry
linked can cause several regressions:

1. In sw_sync_ioctl_inc(), increments larger than INT_MAX call
sync_timeline_signal(obj, INT_MAX) in a loop (or consecutive SW_SYNC_IOC_INC
calls may run):

	while (value > INT_MAX)  {
		sync_timeline_signal(obj, INT_MAX);
		value -= INT_MAX;
	}

	sync_timeline_signal(obj, value);

If obj->value advances by more than INT_MAX past the stale pt->base.seqno at
the head of obj->pt_list, the signed difference in
timeline_fence_signaled() -> __dma_fence_is_later() wraps positive.
timeline_fence_signaled(&pt->base) then returns false at the stale head entry,
causing sync_timeline_signal() to break out of the loop early without
signaling active fences later in obj->pt_list.

2. In sync_pt_create(), rbtree insertion assumes all nodes in obj->pt_tree
have seqno in (obj->value, obj->value + INT_MAX]:

			parent = *p;
			other = rb_entry(parent, typeof(*pt), node);
			cmp = value - other->base.seqno;

If a stale pt with seqno <= obj->value remains in obj->pt_tree and a new
unsignaled fence is created with value in (pt->base.seqno + INT_MAX,
obj->value + INT_MAX], cmp overflows a signed 32-bit int to a negative value.
That inserts the new fence to the left of the stale pt and ahead of earlier
active fences in obj->pt_list, corrupting the list and tree ordering even
after timeline_fence_release() later removes the stale pt.

3. If sync_info_debugfs_show() -> sync_print_obj() -> sync_print_fence() runs
while the skipped pt is still on obj->pt_list, dma_fence_get_status_locked()
calls dma_fence_is_signaled_locked(&pt->base), which sees
timeline_fence_signaled(&pt->base) is true and sets
DMA_FENCE_FLAG_SIGNALED_BIT on pt->base. If sw_sync_debugfs_release() then
runs before timeline_fence_release() acquires obj->lock:

	list_for_each_entry_safe(pt, next, &obj->pt_list, link) {
		dma_fence_set_error(&pt->base, -ENOENT);
		dma_fence_signal_locked(&pt->base);
	}

dma_fence_set_error() triggers WARN_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
&fence->flags)).

Since timeline_fence_release() checks !list_empty(&pt->link) under obj->lock,
should sync_timeline_signal() unlink pt with list_del_init(&pt->link) and
rb_erase(&pt->node, &obj->pt_tree) before continuing when acquiring the fence
reference fails?

>  
>  		list_move_tail(&pt->link, &signalled);
>  		rb_erase(&pt->node, &obj->pt_tree);
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index dd07d128adc3b..4d885c8e1d4f5 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -320,8 +320,9 @@ static inline void dma_fence_put(struct dma_fence *fence)
>  static inline struct dma_fence *dma_fence_get(struct dma_fence *fence)
>  {
>  	if (fence)
> -		kref_get(&fence->refcount);
> -	return fence;
> +		if (kref_get_unless_zero(&fence->refcount))
> +			return fence;
> +	return NULL;
>  }

[Severity: High]
Does changing dma_fence_get() to use kref_get_unless_zero() break the
dma_fence_get() API contract across the kernel?

dma_fence_get() is meant for callers that already hold a valid reference to a
fence, and its kernel-doc comment right above states that it returns the same
fence with its refcount increased by 1.

Replacing kref_get() with kref_get_unless_zero() here suppresses refcount_t
addition-on-zero warnings and saturation when a caller has a genuine
use-after-free bug. Because dma_fence_get() is not annotated with
__must_check, callers that do not check its return value (such as
dma_resv_add_fence(), dma_buf_poll(), dma_fence_array_enable_signaling(),
dma_fence_chain_enable_signaling(), drm_syncobj_add_point(), and
drm_syncobj_replace_fence()) will silently fail to take a reference and later
underflow the refcount in dma_fence_put(). Other callers that pass a non-NULL
fence and dereference or pass the return value to container_of() (such as
i915_request_get(), sync_file_create(), dma_resv_replace_fences(), and
__dma_fence_unwrap_merge()) can unexpectedly receive NULL.

include/linux/dma-fence.h already provides dma_fence_get_rcu() right below
dma_fence_get() for acquiring a reference when the refcount might be zero:

static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
{
	if (kref_get_unless_zero(&fence->refcount))
		return fence;
	else
		return NULL;
}

And drivers/dma-buf/sw_sync.c already uses dma_fence_get_rcu() in
sync_pt_create() for this exact situation:

			} else {
				if (dma_fence_get_rcu(&other->base)) {
					sync_timeline_put(obj);
					kfree(pt);
					pt = other;
					goto unlock;
				}
				p = &parent->rb_left;
			}

Should dma_fence_get() be left unchanged and sync_timeline_signal() call
dma_fence_get_rcu(&pt->base) instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105041446.51979-1-eadavis@sina.com?part=1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05  4:30   ` sashiko-bot
@ 2026-11-05  5:27     ` Edward Adam Davis
  2026-11-05  7:49       ` Philipp Stanner
  2026-11-05 10:03       ` Christian König
  0 siblings, 2 replies; 12+ messages in thread
From: Edward Adam Davis @ 2026-11-05  5:27 UTC (permalink / raw)
  To: sashiko-bot
  Cc: media-ci, sashiko-reviews, dri-devel, linux-kernel, linux-media,
	syzkaller-bugs, sumit.semwal, christian.koenig, phasta,
	robin.clark

There is a race condition between closing a sync file and the SW_SYNC_IOC_INC
operation on a sync obj. When a fence is created via SW_SYNC_IOC_CREATE_FENCE,
its reference count is initialized to 1. When the sync file is closed, the
fence's reference count drops to 0. A race occurs if SW_SYNC_IOC_INC accesses
the fence whose ref count has already reached 0 before dma_fence_release()
calls timeline_fence_release() to erase the corresponding pt_tree node from
the sync object, potentially hitting:

refcount_t: addition on 0; use-after-free.
------------[ cut here ]------------
kernel BUG at lib/refcount.c:34!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
Call Trace:
 kref_get include/linux/kref.h:45 [inline]
 dma_fence_get include/linux/dma-fence.h:323 [inline]
 sync_timeline_signal+0x6d9/0x8a0 drivers/dma-buf/sw_sync.c:224
 sw_sync_ioctl_inc drivers/dma-buf/sw_sync.c:405 [inline]
 sw_sync_ioctl+0x7b4/0x13f0 drivers/dma-buf/sw_sync.c:470

Use dma_fence_get_rcu() to replace dma_get_fence(), if fence reference
count is 0 then do like timeline fence release things: remove pt link
and erase pt node.

Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")
Reported-by: syzbot+cba1a1f459230395a076@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076
Signed-off-by: Edward Adam Davis <eadavis@sina.com>
---
v1 -> v2: use dma_fence_get_rcu() and remove link & erase pt node

 drivers/dma-buf/sw_sync.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 8df20b0218a9..261bc98b1ed6 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -221,7 +221,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
 		if (!timeline_fence_signaled(&pt->base))
 			break;
 
-		dma_fence_get(&pt->base);
+		if (!dma_fence_get_rcu(&pt->base)) {
+			list_del_init(&pt->link);
+			rb_erase(&pt->node, &obj->pt_tree);
+			continue;
+		}
 
 		list_move_tail(&pt->link, &signalled);
 		rb_erase(&pt->node, &obj->pt_tree);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05  5:27     ` [PATCH v2] " Edward Adam Davis
@ 2026-11-05  7:49       ` Philipp Stanner
  2026-11-05  9:35         ` Edward Adam Davis
  2026-11-05 10:03       ` Christian König
  1 sibling, 1 reply; 12+ messages in thread
From: Philipp Stanner @ 2026-11-05  7:49 UTC (permalink / raw)
  To: Edward Adam Davis, sashiko-bot
  Cc: media-ci, sashiko-reviews, dri-devel, linux-kernel, linux-media,
	syzkaller-bugs, sumit.semwal, christian.koenig, phasta,
	robin.clark

On Thu, 2026-11-05 at 13:27 +0800, Edward Adam Davis wrote:
> ---
> v1 -> v2: use dma_fence_get_rcu() and remove link & erase pt node

Ideally, one avoids sending new versions within 24 hours :)

> 
>  drivers/dma-buf/sw_sync.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 8df20b0218a9..261bc98b1ed6 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -221,7 +221,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
>  		if (!timeline_fence_signaled(&pt->base))
>  			break;
>  
> -		dma_fence_get(&pt->base);
> +		if (!dma_fence_get_rcu(&pt->base)) {

You must hold the rcu_read_lock() while doing that, don't you?

This probably works "by coincidence" since the surrounding spinlock
also disables preemption, like the rcu lock does.

That spinlock, why can't it guard the dma_fence reference? Doesn't obj-
>pt_list hold its own reference?



P.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05  7:49       ` Philipp Stanner
@ 2026-11-05  9:35         ` Edward Adam Davis
  2026-11-05 11:21           ` Philipp Stanner
  0 siblings, 1 reply; 12+ messages in thread
From: Edward Adam Davis @ 2026-11-05  9:35 UTC (permalink / raw)
  To: phasta
  Cc: media-ci, sashiko-reviews, dri-devel, linux-kernel, linux-media,
	syzkaller-bugs, sumit.semwal, christian.koenig, phasta,
	robin.clark

On Thu, 05 Nov 2026 08:49:49 +0100, Philipp Stanner wrote:
> > 
> >  drivers/dma-buf/sw_sync.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> > index 8df20b0218a9..261bc98b1ed6 100644
> > --- a/drivers/dma-buf/sw_sync.c
> > +++ b/drivers/dma-buf/sw_sync.c
> > @@ -221,7 +221,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
> >  		if (!timeline_fence_signaled(&pt->base))
> >  			break;
> >  
> > -		dma_fence_get(&pt->base);
> > +		if (!dma_fence_get_rcu(&pt->base)) {
> 
> You must hold the rcu_read_lock() while doing that, don't you?
dma_fence_get_rcu() is simply a wrapper around kref_get_unless_zero(),
it has nothing to do with RCU. For instance, the use of dma_fence_get_rcu()
within sync_pt_create(), as seen in the SW_SYNC_IOC_CREATE_FENCE code path
does not require an RCU lock.
> 
> This probably works "by coincidence" since the surrounding spinlock
> also disables preemption, like the rcu lock does.
> 
> That spinlock, why can't it guard the dma_fence reference? Doesn't obj-
> >pt_list hold its own reference?
This is neither a coincidence nor related to any outer spinlock.
The comments I included in my patch provide a complete explanation of
the fence's reference counting, from the moment of its creation to its
final destruction.

BR,
Edward

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05  4:14 ` [PATCH] dma-buf/sw_sync: Optimize the retrieval of fence reference counts Edward Adam Davis
  2026-11-05  4:30   ` sashiko-bot
@ 2026-11-05  9:53   ` Christian König
  1 sibling, 0 replies; 12+ messages in thread
From: Christian König @ 2026-11-05  9:53 UTC (permalink / raw)
  To: Edward Adam Davis, syzbot+cba1a1f459230395a076
  Cc: dri-devel, linux-kernel, linux-media, syzkaller-bugs,
	sumit.semwal, phasta, robin.clark

On 11/5/26 05:14, Edward Adam Davis wrote:
> There is a race condition between closing a sync file and the SW_SYNC_IOC_INC
> operation on a sync obj. When a fence is created via SW_SYNC_IOC_CREATE_FENCE,
> its reference count is initialized to 1. When the sync file is closed, the
> fence's reference count drops to 0. A race occurs if SW_SYNC_IOC_INC accesses
> the fence whose ref count has already reached 0 before dma_fence_release()
> calls timeline_fence_release() to erase the corresponding pt_tree node from
> the sync object, potentially hitting:
> 
> refcount_t: addition on 0; use-after-free.
> ------------[ cut here ]------------
> kernel BUG at lib/refcount.c:34!
> Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> Call Trace:
>  kref_get include/linux/kref.h:45 [inline]
>  dma_fence_get include/linux/dma-fence.h:323 [inline]
>  sync_timeline_signal+0x6d9/0x8a0 drivers/dma-buf/sw_sync.c:224
>  sw_sync_ioctl_inc drivers/dma-buf/sw_sync.c:405 [inline]
>  sw_sync_ioctl+0x7b4/0x13f0 drivers/dma-buf/sw_sync.c:470
> 
> Use kref_get_unless_zero() to optimize dma_get_fence(), in order to avoid
> acquiring an object with a reference count of 0.
> 
> Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")
> Reported-by: syzbot+cba1a1f459230395a076@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076
> Signed-off-by: Edward Adam Davis <eadavis@sina.com>
> ---
>  drivers/dma-buf/sw_sync.c | 3 ++-
>  include/linux/dma-fence.h | 5 +++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 8df20b0218a9..ad60ad5e5cd9 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -221,7 +221,8 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
>                 if (!timeline_fence_signaled(&pt->base))
>                         break;
> 
> -               dma_fence_get(&pt->base);
> +               if (!dma_fence_get(&pt->base))
> +                       continue;
> 
>                 list_move_tail(&pt->link, &signalled);
>                 rb_erase(&pt->node, &obj->pt_tree);
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index dd07d128adc3..4d885c8e1d4f 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -320,8 +320,9 @@ static inline void dma_fence_put(struct dma_fence *fence)
>  static inline struct dma_fence *dma_fence_get(struct dma_fence *fence)
>  {
>         if (fence)
> -               kref_get(&fence->refcount);
> -       return fence;
> +               if (kref_get_unless_zero(&fence->refcount))
> +                       return fence;
> +       return NULL;

Well we should have an automated filter function for nonsense like that.

First of all we have an dma_fence_get_rcu() function which uses kref_get_unless_zero() for this use case, so modifying the dma_fence_get() function is a clear no-go.

Then SW_SYNC is a testing functionality which can literally be used to intentionally crash the kernel with it! There is a massive warning and tainting the kernel when you try to use it.

So a race condition like that one here is completely negligible to this and so there is no justification for any stable backport whatsoever. We could of course fix it to improve the documentation purpose of the code, but that's about it.

And all of this is either obvious by reading the three lines of code below the ones you modified and/or the documentation of the component.

So please before you send out anything like that take a look around and thing for a few seconds if that really makes sense or is just a waste of time.

Regards,
Christian.

>  }
> 
>  /**
> --
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05  5:27     ` [PATCH v2] " Edward Adam Davis
  2026-11-05  7:49       ` Philipp Stanner
@ 2026-11-05 10:03       ` Christian König
  2026-11-05 12:37         ` Edward Adam Davis
  2026-11-05 13:55         ` Danilo Krummrich
  1 sibling, 2 replies; 12+ messages in thread
From: Christian König @ 2026-11-05 10:03 UTC (permalink / raw)
  To: Edward Adam Davis, sashiko-bot
  Cc: media-ci, sashiko-reviews, dri-devel, linux-kernel, linux-media,
	syzkaller-bugs, sumit.semwal, phasta, robin.clark

On 11/5/26 06:27, Edward Adam Davis wrote:
> There is a race condition between closing a sync file and the SW_SYNC_IOC_INC
> operation on a sync obj. When a fence is created via SW_SYNC_IOC_CREATE_FENCE,
> its reference count is initialized to 1. When the sync file is closed, the
> fence's reference count drops to 0. A race occurs if SW_SYNC_IOC_INC accesses
> the fence whose ref count has already reached 0 before dma_fence_release()
> calls timeline_fence_release() to erase the corresponding pt_tree node from
> the sync object, potentially hitting:
> 
> refcount_t: addition on 0; use-after-free.
> ------------[ cut here ]------------
> kernel BUG at lib/refcount.c:34!
> Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> Call Trace:
>  kref_get include/linux/kref.h:45 [inline]
>  dma_fence_get include/linux/dma-fence.h:323 [inline]
>  sync_timeline_signal+0x6d9/0x8a0 drivers/dma-buf/sw_sync.c:224
>  sw_sync_ioctl_inc drivers/dma-buf/sw_sync.c:405 [inline]
>  sw_sync_ioctl+0x7b4/0x13f0 drivers/dma-buf/sw_sync.c:470
> 
> Use dma_fence_get_rcu() to replace dma_get_fence(), if fence reference
> count is 0 then do like timeline fence release things: remove pt link
> and erase pt node.
> 
> Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")

Please completely drop that. This is not something which has any justification to be backported.

> Reported-by: syzbot+cba1a1f459230395a076@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076
> Signed-off-by: Edward Adam Davis <eadavis@sina.com>
> ---
> v1 -> v2: use dma_fence_get_rcu() and remove link & erase pt node
> 
>  drivers/dma-buf/sw_sync.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 8df20b0218a9..261bc98b1ed6 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -221,7 +221,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
>                 if (!timeline_fence_signaled(&pt->base))
>                         break;
> 
> -               dma_fence_get(&pt->base);
> +               if (!dma_fence_get_rcu(&pt->base)) {

Well that is at least better than before but still needs a code comment explaining why this is possible without holding the RCU read side lock.

Something like /* It's possible to race with timeline_fence_release() here */.

> +                       list_del_init(&pt->link);
> +                       rb_erase(&pt->node, &obj->pt_tree);

What happens is that you race with timeline_fence_release(), so that here is superfluous and potentially even harmful.

Regards,
Christian.

> +                       continue;
> +               }
> 
>                 list_move_tail(&pt->link, &signalled);
>                 rb_erase(&pt->node, &obj->pt_tree);
> --
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05  9:35         ` Edward Adam Davis
@ 2026-11-05 11:21           ` Philipp Stanner
  0 siblings, 0 replies; 12+ messages in thread
From: Philipp Stanner @ 2026-11-05 11:21 UTC (permalink / raw)
  To: Edward Adam Davis
  Cc: media-ci, sashiko-reviews, dri-devel, linux-kernel, linux-media,
	syzkaller-bugs, sumit.semwal, christian.koenig, phasta,
	robin.clark

On Thu, 2026-11-05 at 17:35 +0800, Edward Adam Davis wrote:
> On Thu, 05 Nov 2026 08:49:49 +0100, Philipp Stanner wrote:
> > > 
> > >  drivers/dma-buf/sw_sync.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> > > index 8df20b0218a9..261bc98b1ed6 100644
> > > --- a/drivers/dma-buf/sw_sync.c
> > > +++ b/drivers/dma-buf/sw_sync.c
> > > @@ -221,7 +221,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
> > >  		if (!timeline_fence_signaled(&pt->base))
> > >  			break;
> > >  
> > > -		dma_fence_get(&pt->base);
> > > +		if (!dma_fence_get_rcu(&pt->base)) {
> > 
> > You must hold the rcu_read_lock() while doing that, don't you?
> dma_fence_get_rcu() is simply a wrapper around kref_get_unless_zero(),
> it has nothing to do with RCU. For instance, the use of dma_fence_get_rcu()
> within sync_pt_create(), as seen in the SW_SYNC_IOC_CREATE_FENCE code path
> does not require an RCU lock.

How come the function you're using here literally says in its docu that
you need RCU?

/**
 * dma_fence_get_rcu - get a fence from a dma_resv_list with
 *                     rcu read lock
 * @fence: fence to increase refcount of
 *
 * Function returns NULL if no refcount could be obtained, or the fence.
 */
static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
{
	if (kref_get_unless_zero(&fence->refcount))
		return fence;
	else
		return NULL;
}


The rcu_read_lock() protects your kref_get_unless_zero() from running
into UAF if refcount == 0.

That's why dma_fence_free() uses kfree_rcu(). Your read-side-lock
prevents kfree() from running so that you can safely read the refcount
== 0 from memory.

If we think about it, having kref_get_unless_zero() without RCU doesn't
even make sense, because refcount == 0 must mean that the object is now
supposed to be freed. That's the point of refcounting.

Or am I missing something?

> 

P.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05 10:03       ` Christian König
@ 2026-11-05 12:37         ` Edward Adam Davis
  2026-11-05 13:55         ` Danilo Krummrich
  1 sibling, 0 replies; 12+ messages in thread
From: Edward Adam Davis @ 2026-11-05 12:37 UTC (permalink / raw)
  To: christian.koenig
  Cc: media-ci, sashiko-reviews, dri-devel, linux-kernel, linux-media,
	syzkaller-bugs, sumit.semwal, phasta, robin.clark

On Thu, 5 Nov 2026 11:03:10 +0100, Christian König wrote:
> > There is a race condition between closing a sync file and the SW_SYNC_IOC_INC
> > operation on a sync obj. When a fence is created via SW_SYNC_IOC_CREATE_FENCE,
> > its reference count is initialized to 1. When the sync file is closed, the
> > fence's reference count drops to 0. A race occurs if SW_SYNC_IOC_INC accesses
> > the fence whose ref count has already reached 0 before dma_fence_release()
> > calls timeline_fence_release() to erase the corresponding pt_tree node from
> > the sync object, potentially hitting:
> >
> > refcount_t: addition on 0; use-after-free.
> > ------------[ cut here ]------------
> > kernel BUG at lib/refcount.c:34!
> > Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> > Call Trace:
> >  kref_get include/linux/kref.h:45 [inline]
> >  dma_fence_get include/linux/dma-fence.h:323 [inline]
> >  sync_timeline_signal+0x6d9/0x8a0 drivers/dma-buf/sw_sync.c:224
> >  sw_sync_ioctl_inc drivers/dma-buf/sw_sync.c:405 [inline]
> >  sw_sync_ioctl+0x7b4/0x13f0 drivers/dma-buf/sw_sync.c:470
> >
> > Use dma_fence_get_rcu() to replace dma_get_fence(), if fence reference
> > count is 0 then do like timeline fence release things: remove pt link
> > and erase pt node.
> >
> > Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")
> 
> Please completely drop that. This is not something which has any justification to be backported.
OK.
> 
> > Reported-by: syzbot+cba1a1f459230395a076@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=cba1a1f459230395a076
> > Signed-off-by: Edward Adam Davis <eadavis@sina.com>
> > ---
> > v1 -> v2: use dma_fence_get_rcu() and remove link & erase pt node
> >
> >  drivers/dma-buf/sw_sync.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> > index 8df20b0218a9..261bc98b1ed6 100644
> > --- a/drivers/dma-buf/sw_sync.c
> > +++ b/drivers/dma-buf/sw_sync.c
> > @@ -221,7 +221,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
> >                 if (!timeline_fence_signaled(&pt->base))
> >                         break;
> >
> > -               dma_fence_get(&pt->base);
> > +               if (!dma_fence_get_rcu(&pt->base)) {
> 
> Well that is at least better than before but still needs a code comment explaining why this is possible without holding the RCU read side lock.
> 
> Something like /* It's possible to race with timeline_fence_release() here */.
> 
> > +                       list_del_init(&pt->link);
> > +                       rb_erase(&pt->node, &obj->pt_tree);
> 
> What happens is that you race with timeline_fence_release(), so that here is superfluous and potentially even harmful.
timeline_fence_release() and sync_timeline_signal() are synchronized using
the same spinlock(timeline obj->lock is same as fence->extern_lock), so
there is no race condition between them. Instead, a race condition exists
between sync_timeline_signal() and sync_file_release(), which causes the
fence reference count to drop to zero prematurely.

By removing the link and re-initializing it here, list_empty(&pt->link) will
evaluate to true when the spinlock is acquired within timeline_fence_release(),
preventing the redundant execution of the link-removal and node-erase operation.

BR,
Edward

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05 10:03       ` Christian König
  2026-11-05 12:37         ` Edward Adam Davis
@ 2026-11-05 13:55         ` Danilo Krummrich
  2026-11-05 14:14           ` Christian König
  1 sibling, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2026-11-05 13:55 UTC (permalink / raw)
  To: Christian König
  Cc: Edward Adam Davis, sashiko-bot, media-ci, sashiko-reviews,
	dri-devel, linux-kernel, linux-media, syzkaller-bugs,
	sumit.semwal, phasta, robin.clark, dri-devel

On Thu Nov 5, 2026 at 11:03 AM CET, Christian König wrote:
> On 11/5/26 06:27, Edward Adam Davis wrote:
>> Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")
>
> Please completely drop that. This is not something which has any justification to be backported.

Fixes: identifies the commit that introduced a bug for better reference,
independently of whether the fix should be backported.

If something shouldn't be selected by autosel one can add

	Cc: <stable+noautosel@kernel.org> # reason to exclude from stable

In this case I assume you said that because SW_SYNC is test infrastructure? If
so, AFAIK the stable team explicitly does backport test infrastructure as people
also run selftests on stable kernels.

Thanks,
Danilo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] dma-buf/sw_sync: Optimize the retrieval of fence reference counts
  2026-11-05 13:55         ` Danilo Krummrich
@ 2026-11-05 14:14           ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-11-05 14:14 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Edward Adam Davis, sashiko-bot, media-ci, sashiko-reviews,
	dri-devel, linux-kernel, linux-media, syzkaller-bugs,
	sumit.semwal, phasta, robin.clark, dri-devel

On 11/5/26 14:55, Danilo Krummrich wrote:
> On Thu Nov 5, 2026 at 11:03 AM CET, Christian König wrote:
>> On 11/5/26 06:27, Edward Adam Davis wrote:
>>> Fixes: e531fdb5cd5e ("dma-buf/sw_sync: Avoid recursive lock during fence signal")
>>
>> Please completely drop that. This is not something which has any justification to be backported.
> 
> Fixes: identifies the commit that introduced a bug for better reference,
> independently of whether the fix should be backported.
> 
> If something shouldn't be selected by autosel one can add
> 
> 	Cc: <stable+noautosel@kernel.org> # reason to exclude from stable

Oh, thanks for that! I wasn't aware that this possibility exists as well.

> In this case I assume you said that because SW_SYNC is test infrastructure? If
> so, AFAIK the stable team explicitly does backport test infrastructure as people
> also run selftests on stable kernels.

Well yes and no. The situation is a bit more complicated, the point is that SW_SYNC is largely deprecated test infrastructure.

The provided functionality from SW_SYNC can intentionally hard crash the kernel without any chance to recover.

The patch is now fixing a bug which could potentially crash the kernel while using SW_SYNC, but for a component which does exactly that it is kind of pointless.

I mean it is nice to have for documentation purposes, but SW_SYNC is only still around because we don't want to break the uAPI and keep CI systems running.

And *YES*, SW_SYNC shouldn't have been merged in the first place.

Regards,
Christian.

> 
> Thanks,
> Danilo


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-11-05 14:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04  6:38 [syzbot] [media?] [dri?] kernel BUG in sync_timeline_signal syzbot
2026-11-05  4:14 ` [PATCH] dma-buf/sw_sync: Optimize the retrieval of fence reference counts Edward Adam Davis
2026-11-05  4:30   ` sashiko-bot
2026-11-05  5:27     ` [PATCH v2] " Edward Adam Davis
2026-11-05  7:49       ` Philipp Stanner
2026-11-05  9:35         ` Edward Adam Davis
2026-11-05 11:21           ` Philipp Stanner
2026-11-05 10:03       ` Christian König
2026-11-05 12:37         ` Edward Adam Davis
2026-11-05 13:55         ` Danilo Krummrich
2026-11-05 14:14           ` Christian König
2026-11-05  9:53   ` [PATCH] " Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.