Avoid always-true arguments where possible, they defeat lockdep. ip_set_comment_free() is problematic: called from different contexts, some hold set->lock spinlock (safe), some do not hold a lock but have other means of mutual exclusion (e.g., entire set torn down). Other callers need investigation: ip_set_comment_free() alters set->ext_size in a non-atomic way. I don't see how this is safe except for "entire set is destroyed" case: parallel usage would be a bug. Add a few lockdep assertions to ip_set_init_comment() callpaths to have more confidence in the correctness of the "Called from uadd only, protected by the set spinlock." comment at the start of ip_set_init_comment(). Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Florian Westphal --- include/linux/netfilter/ipset/ip_set.h | 3 +++ net/netfilter/ipset/ip_set_core.c | 3 +-- net/netfilter/ipset/ip_set_hash_gen.h | 6 ++---- net/netfilter/ipset/ip_set_list_set.c | 2 ++ 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h index f9003ec21259..99bc997914f4 100644 --- a/include/linux/netfilter/ipset/ip_set.h +++ b/include/linux/netfilter/ipset/ip_set.h @@ -282,6 +282,9 @@ struct ip_set { void *data; }; +#define ipset_dereference_locked(p, set) \ + rcu_dereference_protected(p, lockdep_is_held(&set->lock)) + static inline void __ip_set_destroy_comment(struct ip_set *set, void *data) { diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c index 6ece5cf305fe..a5f77f639d2a 100644 --- a/net/netfilter/ipset/ip_set_core.c +++ b/net/netfilter/ipset/ip_set_core.c @@ -346,8 +346,7 @@ void ip_set_init_comment(struct ip_set *set, struct ip_set_comment *comment, const struct ip_set_ext *ext) { - struct ip_set_comment_rcu *c = rcu_dereference_protected(comment->c, - lockdep_is_held(&set->lock)); + struct ip_set_comment_rcu *c = ipset_dereference_locked(comment->c, set); size_t len = ext->comment ? strlen(ext->comment) : 0; if (unlikely(c)) { diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h index a0f2cd481b82..e615de2e616b 100644 --- a/net/netfilter/ipset/ip_set_hash_gen.h +++ b/net/netfilter/ipset/ip_set_hash_gen.h @@ -11,8 +11,6 @@ #include #include -#define __ipset_dereference(p) \ - rcu_dereference_protected(p, 1) #define ipset_dereference_nfnl(p) \ rcu_dereference_protected(p, \ lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET)) @@ -271,7 +269,7 @@ mtype_add_cidr(struct ip_set *set, struct htype *h, u8 cidr, u8 n) int i, j, found, len = 0, ret = 0; spin_lock_bh(&set->lock); - nets = __ipset_dereference(h->rnets[n]); + nets = ipset_dereference_locked(h->rnets[n], set); /* Add in increasing prefix order, so larger cidr first */ for (i = 0, found = -1; i < nets->len; i++) { if (nets->nets[i].count) @@ -323,7 +321,7 @@ mtype_del_cidr(struct ip_set *set, struct htype *h, u8 cidr, u8 n) int found; spin_lock_bh(&set->lock); - nets = __ipset_dereference(h->rnets[n]); + nets = ipset_dereference_locked(h->rnets[n], set); for (i = 0, found = -1; i < nets->len; i++) { if (nets->nets[i].count) len++; diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c index d7ddc57a4eca..27bc96458e13 100644 --- a/net/netfilter/ipset/ip_set_list_set.c +++ b/net/netfilter/ipset/ip_set_list_set.c @@ -326,6 +326,8 @@ list_set_udel(struct ip_set *set, void *value, const struct ip_set_ext *ext, struct set_adt_elem *d = value; struct set_elem *e, *n, *next, *prev = NULL; + lockdep_assert_held(&set->lock); + list_for_each_entry_safe(e, n, &map->members, list) { if (SET_WITH_TIMEOUT(set) && ip_set_timeout_expired(ext_timeout(e, set))) -- 2.54.0