]> granicus.if.org Git - ipset/commitdiff
Fix "don't update counters" mode when counters used at the matching
authorJozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Thu, 4 Jan 2018 12:21:26 +0000 (13:21 +0100)
committerJozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Thu, 4 Jan 2018 12:21:26 +0000 (13:21 +0100)
The matching of the counters was not taken into account, fixed.

kernel/include/linux/netfilter/ipset/ip_set.h
kernel/include/linux/netfilter/ipset/ip_set_counter.h
kernel/net/netfilter/ipset/ip_set_bitmap_gen.h
kernel/net/netfilter/ipset/ip_set_core.c
kernel/net/netfilter/ipset/ip_set_hash_gen.h
kernel/net/netfilter/ipset/ip_set_list_set.c
kernel/net/netfilter/xt_set.c
tests/hash:net.t
tests/iptables.sh
tests/match_target.t

index 3e44e66e9d4cc4a6bab6366dbadd7a0c13c32a98..3986e63111b339f6314ae5d2e1d6ca52d4a2f597 100644 (file)
@@ -123,6 +123,8 @@ struct ip_set_ext {
        u64 bytes;
        char *comment;
        u32 timeout;
+       u8 packets_op;
+       u8 bytes_op;
 };
 
 struct ip_set;
@@ -340,6 +342,10 @@ extern int ip_set_get_extensions(struct ip_set *set, struct nlattr *tb[],
                                 struct ip_set_ext *ext);
 extern int ip_set_put_extensions(struct sk_buff *skb, const struct ip_set *set,
                                 const void *e, bool active);
+extern bool ip_set_match_extensions(struct ip_set *set,
+                                   const struct ip_set_ext *ext,
+                                   struct ip_set_ext *mext,
+                                   u32 flags, void *data);
 
 static inline int
 ip_set_get_hostipaddr4(struct nlattr *nla, u32 *ipaddr)
index 8bf91923b60397f31dfc17e9584f204ba32f043d..a0d134ab095fcb41f0ced12b52e3e6d44a05ec62 100644 (file)
@@ -34,20 +34,33 @@ ip_set_get_packets(const struct ip_set_counter *counter)
        return (u64)atomic64_read(&(counter)->packets);
 }
 
+static inline bool
+ip_set_match_counter(u64 counter, u64 match, u8 op)
+{
+       switch (op) {
+       case IPSET_COUNTER_NONE:
+               return true;
+       case IPSET_COUNTER_EQ:
+               return counter == match;
+       case IPSET_COUNTER_NE:
+               return counter != match;
+       case IPSET_COUNTER_LT:
+               return counter < match;
+       case IPSET_COUNTER_GT:
+               return counter > match;
+       }
+       return false;
+}
+
 static inline void
 ip_set_update_counter(struct ip_set_counter *counter,
-                     const struct ip_set_ext *ext,
-                     struct ip_set_ext *mext, u32 flags)
+                     const struct ip_set_ext *ext, u32 flags)
 {
        if (ext->packets != ULLONG_MAX &&
            !(flags & IPSET_FLAG_SKIP_COUNTER_UPDATE)) {
                ip_set_add_bytes(ext->bytes, counter);
                ip_set_add_packets(ext->packets, counter);
        }
-       if (flags & IPSET_FLAG_MATCH_COUNTERS) {
-               mext->packets = ip_set_get_packets(counter);
-               mext->bytes = ip_set_get_bytes(counter);
-       }
 }
 
 static inline bool
index aa3dbb139d22d77d7dd4b1421818574549445e83..0c9db1937eb4cb767677ca6be27f3edfcb9a11db 100644 (file)
@@ -129,14 +129,7 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 
        if (ret <= 0)
                return ret;
-       if (SET_WITH_TIMEOUT(set) &&
-           ip_set_timeout_expired(ext_timeout(x, set)))
-               return 0;
-       if (SET_WITH_COUNTER(set))
-               ip_set_update_counter(ext_counter(x, set), ext, mext, flags);
-       if (SET_WITH_SKBINFO(set))
-               ip_set_get_skbinfo(ext_skbinfo(x, set), ext, mext, flags);
-       return 1;
+       return ip_set_match_extensions(set, ext, mext, flags, x);
 }
 
 static int
index 32b7c0c0e26757cf6f0ee736a45b7c598868a63e..134e708e224c6299cc4456ce195ed303e13fe6ac 100644 (file)
@@ -477,6 +477,31 @@ ip_set_put_extensions(struct sk_buff *skb, const struct ip_set *set,
 }
 EXPORT_SYMBOL_GPL(ip_set_put_extensions);
 
+bool
+ip_set_match_extensions(struct ip_set *set, const struct ip_set_ext *ext,
+                       struct ip_set_ext *mext, u32 flags, void *data)
+{
+       if (SET_WITH_TIMEOUT(set) &&
+           ip_set_timeout_expired(ext_timeout(data, set)))
+               return false;
+       if (SET_WITH_COUNTER(set)) {
+               struct ip_set_counter *counter = ext_counter(data, set);
+
+               if (flags & IPSET_FLAG_MATCH_COUNTERS &&
+                   !(ip_set_match_counter(ip_set_get_packets(counter),
+                               mext->packets, mext->packets_op) &&
+                     ip_set_match_counter(ip_set_get_bytes(counter),
+                               mext->bytes, mext->bytes_op)))
+                       return false;
+               ip_set_update_counter(counter, ext, flags);
+       }
+       if (SET_WITH_SKBINFO(set))
+               ip_set_get_skbinfo(ext_skbinfo(data, set),
+                                  ext, mext, flags);
+       return true;
+}
+EXPORT_SYMBOL_GPL(ip_set_match_extensions);
+
 /* Creating/destroying/renaming/swapping affect the existence and
  * the properties of a set. All of these can be executed from userspace
  * only and serialized by the nfnl mutex indirectly from nfnetlink.
index d24ee3c87788fb9e23f5ff6d26a090c72053b29c..291c7d4fb0b1564ecb28fe870df97ede16985a7d 100644 (file)
@@ -919,12 +919,9 @@ static inline int
 mtype_data_match(struct mtype_elem *data, const struct ip_set_ext *ext,
                 struct ip_set_ext *mext, struct ip_set *set, u32 flags)
 {
-       if (SET_WITH_COUNTER(set))
-               ip_set_update_counter(ext_counter(data, set),
-                                     ext, mext, flags);
-       if (SET_WITH_SKBINFO(set))
-               ip_set_get_skbinfo(ext_skbinfo(data, set),
-                                  ext, mext, flags);
+       if (!ip_set_match_extensions(set, ext, mext, flags, data))
+               return 0;
+       /* nomatch entries return -ENOTEMPTY */
        return mtype_do_data_match(data);
 }
 
@@ -943,9 +940,9 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
        struct mtype_elem *data;
 #if IPSET_NET_COUNT == 2
        struct mtype_elem orig = *d;
-       int i, j = 0, k;
+       int ret, i, j = 0, k;
 #else
-       int i, j = 0;
+       int ret, i, j = 0;
 #endif
        u32 key, multi = 0;
 
@@ -971,18 +968,13 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
                        data = ahash_data(n, i, set->dsize);
                        if (!mtype_data_equal(data, d, &multi))
                                continue;
-                       if (SET_WITH_TIMEOUT(set)) {
-                               if (!ip_set_timeout_expired(
-                                               ext_timeout(data, set)))
-                                       return mtype_data_match(data, ext,
-                                                               mext, set,
-                                                               flags);
+                       ret = mtype_data_match(data, ext, mext, set, flags);
+                       if (ret != 0)
+                               return ret;
 #ifdef IP_SET_HASH_WITH_MULTI
-                               multi = 0;
+                       /* No match, reset multiple match flag */
+                       multi = 0;
 #endif
-                       } else
-                               return mtype_data_match(data, ext,
-                                                       mext, set, flags);
                }
 #if IPSET_NET_COUNT == 2
                }
@@ -1029,12 +1021,11 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,
                if (!test_bit(i, n->used))
                        continue;
                data = ahash_data(n, i, set->dsize);
-               if (mtype_data_equal(data, d, &multi) &&
-                   !(SET_WITH_TIMEOUT(set) &&
-                     ip_set_timeout_expired(ext_timeout(data, set)))) {
-                       ret = mtype_data_match(data, ext, mext, set, flags);
+               if (!mtype_data_equal(data, d, &multi))
+                       continue;
+               ret = mtype_data_match(data, ext, mext, set, flags);
+               if (ret != 0)
                        goto out;
-               }
        }
 out:
        return ret;
index 55ecea2d444a74e9c91ea1c572e04d129fb448f1..e8c67dc20ee3e330fcd17040e7dbe188bd8e8074 100644 (file)
@@ -57,8 +57,9 @@ list_set_ktest(struct ip_set *set, const struct sk_buff *skb,
               struct ip_set_adt_opt *opt, const struct ip_set_ext *ext)
 {
        struct list_set *map = set->data;
+       struct ip_set_ext *mext = &opt->ext;
        struct set_elem *e;
-       u32 cmdflags = opt->cmdflags;
+       u32 flags = opt->cmdflags;
        int ret;
 
        /* Don't lookup sub-counters at all */
@@ -66,21 +67,11 @@ list_set_ktest(struct ip_set *set, const struct sk_buff *skb,
        if (opt->cmdflags & IPSET_FLAG_SKIP_SUBCOUNTER_UPDATE)
                opt->cmdflags &= ~IPSET_FLAG_SKIP_COUNTER_UPDATE;
        list_for_each_entry_rcu(e, &map->members, list) {
-               if (SET_WITH_TIMEOUT(set) &&
-                   ip_set_timeout_expired(ext_timeout(e, set)))
-                       continue;
                ret = ip_set_test(e->id, skb, par, opt);
-               if (ret > 0) {
-                       if (SET_WITH_COUNTER(set))
-                               ip_set_update_counter(ext_counter(e, set),
-                                                     ext, &opt->ext,
-                                                     cmdflags);
-                       if (SET_WITH_SKBINFO(set))
-                               ip_set_get_skbinfo(ext_skbinfo(e, set),
-                                                  ext, &opt->ext,
-                                                  cmdflags);
-                       return ret;
-               }
+               if (ret <= 0)
+                       continue;
+               if (ip_set_match_extensions(set, ext, mext, flags, e))
+                       return 1;
        }
        return 0;
 }
index 01868aaf2d9f69491c71a3792fc19bb788fc01c6..a7ac9134d63bf9e5029c43f0fce65d39f3c197ed 100644 (file)
@@ -56,13 +56,17 @@ match_set(ip_set_id_t index, const struct sk_buff *skb,
        return inv;
 }
 
-#define ADT_OPT(n, f, d, fs, cfs, t)   \
-struct ip_set_adt_opt n = {            \
-       .family = f,                    \
-       .dim = d,                       \
-       .flags = fs,                    \
-       .cmdflags = cfs,                \
-       .ext.timeout = t,               \
+#define ADT_OPT(n, f, d, fs, cfs, t, p, b, po, bo)     \
+struct ip_set_adt_opt n = {                            \
+       .family = f,                                    \
+       .dim = d,                                       \
+       .flags = fs,                                    \
+       .cmdflags = cfs,                                \
+       .ext.timeout = t,                               \
+       .ext.packets = p,                               \
+       .ext.bytes = b,                                 \
+       .ext.packets_op = po,                           \
+       .ext.bytes_op = bo,                             \
 }
 
 /* Revision 0 interface: backward compatible with netfilter/iptables */
@@ -73,7 +77,8 @@ set_match_v0(const struct sk_buff *skb, CONST struct xt_action_param *par)
        const struct xt_set_info_match_v0 *info = par->matchinfo;
 
        ADT_OPT(opt, XT_FAMILY(par), info->match_set.u.compat.dim,
-               info->match_set.u.compat.flags, 0, UINT_MAX);
+               info->match_set.u.compat.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
 
        return match_set(info->match_set.index, skb, par, &opt,
                         info->match_set.u.compat.flags & IPSET_INV_MATCH);
@@ -136,7 +141,8 @@ set_match_v1(const struct sk_buff *skb, CONST struct xt_action_param *par)
        const struct xt_set_info_match_v1 *info = par->matchinfo;
 
        ADT_OPT(opt, XT_FAMILY(par), info->match_set.dim,
-               info->match_set.flags, 0, UINT_MAX);
+               info->match_set.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
 
        if (opt.flags & IPSET_RETURN_NOMATCH)
                opt.cmdflags |= IPSET_FLAG_RETURN_NOMATCH;
@@ -177,46 +183,22 @@ set_match_v1_destroy(const struct xt_mtdtor_param *par)
 
 /* Revision 3 match */
 
-static bool
-match_counter0(u64 counter, const struct ip_set_counter_match0 *info)
-{
-       switch (info->op) {
-       case IPSET_COUNTER_NONE:
-               return true;
-       case IPSET_COUNTER_EQ:
-               return counter == info->value;
-       case IPSET_COUNTER_NE:
-               return counter != info->value;
-       case IPSET_COUNTER_LT:
-               return counter < info->value;
-       case IPSET_COUNTER_GT:
-               return counter > info->value;
-       }
-       return false;
-}
-
 static bool
 set_match_v3(const struct sk_buff *skb, CONST struct xt_action_param *par)
 {
        const struct xt_set_info_match_v3 *info = par->matchinfo;
-       int ret;
 
        ADT_OPT(opt, XT_FAMILY(par), info->match_set.dim,
-               info->match_set.flags, info->flags, UINT_MAX);
+               info->match_set.flags, info->flags, UINT_MAX,
+               info->packets.value, info->bytes.value,
+               info->packets.op, info->bytes.op);
 
        if (info->packets.op != IPSET_COUNTER_NONE ||
            info->bytes.op != IPSET_COUNTER_NONE)
                opt.cmdflags |= IPSET_FLAG_MATCH_COUNTERS;
 
-       ret = match_set(info->match_set.index, skb, par, &opt,
-                       info->match_set.flags & IPSET_INV_MATCH);
-
-       if (!(ret && opt.cmdflags & IPSET_FLAG_MATCH_COUNTERS))
-               return ret;
-
-       if (!match_counter0(opt.ext.packets, &info->packets))
-               return false;
-       return match_counter0(opt.ext.bytes, &info->bytes);
+       return match_set(info->match_set.index, skb, par, &opt,
+                        info->match_set.flags & IPSET_INV_MATCH);
 }
 
 #define set_match_v3_checkentry        set_match_v1_checkentry
@@ -224,46 +206,22 @@ set_match_v3(const struct sk_buff *skb, CONST struct xt_action_param *par)
 
 /* Revision 4 match */
 
-static bool
-match_counter(u64 counter, const struct ip_set_counter_match *info)
-{
-       switch (info->op) {
-       case IPSET_COUNTER_NONE:
-               return true;
-       case IPSET_COUNTER_EQ:
-               return counter == info->value;
-       case IPSET_COUNTER_NE:
-               return counter != info->value;
-       case IPSET_COUNTER_LT:
-               return counter < info->value;
-       case IPSET_COUNTER_GT:
-               return counter > info->value;
-       }
-       return false;
-}
-
 static bool
 set_match_v4(const struct sk_buff *skb, CONST struct xt_action_param *par)
 {
        const struct xt_set_info_match_v4 *info = par->matchinfo;
-       int ret;
 
        ADT_OPT(opt, XT_FAMILY(par), info->match_set.dim,
-               info->match_set.flags, info->flags, UINT_MAX);
+               info->match_set.flags, info->flags, UINT_MAX,
+               info->packets.value, info->bytes.value,
+               info->packets.op, info->bytes.op);
 
        if (info->packets.op != IPSET_COUNTER_NONE ||
            info->bytes.op != IPSET_COUNTER_NONE)
                opt.cmdflags |= IPSET_FLAG_MATCH_COUNTERS;
 
-       ret = match_set(info->match_set.index, skb, par, &opt,
-                       info->match_set.flags & IPSET_INV_MATCH);
-
-       if (!(ret && opt.cmdflags & IPSET_FLAG_MATCH_COUNTERS))
-               return ret;
-
-       if (!match_counter(opt.ext.packets, &info->packets))
-               return false;
-       return match_counter(opt.ext.bytes, &info->bytes);
+       return match_set(info->match_set.index, skb, par, &opt,
+                        info->match_set.flags & IPSET_INV_MATCH);
 }
 
 #define set_match_v4_checkentry        set_match_v1_checkentry
@@ -285,9 +243,11 @@ set_target_v0(struct sk_buff *skb, const struct xt_action_param *par)
        const struct xt_set_info_target_v0 *info = par->targinfo;
 
        ADT_OPT(add_opt, XT_FAMILY(par), info->add_set.u.compat.dim,
-               info->add_set.u.compat.flags, 0, UINT_MAX);
+               info->add_set.u.compat.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
        ADT_OPT(del_opt, XT_FAMILY(par), info->del_set.u.compat.dim,
-               info->del_set.u.compat.flags, 0, UINT_MAX);
+               info->del_set.u.compat.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
 
        if (info->add_set.index != IPSET_INVALID_ID)
                ip_set_add(info->add_set.index, skb, CAST_TO_MATCH par,
@@ -363,9 +323,11 @@ set_target_v1(struct sk_buff *skb, const struct xt_action_param *par)
        const struct xt_set_info_target_v1 *info = par->targinfo;
 
        ADT_OPT(add_opt, XT_FAMILY(par), info->add_set.dim,
-               info->add_set.flags, 0, UINT_MAX);
+               info->add_set.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
        ADT_OPT(del_opt, XT_FAMILY(par), info->del_set.dim,
-               info->del_set.flags, 0, UINT_MAX);
+               info->del_set.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
 
        if (info->add_set.index != IPSET_INVALID_ID)
                ip_set_add(info->add_set.index, skb, CAST_TO_MATCH par,
@@ -437,9 +399,11 @@ set_target_v2(struct sk_buff *skb, const struct xt_action_param *par)
        const struct xt_set_info_target_v2 *info = par->targinfo;
 
        ADT_OPT(add_opt, XT_FAMILY(par), info->add_set.dim,
-               info->add_set.flags, info->flags, info->timeout);
+               info->add_set.flags, info->flags, info->timeout,
+               0, 0, 0, 0);
        ADT_OPT(del_opt, XT_FAMILY(par), info->del_set.dim,
-               info->del_set.flags, 0, UINT_MAX);
+               info->del_set.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
 
        /* Normalize to fit into jiffies */
        if (add_opt.ext.timeout != IPSET_NO_TIMEOUT &&
@@ -469,11 +433,14 @@ set_target_v3(struct sk_buff *skb, const struct xt_action_param *par)
        int ret;
 
        ADT_OPT(add_opt, XT_FAMILY(par), info->add_set.dim,
-               info->add_set.flags, info->flags, info->timeout);
+               info->add_set.flags, info->flags, info->timeout,
+               0, 0, 0, 0);
        ADT_OPT(del_opt, XT_FAMILY(par), info->del_set.dim,
-               info->del_set.flags, 0, UINT_MAX);
+               info->del_set.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
        ADT_OPT(map_opt, XT_FAMILY(par), info->map_set.dim,
-               info->map_set.flags, 0, UINT_MAX);
+               info->map_set.flags, 0, UINT_MAX,
+               0, 0, 0, 0);
 
        /* Normalize to fit into jiffies */
        if (add_opt.ext.timeout != IPSET_NO_TIMEOUT &&
index 4db9e9a54f0504ee73a9df8470497fba90b58ca5..501f63eb11f642e17348631db1acff82d10bc0cc 100644 (file)
@@ -86,6 +86,8 @@
 0 ipset -A test 1.1.1.0/26
 # Check non-matching IP
 1 ipset -T test 1.1.1.1
+# Check non-matching IP with nomatch flag
+0 ipset -T test 1.1.1.1 nomatch
 # Check matching IP from non-matchin small net
 0 ipset -T test 1.1.1.3
 # Check non-matching IP from larger net
index 8bc77efa239c6bec570b08774561f0b6bab90b55..bca3253c6939f2f87468feb990aece528ba69715 100755 (executable)
@@ -116,6 +116,15 @@ netiface)
        $cmd -A OUTPUT -m set --match-set test dst,dst -j LOG --log-prefix "in set netiface: "
        $cmd -A OUTPUT -d 10.255.255.254 -j DROP
        ;;
+counter)
+       $ipset n test hash:ip counters
+       $ipset a test 10.255.255.64
+       $cmd -A OUTPUT -m set --match-set test src --packets-gt 1 ! --update-counters -j DROP
+       $cmd -A OUTPUT -m set --match-set test src -j DROP
+       ./sendip.sh -p ipv4 -id 10.255.255.254 -is 10.255.255.64 -p udp -ud 80 -us 1025 10.255.255.254 >/dev/null 2>&1
+       ./sendip.sh -p ipv4 -id 10.255.255.254 -is 10.255.255.64 -p udp -ud 80 -us 1025 10.255.255.254 >/dev/null 2>&1
+       ./sendip.sh -p ipv4 -id 10.255.255.254 -is 10.255.255.64 -p udp -ud 80 -us 1025 10.255.255.254 >/dev/null 2>&1
+       ;;
 stop)
        $cmd -F
        $cmd -X
index 3d7ed1b9c5d3960a07ba0c8962e7d71d4439049d..4141ce6bddde9f984cd57f5851604c615d784697 100644 (file)
 0 ./check_klog.sh 10.255.255.64 udp 1025 netiface
 # Destroy sets and rules
 0 ./iptables.sh inet stop
+# Create set and rules for the counter test
+0 ./iptables.sh inet counter
+# Check packet counter
+0 ipset l test |grep -q '^10.255.255.64 packets 2'
+# Destroy sets and rules
+0 ./iptables.sh inet stop
 # eof