From ad92ed77e77fe421a86f0fde907c51286ed47928 Mon Sep 17 00:00:00 2001 From: Jozsef Kadlecsik Date: Thu, 4 Jan 2018 13:21:26 +0100 Subject: Fix "don't update counters" mode when counters used at the matching The matching of the counters was not taken into account, fixed. --- kernel/include/linux/netfilter/ipset/ip_set.h | 6 ++ .../include/linux/netfilter/ipset/ip_set_counter.h | 25 +++-- kernel/net/netfilter/ipset/ip_set_bitmap_gen.h | 9 +- kernel/net/netfilter/ipset/ip_set_core.c | 25 +++++ kernel/net/netfilter/ipset/ip_set_hash_gen.h | 37 +++---- kernel/net/netfilter/ipset/ip_set_list_set.c | 21 ++-- kernel/net/netfilter/xt_set.c | 119 ++++++++------------- tests/hash:net.t | 2 + tests/iptables.sh | 9 ++ tests/match_target.t | 6 ++ 10 files changed, 131 insertions(+), 128 deletions(-) diff --git a/kernel/include/linux/netfilter/ipset/ip_set.h b/kernel/include/linux/netfilter/ipset/ip_set.h index 3e44e66..3986e63 100644 --- a/kernel/include/linux/netfilter/ipset/ip_set.h +++ b/kernel/include/linux/netfilter/ipset/ip_set.h @@ -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) diff --git a/kernel/include/linux/netfilter/ipset/ip_set_counter.h b/kernel/include/linux/netfilter/ipset/ip_set_counter.h index 8bf9192..a0d134a 100644 --- a/kernel/include/linux/netfilter/ipset/ip_set_counter.h +++ b/kernel/include/linux/netfilter/ipset/ip_set_counter.h @@ -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 diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h b/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h index aa3dbb1..0c9db19 100644 --- a/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h @@ -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 diff --git a/kernel/net/netfilter/ipset/ip_set_core.c b/kernel/net/netfilter/ipset/ip_set_core.c index 32b7c0c..134e708 100644 --- a/kernel/net/netfilter/ipset/ip_set_core.c +++ b/kernel/net/netfilter/ipset/ip_set_core.c @@ -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. diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h index d24ee3c..291c7d4 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h +++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h @@ -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; diff --git a/kernel/net/netfilter/ipset/ip_set_list_set.c b/kernel/net/netfilter/ipset/ip_set_list_set.c index 55ecea2..e8c67dc 100644 --- a/kernel/net/netfilter/ipset/ip_set_list_set.c +++ b/kernel/net/netfilter/ipset/ip_set_list_set.c @@ -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; } diff --git a/kernel/net/netfilter/xt_set.c b/kernel/net/netfilter/xt_set.c index 01868aa..a7ac913 100644 --- a/kernel/net/netfilter/xt_set.c +++ b/kernel/net/netfilter/xt_set.c @@ -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 && diff --git a/tests/hash:net.t b/tests/hash:net.t index 4db9e9a..501f63e 100644 --- a/tests/hash:net.t +++ b/tests/hash:net.t @@ -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 diff --git a/tests/iptables.sh b/tests/iptables.sh index 8bc77ef..bca3253 100755 --- a/tests/iptables.sh +++ b/tests/iptables.sh @@ -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 diff --git a/tests/match_target.t b/tests/match_target.t index 3d7ed1b..4141ce6 100644 --- a/tests/match_target.t +++ b/tests/match_target.t @@ -112,4 +112,10 @@ 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 -- cgit v1.2.3