On 2024-11-03 18:29:04 [-0800], syzbot wrote: > syzbot has bisected this issue to: > > commit 560af5dc839eef08a273908f390cfefefb82aa04 > Author: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxxxxxxxxx > 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:
