We fixed a similar problem before in commit 7f4f7dd4417d ("netfilter:
ipset: ipset list may return wrong member count for set with timeout").
The same issue exists in ip_set_bitmap_gen.h as well.
Test case:
$ ipset create test bitmap:ip range 10.0.0.0/8 netmask 24 timeout 5
$ ipset add test 10.0.0.0
$ ipset add test 10.255.255.255
$ sleep 5s
$ ipset list test
Name: test
Type: bitmap:ip
Revision: 3
Header: range 10.0.0.0-10.255.255.255 netmask 24 timeout 5
Size in memory: 532568
References: 0
Number of entries: 2
Members:
We return "Number of entries: 2" but no members are listed. That is
because when we run mtype_head the garbage collector hasn't run yet, but
mtype_list checks and cleans up members with expired timeout. To avoid
this we can run mtype_expire before printing the number of elements in
mytype_head().
Reviewed-by: Joshua Hunt <johunt@xxxxxxxxxx>
Signed-off-by: Vishwanath Pai <vpai@xxxxxxxxxx>
---
net/netfilter/ipset/ip_set_bitmap_gen.h | 46 ++++++++++++++++++-------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h b/net/netfilter/ipset/ip_set_bitmap_gen.h
index 26ab0e9612d8..dd871305bd6e 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -28,6 +28,7 @@
#define mtype_del IPSET_TOKEN(MTYPE, _del)
#define mtype_list IPSET_TOKEN(MTYPE, _list)
#define mtype_gc IPSET_TOKEN(MTYPE, _gc)
+#define mtype_expire IPSET_TOKEN(MTYPE, _expire)
#define mtype MTYPE
#define get_ext(set, map, id) ((map)->extensions + ((set)->dsize * (id)))
@@ -88,13 +89,44 @@ mtype_memsize(const struct mtype *map, size_t dsize)
map->elements * dsize;
}
+/* We should grab set->lock before calling this function */
+static void
+mtype_expire(struct ip_set *set)
+{
+ struct mtype *map = set->data;
+ void *x;
+ u32 id;
+
+ for (id = 0; id < map->elements; id++)
+ if (mtype_gc_test(id, map, set->dsize)) {
+ x = get_ext(set, map, id);
+ if (ip_set_timeout_expired(ext_timeout(x, set))) {
+ clear_bit(id, map->members);
+ ip_set_ext_destroy(set, x);
+ set->elements--;
+ }
+ }
+}
+
static int
mtype_head(struct ip_set *set, struct sk_buff *skb)
{
const struct mtype *map = set->data;
struct nlattr *nested;
- size_t memsize = mtype_memsize(map, set->dsize) + set->ext_size;
+ size_t memsize;
+
+ /* If any members have expired, set->elements will be wrong,
+ * mytype_expire function will update it with the right count.
+ * set->elements can still be incorrect in the case of a huge set
+ * because elements can timeout during set->list().
+ */
+ if (SET_WITH_TIMEOUT(set)) {
+ spin_lock_bh(&set->lock);
+ mtype_expire(set);
+ spin_unlock_bh(&set->lock);
+ }
+ memsize = mtype_memsize(map, set->dsize) + set->ext_size;
nested = nla_nest_start(skb, IPSET_ATTR_DATA);
if (!nested)
goto nla_put_failure;
@@ -266,22 +298,12 @@ mtype_gc(struct timer_list *t)
{
struct mtype *map = from_timer(map, t, gc);
struct ip_set *set = map->set;
- void *x;
- u32 id;
/* We run parallel with other readers (test element)
* but adding/deleting new entries is locked out
*/
spin_lock_bh(&set->lock);
- for (id = 0; id < map->elements; id++)
- if (mtype_gc_test(id, map, set->dsize)) {
- x = get_ext(set, map, id);
- if (ip_set_timeout_expired(ext_timeout(x, set))) {
- clear_bit(id, map->members);
- ip_set_ext_destroy(set, x);
- set->elements--;
- }
- }
+ mtype_expire(set);
spin_unlock_bh(&set->lock);
map->gc.expires = jiffies + IPSET_GC_PERIOD(set->timeout) * HZ;
--
2.25.1
