[PATCH RFC nf-next 05/12] netfilter: ipset: add and use ip_set_init_comment_slow

[Florian Westphal <fw@strlen.de>]

ip_set_init_comment alters set->ext_size, this is racy.
bitmap and list set types serialize on set->lock, but the hash versions
don't do that.

Switch hash to new ip_set_init_comment_slow.
Alternatives would be to avoid set->ext_size altogether or use
atomic_add/sub for this.

Prefer simpler version.  Also add lockdep annotations to document
this.

ip_set_ext_destroy() has the same issue, but this is harder to fix
because there is at least one case where this gets called without
lock where its safe to do so: set is destroyed.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/netfilter/ipset/ip_set.h  | 9 +++++++++
 net/netfilter/ipset/ip_set_bitmap_gen.h | 2 ++
 net/netfilter/ipset/ip_set_core.c       | 3 ++-
 net/netfilter/ipset/ip_set_hash_gen.h   | 2 +-
 net/netfilter/ipset/ip_set_list_set.c   | 4 ++++
 5 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index b98331572ad2..50cd719bc270 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -502,6 +502,15 @@ ip_set_timeout_set(unsigned long *timeout, u32 value)
 void ip_set_init_comment(struct ip_set *set, struct ip_set_comment *comment,
 			 const struct ip_set_ext *ext);
 
+static inline void
+ip_set_init_comment_slow(struct ip_set *set, struct ip_set_comment *comment,
+			 const struct ip_set_ext *ext)
+{
+	spin_lock_bh(&set->lock);
+	ip_set_init_comment(set, comment, ext);
+	spin_unlock_bh(&set->lock);
+}
+
 static inline void
 ip_set_init_counter(struct ip_set_counter *counter,
 		    const struct ip_set_ext *ext)
diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h b/net/netfilter/ipset/ip_set_bitmap_gen.h
index bb9b5bed10e1..b13cde902c17 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -135,6 +135,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 	void *x = get_ext(set, map, e->id);
 	int ret = mtype_do_add(e, map, flags, set->dsize);
 
+	lockdep_assert_held(&set->lock);
+
 	if (ret == IPSET_ADD_FAILED) {
 		if (SET_WITH_TIMEOUT(set) &&
 		    ip_set_timeout_expired(ext_timeout(x, set))) {
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 6cfad152d7d1..3d6a78ad93f5 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -346,7 +346,8 @@ 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, 1);
+	struct ip_set_comment_rcu *c = rcu_dereference_protected(comment->c,
+								 lockdep_is_held(&set->lock));
 	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 129f06ed85f8..c3dda56d786c 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1065,7 +1065,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 	if (SET_WITH_COUNTER(set))
 		ip_set_init_counter(ext_counter(data, set), ext);
 	if (SET_WITH_COMMENT(set))
-		ip_set_init_comment(set, ext_comment(data, set), ext);
+		ip_set_init_comment_slow(set, ext_comment(data, set), ext);
 	if (SET_WITH_SKBINFO(set))
 		ip_set_init_skbinfo(ext_skbinfo(data, set), ext);
 	/* Must come last for the case when timed out entry is reused */
diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
index 1cef84f15e8c..d7ddc57a4eca 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -221,6 +221,8 @@ static void
 list_set_init_extensions(struct ip_set *set, const struct ip_set_ext *ext,
 			 struct set_elem *e)
 {
+	lockdep_assert_held(&set->lock);
+
 	if (SET_WITH_COUNTER(set))
 		ip_set_init_counter(ext_counter(e, set), ext);
 	if (SET_WITH_COMMENT(set))
@@ -241,6 +243,8 @@ list_set_uadd(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 	struct set_elem *e, *n, *prev, *next;
 	bool flag_exist = flags & IPSET_FLAG_EXIST;
 
+	lockdep_assert_held(&set->lock);
+
 	/* Find where to add the new entry */
 	n = prev = next = NULL;
 	list_for_each_entry_rcu(e, &map->members, list) {
-- 
2.54.0