[syzbot] [bpf?] WARNING: locking bug in bpf_map_put

[syzbot] [bpf?] WARNING: locking bug in bpf_map_put

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    f9f24ca362a4 Add linux-next specific files for 20241031
git tree:       linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=14886630580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=328572ed4d152be9
dashboard link: https://syzkaller.appspot.com/bug?extid=d2adb332fe371b0595e3
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=174432a7980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14ffe55f980000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/eb84549dd6b3/disk-f9f24ca3.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/beb29bdfa297/vmlinux-f9f24ca3.xz
kernel image: https://storage.googleapis.com/syzbot-assets/8881fe3245ad/bzImage-f9f24ca3.xz

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

=============================
[ BUG: Invalid wait context ]
6.12.0-rc5-next-20241031-syzkaller #0 Not tainted
-----------------------------
syz-executor304/5844 is trying to lock:
ffffffff8e9ba4b8 (map_idr_lock){+...}-{3:3}, at: bpf_map_free_id kernel/bpf/syscall.c:468 [inline]
ffffffff8e9ba4b8 (map_idr_lock){+...}-{3:3}, at: bpf_map_put+0x9a/0x380 kernel/bpf/syscall.c:902
other info that might help us debug this:
context-{5:5}
2 locks held by syz-executor304/5844:
 #0: ffffffff8e939f20 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire include/linux/rcupdate.h:337 [inline]
 #0: ffffffff8e939f20 (rcu_read_lock){....}-{1:3}, at: rcu_read_lock include/linux/rcupdate.h:849 [inline]
 #0: ffffffff8e939f20 (rcu_read_lock){....}-{1:3}, at: map_delete_elem+0x338/0x5c0 kernel/bpf/syscall.c:1777
 #1: ffff88807b870410 (&htab->lockdep_key){....}-{2:2}, at: htab_lock_bucket+0x1a4/0x370 kernel/bpf/hashtab.c:167
stack backtrace:
CPU: 1 UID: 0 PID: 5844 Comm: syz-executor304 Not tainted 6.12.0-rc5-next-20241031-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 print_lock_invalid_wait_context kernel/locking/lockdep.c:4826 [inline]
 check_wait_context kernel/locking/lockdep.c:4898 [inline]
 __lock_acquire+0x15a8/0x2100 kernel/locking/lockdep.c:5176
 lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5849
 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
 _raw_spin_lock_irqsave+0xd5/0x120 kernel/locking/spinlock.c:162
 bpf_map_free_id kernel/bpf/syscall.c:468 [inline]
 bpf_map_put+0x9a/0x380 kernel/bpf/syscall.c:902
 htab_put_fd_value kernel/bpf/hashtab.c:911 [inline]
 free_htab_elem+0xbb/0x460 kernel/bpf/hashtab.c:946
 htab_map_delete_elem+0x576/0x6b0 kernel/bpf/hashtab.c:1438
 map_delete_elem+0x431/0x5c0 kernel/bpf/syscall.c:1778
 __sys_bpf+0x598/0x810 kernel/bpf/syscall.c:5745
 __do_sys_bpf kernel/bpf/syscall.c:5861 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:5859 [inline]
 __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5859
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f9ad03385e9
Code: 48 83 c4 28 c3 e8 37 17 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffd14d58828 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
RAX: ffffffffffffffda RBX: 00007ffd14d589f8 RCX: 00007f9ad03385e9
RDX: 0000000000000020 RSI: 0000000020000300 RDI: 0000000000000003
R


---
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

Re: [syzbot] [bpf?] WARNING: locking bug in bpf_map_put

[syzbot]

syzbot has bisected this issue to:

commit 560af5dc839eef08a273908f390cfefefb82aa04
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date:   Wed Oct 9 15:45:03 2024 +0000

    lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=122a4740580000
start commit:   f9f24ca362a4 Add linux-next specific files for 20241031
git tree:       linux-next
final oops:     https://syzkaller.appspot.com/x/report.txt?x=112a4740580000
console output: https://syzkaller.appspot.com/x/log.txt?x=162a4740580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=328572ed4d152be9
dashboard link: https://syzkaller.appspot.com/bug?extid=d2adb332fe371b0595e3
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=174432a7980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14ffe55f980000

Reported-by: syzbot+d2adb332fe371b0595e3@syzkaller.appspotmail.com
Fixes: 560af5dc839e ("lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Re: [syzbot] [bpf?] WARNING: locking bug in bpf_map_put

[Sebastian Andrzej Siewior]

On 2024-11-03 18:29:04 [-0800], syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit 560af5dc839eef08a273908f390cfefefb82aa04
> Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Date:   Wed Oct 9 15:45:03 2024 +0000
> 
>     lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=122a4740580000
> start commit:   f9f24ca362a4 Add linux-next specific files for 20241031
> git tree:       linux-next
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=112a4740580000
> console output: https://syzkaller.appspot.com/x/log.txt?x=162a4740580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=328572ed4d152be9
> dashboard link: https://syzkaller.appspot.com/bug?extid=d2adb332fe371b0595e3
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=174432a7980000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14ffe55f980000
> 
> Reported-by: syzbot+d2adb332fe371b0595e3@syzkaller.appspotmail.com
> Fixes: 560af5dc839e ("lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.")
> 
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

This is due to raw_spinlock_t in bucket::lock and the acquired
spinlock_t underneath. Would it would to move free part outside of the
locked section?

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index b14b87463ee04..1d8d09fdd2da5 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -824,13 +824,14 @@ static bool htab_lru_map_delete_node(void *arg, struct bpf_lru_node *node)
 	hlist_nulls_for_each_entry_rcu(l, n, head, hash_node)
 		if (l == tgt_l) {
 			hlist_nulls_del_rcu(&l->hash_node);
-			check_and_free_fields(htab, l);
 			bpf_map_dec_elem_count(&htab->map);
 			break;
 		}
 
 	htab_unlock_bucket(htab, b, tgt_l->hash, flags);
 
+	if (l == tgt_l)
+		check_and_free_fields(htab, l);
 	return l == tgt_l;
 }
 
@@ -1181,14 +1182,18 @@ static long htab_map_update_elem(struct bpf_map *map, void *key, void *value,
 	 * concurrent search will find it before old elem
 	 */
 	hlist_nulls_add_head_rcu(&l_new->hash_node, head);
-	if (l_old) {
+	if (l_old)
 		hlist_nulls_del_rcu(&l_old->hash_node);
+	htab_unlock_bucket(htab, b, hash, flags);
+
+	if (l_old) {
 		if (!htab_is_prealloc(htab))
 			free_htab_elem(htab, l_old);
 		else
 			check_and_free_fields(htab, l_old);
 	}
-	ret = 0;
+	return 0;
+
 err:
 	htab_unlock_bucket(htab, b, hash, flags);
 	return ret;
@@ -1433,14 +1438,15 @@ static long htab_map_delete_elem(struct bpf_map *map, void *key)
 
 	l = lookup_elem_raw(head, hash, key, key_size);
 
-	if (l) {
+	if (l)
 		hlist_nulls_del_rcu(&l->hash_node);
-		free_htab_elem(htab, l);
-	} else {
+	else
 		ret = -ENOENT;
-	}
 
 	htab_unlock_bucket(htab, b, hash, flags);
+
+	if (l)
+		free_htab_elem(htab, l);
 	return ret;
 }
 
@@ -1647,14 +1653,16 @@ static int __htab_map_lookup_and_delete_elem(struct bpf_map *map, void *key,
 		}
 
 		hlist_nulls_del_rcu(&l->hash_node);
-		if (!is_lru_map)
-			free_htab_elem(htab, l);
 	}
 
 	htab_unlock_bucket(htab, b, hash, bflags);
 
-	if (is_lru_map && l)
-		htab_lru_push_free(htab, l);
+	if (l) {
+		if (is_lru_map)
+			htab_lru_push_free(htab, l);
+		else
+			free_htab_elem(htab, l);
+	}
 
 	return ret;
 }
@@ -1851,15 +1859,12 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
 
 			/* bpf_lru_push_free() will acquire lru_lock, which
 			 * may cause deadlock. See comments in function
-			 * prealloc_lru_pop(). Let us do bpf_lru_push_free()
-			 * after releasing the bucket lock.
+			 * prealloc_lru_pop(). htab_lru_push_free() may allocate
+			 * sleeping locks. Let us do bpf_lru_push_free() after
+			 * releasing the bucket lock.
 			 */
-			if (is_lru_map) {
-				l->batch_flink = node_to_free;
-				node_to_free = l;
-			} else {
-				free_htab_elem(htab, l);
-			}
+			l->batch_flink = node_to_free;
+			node_to_free = l;
 		}
 		dst_key += key_size;
 		dst_val += value_size;
@@ -1871,7 +1876,10 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
 	while (node_to_free) {
 		l = node_to_free;
 		node_to_free = node_to_free->batch_flink;
-		htab_lru_push_free(htab, l);
+		if (is_lru_map)
+			htab_lru_push_free(htab, l);
+		else
+			free_htab_elem(htab, l);
 	}
 
 next_batch:

Re: [syzbot] [bpf?] WARNING: locking bug in bpf_map_put

[Hou Tao]

Hi,

On 11/5/2024 12:28 AM, Sebastian Andrzej Siewior wrote:
> On 2024-11-03 18:29:04 [-0800], syzbot wrote:
>> syzbot has bisected this issue to:
>>
>> commit 560af5dc839eef08a273908f390cfefefb82aa04
>> Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> Date:   Wed Oct 9 15:45:03 2024 +0000
>>
>>     lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.
>>
>> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=122a4740580000
>> start commit:   f9f24ca362a4 Add linux-next specific files for 20241031
>> git tree:       linux-next
>> final oops:     https://syzkaller.appspot.com/x/report.txt?x=112a4740580000
>> console output: https://syzkaller.appspot.com/x/log.txt?x=162a4740580000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=328572ed4d152be9
>> dashboard link: https://syzkaller.appspot.com/bug?extid=d2adb332fe371b0595e3
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=174432a7980000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14ffe55f980000
>>
>> Reported-by: syzbot+d2adb332fe371b0595e3@syzkaller.appspotmail.com
>> Fixes: 560af5dc839e ("lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.")
>>
>> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> This is due to raw_spinlock_t in bucket::lock and the acquired
> spinlock_t underneath. Would it would to move free part outside of the
> locked section?

I think moving free_htab_elem() after htab_unlock_bucket() is OK. But
the fix below is not enough, and there is some corn cases for
pre-allocated element . I had written a patch for the problem a few day
ago because the problem can be easily reproduced by running test_maps. I
am also writing a selftest patch for it.  I could post the patch and the
selftest patch if you are OK with it.
>
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index b14b87463ee04..1d8d09fdd2da5 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -824,13 +824,14 @@ static bool htab_lru_map_delete_node(void *arg, struct bpf_lru_node *node)
>  	hlist_nulls_for_each_entry_rcu(l, n, head, hash_node)
>  		if (l == tgt_l) {
>  			hlist_nulls_del_rcu(&l->hash_node);
> -			check_and_free_fields(htab, l);
>  			bpf_map_dec_elem_count(&htab->map);
>  			break;
>  		}
>  
>  	htab_unlock_bucket(htab, b, tgt_l->hash, flags);
>  
> +	if (l == tgt_l)
> +		check_and_free_fields(htab, l);
>  	return l == tgt_l;
>  }
>  
> @@ -1181,14 +1182,18 @@ static long htab_map_update_elem(struct bpf_map *map, void *key, void *value,
>  	 * concurrent search will find it before old elem
>  	 */
>  	hlist_nulls_add_head_rcu(&l_new->hash_node, head);
> -	if (l_old) {
> +	if (l_old)
>  		hlist_nulls_del_rcu(&l_old->hash_node);
> +	htab_unlock_bucket(htab, b, hash, flags);
> +
> +	if (l_old) {
>  		if (!htab_is_prealloc(htab))
>  			free_htab_elem(htab, l_old);
>  		else
>  			check_and_free_fields(htab, l_old);
>  	}
> -	ret = 0;
> +	return 0;
> +
>  err:
>  	htab_unlock_bucket(htab, b, hash, flags);
>  	return ret;
> @@ -1433,14 +1438,15 @@ static long htab_map_delete_elem(struct bpf_map *map, void *key)
>  
>  	l = lookup_elem_raw(head, hash, key, key_size);
>  
> -	if (l) {
> +	if (l)
>  		hlist_nulls_del_rcu(&l->hash_node);
> -		free_htab_elem(htab, l);
> -	} else {
> +	else
>  		ret = -ENOENT;
> -	}
>  
>  	htab_unlock_bucket(htab, b, hash, flags);
> +
> +	if (l)
> +		free_htab_elem(htab, l);
>  	return ret;
>  }
>  
> @@ -1647,14 +1653,16 @@ static int __htab_map_lookup_and_delete_elem(struct bpf_map *map, void *key,
>  		}
>  
>  		hlist_nulls_del_rcu(&l->hash_node);
> -		if (!is_lru_map)
> -			free_htab_elem(htab, l);
>  	}
>  
>  	htab_unlock_bucket(htab, b, hash, bflags);
>  
> -	if (is_lru_map && l)
> -		htab_lru_push_free(htab, l);
> +	if (l) {
> +		if (is_lru_map)
> +			htab_lru_push_free(htab, l);
> +		else
> +			free_htab_elem(htab, l);
> +	}
>  
>  	return ret;
>  }
> @@ -1851,15 +1859,12 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
>  
>  			/* bpf_lru_push_free() will acquire lru_lock, which
>  			 * may cause deadlock. See comments in function
> -			 * prealloc_lru_pop(). Let us do bpf_lru_push_free()
> -			 * after releasing the bucket lock.
> +			 * prealloc_lru_pop(). htab_lru_push_free() may allocate
> +			 * sleeping locks. Let us do bpf_lru_push_free() after
> +			 * releasing the bucket lock.
>  			 */
> -			if (is_lru_map) {
> -				l->batch_flink = node_to_free;
> -				node_to_free = l;
> -			} else {
> -				free_htab_elem(htab, l);
> -			}
> +			l->batch_flink = node_to_free;
> +			node_to_free = l;
>  		}
>  		dst_key += key_size;
>  		dst_val += value_size;
> @@ -1871,7 +1876,10 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
>  	while (node_to_free) {
>  		l = node_to_free;
>  		node_to_free = node_to_free->batch_flink;
> -		htab_lru_push_free(htab, l);
> +		if (is_lru_map)
> +			htab_lru_push_free(htab, l);
> +		else
> +			free_htab_elem(htab, l);
>  	}
>  
>  next_batch:
>
> .