Re: [PATCH nf V2] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table

Hi Pablo, 2017-03-24 19:37 GMT+08:00 Pablo Neira Ayuso : [...] >> +struct nfnl_cthelper { >> + struct list_headlist; >> + struct nf_conntrack_helper *helper; >> +}; > > I overlook this. Any reason for not using this declaration instead? > > struct

[PATCH nf] netfilter: nfnetlink_queue: fix secctx memory leak

From: Liping Zhang <zlpnob...@gmail.com> We must call security_release_secctx to free the memory returned by security_secid_to_secctx, otherwise memory may be leaked forever. Fixes: ef493bd930ae ("netfilter: nfnetlink_queue: add security context information") Signed-off-by: Lipi

Re: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register

Hi Feng, 2017-03-25 10:24 GMT+08:00 : [...] > @@ -1293,12 +1283,7 @@ static int __init nf_nat_snmp_basic_init(void) > BUG_ON(nf_nat_snmp_hook != NULL); > RCU_INIT_POINTER(nf_nat_snmp_hook, help); > > - ret =

[PATCH nf] netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister

From: Liping Zhang <zlpnob...@gmail.com> If one cpu is doing nf_ct_extend_unregister while another cpu is doing __nf_ct_ext_add_length, then we may hit BUG_ON(t == NULL). Moreover, there's no synchronize_rcu invocation after set nf_ct_ext_types[id] to NULL, so it's possible that we may

[PATCH nf V3] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table

From: Liping Zhang <zlpnob...@gmail.com> The nf_ct_helper_hash table is protected by nf_ct_helper_mutex, while nfct_helper operation is protected by nfnl_lock(NFNL_SUBSYS_CTHELPER). So it's possible that one CPU is walking the nf_ct_helper_hash for cthelper add/get/del, another cpu is

[PATCH nf V2] netfilter: invoke synchronize_rcu after set the _hook_ to NULL

From: Liping Zhang <zlpnob...@gmail.com> Otherwise, another CPU may access the invalid pointer. For example: CPU0CPU1 - rcu_read_lock(); - pfunc = _hook_; _hook_ = NULL; - mod unload - -

[PATCH nf] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

From: Liping Zhang <zlpnob...@gmail.com> cthelpers added via nfnetlink may have the same tuple, i.e. except for the l3proto and l4proto, other fields are all zero. So even with the different names, we will also fail to add them: # nfct helper add ssdp inet udp # nfct helper add tftp in

[PATCH nf V2] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table

From: Liping Zhang <zlpnob...@gmail.com> The nf_ct_helper_hash table is protected by nf_ct_helper_mutex, while nfct_helper operation is protected by nfnl_lock(NFNL_SUBSYS_CTHELPER). So it's possible that one CPU is walking the nf_ct_helper_hash for cthelper add/get/del, another cpu is

[PATCH nf V2] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

From: Liping Zhang <zlpnob...@gmail.com> cthelpers added via nfnetlink may have the same tuple, i.e. except for the l3proto and l4proto, other fields are all zero. So even with the different names, we will also fail to add them: # nfct helper add ssdp inet udp # nfct helper add tftp in

Re: [PATCH nf V2] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

Hi Pablo, 2017-03-29 21:00 GMT+08:00 Liping Zhang <zlpnob...@163.com>: > From: Liping Zhang <zlpnob...@gmail.com> > > cthelpers added via nfnetlink may have the same tuple, i.e. except for > the l3proto and l4proto, other fields are all zero. So even with the > differe

Re: [PATCH nf] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

Hi Pablo, 2017-03-29 18:41 GMT+08:00 Pablo Neira Ayuso : [...] > Wait. > > Just a comestic change, would this look better if we just do: > > hlist_for_each_entry(cur, _ct_helper_hash[h], hnode) { > if (!strcmp(h->name, name) && >

[PATCH nf 5/5] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table

From: Liping Zhang <zlpnob...@gmail.com> The nf_ct_helper_hash table is protected by nf_ct_helper_mutex, while nfct_helper operation is protected by nfnl_lock(NFNL_SUBSYS_CTHELPER). So it's possible that one CPU is walking the nf_ct_helper_hash for cthelper add/get/del, another cpu is

[PATCH nf 1/5] netfilter: nfnl_cthelper: don't report error if NFCTH_PRIV_DATA_LEN is empty

From: Liping Zhang <zlpnob...@gmail.com> Currently, when we create cthelper via nfnetlink, -EINVAL will be returned if the NFCTH_PRIV_DATA_LEN attribute is empty. But enforcing the user to specify the NFCTH_PRIV_DATA_LEN attr seems unnecessary, so it's better to set the helper->data_le

[PATCH nf 4/5] netfilter: nfnl_cthelper: fix memory leak when do update

From: Liping Zhang <zlpnob...@gmail.com> When invoke nfnl_cthelper_update, we will malloc a new expect_policy, then only point the helper->expect_policy to the new one but ignore the old one, so it will be leaked forever. Another issue is that the user can modify the expect_class_max

[PATCH nf 0/5] netfilter: nfnl_cthelper: fix some bugs

From: Liping Zhang <zlpnob...@gmail.com> This patch set aims to fix some bugs related to nfnetlink_cthelper. They are: 1. if NFCTH_PRIV_DATA_LEN attr is empty, we cannot create a cthelper via nfnetlink 2. helper->expect_class_max is incorrect 3. when update cthelper via nfnetlink, me

[PATCH libnetfilter_cthelper] examples: fix double free in nftc-helper-add

From: Liping Zhang <zlpnob...@gmail.com> After inputting the following test command, core dump happened: # ./examples/nfct-helper-add test 1 *** Error in `.../libnetfilter_cthelper/examples/.libs/lt-nfct-helper-add': double free or corruption (fasttop): 0x01

[PATCH nf 3/5] netfilter: drop const qualifier from struct nf_conntrack_expect_policy

From: Liping Zhang <zlpnob...@gmail.com> So we can modify the nf_conntrack_expect_policy directly, the next patch will need this. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- include/net/netfilter/nf_conntrack_helper.h | 4 ++-- net/ipv4/netfilter/nf_nat_snmp_basic.c

[PATCH nf] netfilter: nft_ct: do cleanup work when NFTA_CT_DIRECTION is invalid

From: Liping Zhang <zlpnob...@gmail.com> We should jump to invoke __nft_ct_set_destroy() instead of just return error. Fixes: edee4f1e9245 ("netfilter: nft_ct: add zone id set support") Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- net/netfilter/nft_ct.c | 3

[PATCH nf] netfilter: invoke synchronize_rcu after set the _hook_ to NULL

From: Liping Zhang <zlpnob...@gmail.com> Otherwise, another CPU may access the invalid pointer. For example: CPU0CPU1 - rcu_read_lock(); - pfunc = _hook_; _hook_ = NULL; - mod unload - -

[PATCH libnetfilter_cthelper] src: fix incorrect building and parsing of the NFCTH_POLICY_SETX attribute

From: Liping Zhang <zlpnob...@gmail.com> In nfct_helper_nlmsg_build_policy(), we always set the attribute type to NFCTH_POLICY_SET, so we cannot add more than one nfct_helper_policy to the kernel. Also: in nfct_helper_nlmsg_parse_policy(), we will increase the helper->policy_num

Re: [PATCH nf 1/5] netfilter: nfnl_cthelper: don't report error if NFCTH_PRIV_DATA_LEN is empty

Hi Pablo, 2017-03-21 18:18 GMT+08:00 Pablo Neira Ayuso <pa...@netfilter.org>: > On Sun, Mar 19, 2017 at 10:35:58PM +0800, Liping Zhang wrote: >> From: Liping Zhang <zlpnob...@gmail.com> >> >> Currently, when we create cthelper via nfnetlink, -EINVAL will be >&

Re: [PATCH nf 5/5] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table

2017-03-21 23:26 GMT+08:00 Pablo Neira Ayuso : [...] >> >> After I have a closer look, I find that we do not support netns for the >> nfct_helper currently. So this possible_net_t field is not necessary for >> the time being. > > Oh, I see. This is probably one of the

Re: [PATCH nf 1/2,v3] netfilter: nfnetlink_cthelper: fix runtime expectation policy updates

rg> > --- > v3: Fixed expect_class_max semantics. Compile-tested only. Acked-by: Liping Zhang <zlpnob...@gmail.com> [...] > + /* Check first that all policy attributes are well-formed, so we don't > +* leave things in inconsistent state on errors. > +*/ Good

[PATCH libnetfilter_cthelper] examples: kill the "invalid argument" error in nftc-helper-add

From: Liping Zhang <zlpnob...@gmail.com> NFCTH_PRIV_DATA_LEN is a must attribute required by the kernel when creating the cthelper, add it now. Otherwise -EINVAL will be returned. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- examples/nfct-helper-add.c | 1 + 1 file changed,

Re: [PATCH 05/10] netfilter: nf_tables: fix mismatch in big-endian system

Hi David, 2017-03-16 18:58 GMT+08:00 David Laight : [...] >> For the similar reason, when loading an u16 value from the u32 data >> register, we should use "*(u16 *) sreg;" instead of "(u16)*sreg;", >> the 2nd method will get the wrong value in the big-endian system. > ...

[PATCH nft] hash: generate a random seed if seed option is empty

From: Liping Zhang <zlpnob...@gmail.com> Typing the "nft add rule x y ct mark set jhash ip saddr mod 2" will not generate a random seed, instead, the seed will always be zero. So if seed option is empty, we shoulde not set the NFTA_HASH_SEED attribute, then a random seed

[PATCH nf] netfilter: nft_hash: do not dump the auto generated seed

From: Liping Zhang <zlpnob...@gmail.com> This can prevent the nft utility from printing out the auto generated seed to the user, which is unnecessary and confusing. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- net/netfilter/nft_hash.c | 10 +++--- 1 file changed, 7 inse

Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

2017-04-09 16:26 GMT+08:00 Jan Engelhardt : > > On Sunday 2017-04-09 05:42, Arushi Singhal wrote: >>On Sun, Apr 9, 2017 at 1:44 AM, Pablo Neira Ayuso wrote: >> On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote: >> > On Saturday

[PATCH nf] netfilter: make it safer during the inet6_dev->addr_list traversal

From: Liping Zhang <zlpnob...@gmail.com> inet6_dev->addr_list is protected by inet6_dev->lock, so only using rcu_read_lock is not enough, we should acquire read_lock_bh(>lock) before the inet6_dev->addr_list traversal. Signed-off-by: Liping Zhang <zlpnob...@gmail.co

[PATCH nf] netfilter: nf_ct_expect: use proper RCU list traversal/update APIs

From: Liping Zhang <zlpnob...@gmail.com> We should use proper RCU list APIs to manipulate help->expectations, as we can dump the conntrack's expectations via nfnetlink, i.e. in ctnetlink_exp_ct_dump_table(), where only rcu_read_lock is acquired. So for list trave

[PATCH nf] netfilter: ctnetlink: skip dumping expect when nfct_help(ct) is NULL

From: Liping Zhang <zlpnob...@gmail.com> For IPCTNL_MSG_EXP_GET, if the CTA_EXPECT_MASTER attr is specified, then the NLM_F_DUMP request will dump the expectations related to this connection tracking. But we forget to check whether the conntrack has nf_conn_help or not, so if nfct_h

[PATCH nf] netfilter: ctnetlink: using bit to represent the ct event

From: Liping Zhang <zlpnob...@gmail.com> Otherwise, creating a new conntrack via nfnetlink: # conntrack -I -p udp -s 1.1.1.1 -d 2.2.2.2 -t 10 --sport 10 --dport 20 will emit the wrong ct events(where UPDATE should be NEW): # conntrack -E [UPDATE] udp 17 10 src=1.1.1.1 dst=2

[PATCH nf] netfilter: ctnetlink: remove unnecessary nf_conntrack_expect_lock protection

From: Liping Zhang <zlpnob...@gmail.com> Currently, ctnetlink_change_helper() is always protected by _expect_lock, this is unnecessary, since the operations are unrelated to _expect_lock. Also this will cause a deadlock when deleting the helper from a conntrack, as _expect_lock will be

[PATCH nf] netfilter: ctnetlink: make it safer when checking the ct helper name

From: Liping Zhang <zlpnob...@gmail.com> One CPU is doing ctnetlink_change_helper(), while another CPU is doing unhelp() at the same time. So even if help->helper is not NULL at first, the later statement strcmp(help->helper->name, ...) may still access the NULL pointer.

[PATCH nf] netfilter: ctnetlink: make it safer when updating ct->status

From: Liping Zhang <zlpnob...@gmail.com> User can update the ct->status via nfnetlink, but using a non-atomic operation "ct->status |= status;". This is unsafe, and may clear IPS_DYING_BIT bit set by another CPU unexpectedly. For example: CPU0

Re: [PATCH nf] netfilter: ctnetlink: remove unnecessary nf_conntrack_expect_lock protection

Hi Pablo, 2017-04-10 20:02 GMT+08:00 Pablo Neira Ayuso : [...] >> Since ctnetlink_change_conntrack is unrelated to nf_conntrack_expect_lock, >> so remove it can fix this issue. > > But packets may be updating a conntrack at the same time that we're > mangling via ctnetlink,

Re: [PATCH nf] netfilter: ctnetlink: remove unnecessary nf_conntrack_expect_lock protection

Hi Pablo, 2017-04-09 5:16 GMT+08:00 Pablo Neira Ayuso <pa...@netfilter.org>: > On Sat, Apr 01, 2017 at 10:14:24PM +0800, Liping Zhang wrote: >> @@ -1960,9 +1955,7 @@ static int ctnetlink_new_conntrack(struct net *net, >> struct sock *ctnl, >>

[PATCH nf 0/2] netfilter: reject cthelper del request if it is in use

From: Liping Zhang <zlpnob...@gmail.com> User can still delete the cthelper even if it is in use: # nfct helper add ssdp inet udp # iptables -t raw -A OUTPUT -p udp -j CT --helper ssdp # nfct helper delete ssdp //--> succeed! This will cause a use-after-free error. So we s

[PATCH nf 1/2] netfilter: introduce nf_conntrack_helper_put helper function

From: Liping Zhang <zlpnob...@gmail.com> And convert module_put invocation to nf_conntrack_helper_put, this is prepared for the later patch, which will add a refcnt for cthelper. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- include/net/netfilter/nf_conntrack_helper.h

[PATCH nf 2/2] netfilter: nfnl_cthelper: reject del request if helper obj is in use

From: Liping Zhang <zlpnob...@gmail.com> We can still delete the ct helper even if it is in use, this will cause a use-after-free error. In more detail, I mean: # nfct helper add ssdp inet udp # iptables -t raw -A OUTPUT -p udp -j CT --helper ssdp # nfct helper delete ssdp //-->

Re: [PATCH nf] netfilter: ctnetlink: make it safer when updating ct->status

Hi Feng, 2017-04-13 10:42 GMT+08:00 Gao Feng : [...] >> +static void >> +__ctnetlink_change_status(struct nf_conn *ct, unsigned long on, >> + unsigned long off) >> +{ >> + unsigned long mask; >> + unsigned int bit; >> + >> + for (bit = 0;

Re: [PATCH nf] netfilter: ctnetlink: make it safer when updating ct->status

Hi Feng, 2017-04-13 11:22 GMT+08:00 Gao Feng : [...] >> No, it's better to do this together, there are two invocations, it's not >> good to >> copy these codes twice. > > You mean " on &= ~ IPS_UNCHANGEABLE_MASK " and " off &= ~ > IPS_UNCHANGEABLE_MASK " seems

[PATCH nf V3] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

From: Liping Zhang <zlpnob...@gmail.com> cthelpers added via nfnetlink may have the same tuple, i.e. except for the l3proto and l4proto, other fields are all zero. So even with the different names, we will also fail to add them: # nfct helper add ssdp inet udp # nfct helper add tftp in

Re: [PATCH nf V2] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

Hi Pablo, 2017-04-15 17:28 GMT+08:00 Pablo Neira Ayuso : > Hm, this patch requires no changes actually. Now I understand why > you're confused there. > > So let me know if you I should just take this or wait for you to > resubmit. > > In case of doubt, resubmitting is just

[PATCH nft V2] hash: generate a random seed if seed option is empty

From: Liping Zhang <zlpnob...@gmail.com> Typing the "nft add rule x y ct mark set jhash ip saddr mod 2" will not generate a random seed, instead, the seed will always be zero. So if seed option is empty, we shoulde not set the NFTA_HASH_SEED attribute, then a random seed w

[PATCH nf 2/4] netfilter: ctnetlink: fix deadlock due to acquire _expect_lock twice

From: Liping Zhang <zlpnob...@gmail.com> Currently, ctnetlink_change_conntrack is always protected by _expect_lock, but this will cause a deadlock when deleting the helper from a conntrack, as the _expect_lock will be acquired again by nf_ct_remove_expectations: CPU0

[PATCH nf 3/4] netfilter: ctnetlink: make it safer when updating ct->status

From: Liping Zhang <zlpnob...@gmail.com> After converting to use rcu for conntrack hash, one CPU may update the ct->status via ctnetlink, while another CPU may process the packets and update the ct->status. So the non-atomic operation "ct->status |= status;" via

[PATCH nf 4/4] netfilter: ctnetlink: acquire ct->lock before operating nf_ct_seqadj

From: Liping Zhang <zlpnob...@gmail.com> We should acquire the ct->lock before accessing or modifying the nf_ct_seqadj, as another CPU may modify the nf_ct_seqadj at the same time during its packet proccessing. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> ---

[PATCH nf 0/4] netfilter: ctnetlink: fix some bugs related to ct update

From: Liping Zhang <zlpnob...@gmail.com> This patch set aims to fix some bugs related to ctnetlink_change_conntrack. First, we may invoke request_module with rcu_read_lock held, this is wrong, as the request_module invocation may sleep. Fixed by PATCH #1. Second, the unnec

[PATCH nf 1/4] netfilter: ctnetlink: drop the incorrect cthelper module request

From: Liping Zhang <zlpnob...@gmail.com> First, when creating a new ct, we will invoke request_module to try to load the related inkernel cthelper. So there's no need to call the request_module again when updating the ct helpinfo. Second, ctnetlink_change_helper may be called with rcu_rea

Re: [PATCH nft] hash: generate a random seed if seed option is empty

Hi Pablo, 2017-04-14 4:57 GMT+08:00 Pablo Neira Ayuso : [...] >> - nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); >> + if (expr->hash.seed) >> + nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); > > I prefer we have a

Re: [PATCH nf V2] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

Hi Pablo, 2017-04-14 6:29 GMT+08:00 Pablo Neira Ayuso : [...] >> After I have a closer look, inside hlist_for_each_entry_rcu, we use the >> rcu_dereference_raw() to get the pointer, and this will not generate warning: >> >> #define hlist_for_each_entry_rcu(pos, head, member)

Re: [PATCH nf 2/2] netfilter: nfnl_cthelper: reject del request if helper obj is in use

Hi Pablo, 2017-04-14 7:16 GMT+08:00 Pablo Neira Ayuso : [...] >> #ifndef _NF_CONNTRACK_HELPER_H >> #define _NF_CONNTRACK_HELPER_H >> +#include >> #include >> #include >> #include >> @@ -26,6 +27,7 @@ struct nf_conntrack_helper { >> struct hlist_node hnode;

Re: [PATCH nf V2] netfilter: ctnetlink: make it safer when updating ct->status

Hi Pablo, 2017-04-14 7:25 GMT+08:00 Pablo Neira Ayuso <pa...@netfilter.org>: > On Thu, Apr 13, 2017 at 08:53:47PM +0800, Liping Zhang wrote: >> From: Liping Zhang <zlpnob...@gmail.com> >> >> User can update the ct->status via nfnetlink, but using a non-atomi

[PATCH nf V2] netfilter: ctnetlink: make it safer when updating ct->status

From: Liping Zhang <zlpnob...@gmail.com> User can update the ct->status via nfnetlink, but using a non-atomic operation "ct->status |= status;". This is unsafe, and may clear IPS_DYING_BIT bit set by another CPU unexpectedly. For example: CPU0

[PATCH nf] netfilter: nft_dynset: continue to next expr if _OP_ADD succeeded

From: Liping Zhang <zlpnob...@gmail.com> Currently, after adding the following nft rules: # nft add set x target1 { type ipv4_addr \; flags timeout \;} # nft add rule x y set add ip daddr timeout 1d @target1 counter the counters will always be zero despite of the elements are

Re: [PATCH nf 1/1] netfilter: h323,sip: Fix possible dead loop in nat_rtp_rtcp and nf_nat_sdp_media

Hi, 2017-03-02 15:57 GMT+08:00 : > From: Gao Feng > > When h323 and sip try to insert expect nodes, they would increase > the port by 2 for loop, and the loop condition is that "port != 0". > So when the start port is odd number, port never increases to zero.

[PATCH nf] netfilter: nf_tables: fix missmatch in big-endian system

From: Liping Zhang <zlpnob...@gmail.com> Currently, there are two different methods to store an u16 integer to the u32 data register. For example: u32 *dest = >data[priv->dreg]; 1. *dest = 0; *(u16 *) dest = val_u16; 2. *dest = val_u16; For method 1, the u16 value will be

Re: [PATCH nf] netfilter: nf_tables: fix missmatch in big-endian system

2017-03-09 18:36 GMT+08:00 Arturo Borrero Gonzalez <art...@debian.org>: > On 8 March 2017 at 15:54, Liping Zhang <zlpnob...@163.com> wrote: >> >> +/* Store/load an u16 or u8 integer to/from the u32 data register. >> + * >> + * Note, when using concatenations

Re: [PATCH 1/2 nf] netfilter: nft_set_bitmap: keep a list of dummy elements

Hi Pablo, 2017-03-13 20:27 GMT+08:00 Pablo Neira Ayuso : > Element comments may come without any prior set flag, so we have to keep > a list of dummy struct nft_set_ext to keep this information around. This > is only useful for set dumps to userspace. From the packet path,

Re: [PATCH 1/2 nf] netfilter: nft_set_bitmap: keep a list of dummy elements

Hi Pablo, 2017-03-14 1:23 GMT+08:00 Pablo Neira Ayuso : [...] > I would like this only describes the representation that is exposed to > the packet path, not the real memory consumption of it. I know I'm > kind of cheating, but with this I'm also giving this bitmap an >

Re: [PATCH 1/2 nf] netfilter: nft_set_bitmap: keep a list of dummy elements

Hi Pablo, 2017-03-14 20:19 GMT+08:00 Pablo Neira Ayuso : [...] > Another possibility is to simply regard desc->size over the memory > scalability notation when provided. I think this just needs an update > from nft userspace. Look, bitmap and hashtable are both described as >

[PATCH nf-next] netfilter: nft_set_rbtree: use per-set rwlock to improve the scalability

From: Liping Zhang <zlpnob...@gmail.com> Karel Rericha reported that in his test case, ICMP packets going through boxes had normally about 5ms latency. But when running nft, actually listing the sets with interval flags, latency would go up to 30-100ms. This was observed when router thro

[PATCH nft] src: fix crash when inputting an incomplete set add command

From: Liping Zhang <zlpnob...@gmail.com> After inputting the following nft command, set->keytype is not initialized but we try to destroy it, so NULL pointer dereference will happen: # nft add set t s Segmentation fault (core dumped) #0 dtype_free (dtype=0x0) at datatype.c:

[PATCH nf-next] netfilter: limit: use per-rule spinlock to improve the scalability

From: Liping Zhang <zlpnob...@gmail.com> The limit token is independent between each rules, so there's no need to use a global spinlock. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- net/netfilter/nft_limit.c | 10 +- net/netfilter/xt_limit.c | 11 ++- 2 f

Re: [PATCH nf 1/2] netfilter: nft_set_bitmap: fetch the element key based on the set->klen

Hi Pablo, 2017-03-06 20:01 GMT+08:00 Pablo Neira Ayuso : [...] > Right, the userdata case is not handled properly. And in this case we > have no specific flag at set creation, so element comments may come > without previous notice via set flags. > > I think we have to keep a

[PATCH nf 1/2] netfilter: nft_set_bitmap: fetch the element key based on the set->klen

From: Liping Zhang <zlpnob...@gmail.com> Currently we just assume the element key as a u32 integer, regardless of the set key length. This is incorrect, for example, the tcp port number is only 16 bits. So when we use the nft_payload expr to get the tcp dport and store it to dreg, the

[PATCH nf 2/2] netfilter: nft_set_bitmap: fix memory leak

From: Liping Zhang <zlpnob...@gmail.com> Unlike other set types, we will not link struct nft_set_ext into our internal structure, so we need to free it after activation, otherwise the nft_set_ext's memory will be leaked forever. Also move nf_tables_setelem_notify() to the front of ops->

[PATCH nf-next] netfilter: nf_tables: validate the expr explicitly after init successfully

From: Liping Zhang <zlpnob...@gmail.com> When we want to validate the expr's dependency or hooks, we must do two things to accomplish it. First, write a X_validate callback function and point ->validate to it. Second, call X_validate in init routine. This is very common, such as fib, na

Re: [PATCH nf 1/1] netfilter: h323,sip: Fix possible dead loop in nat_rtp_rtcp and nf_nat_sdp_media

Hi, 2017-03-02 18:18 GMT+08:00 Gao Feng : [...] > The expect class is NF_CT_EXPECT_CLASS_DEFAULT, and proto is > IPPROTO_UDP at the function "expect_rtp_rtcp", > And it makes sure the port is even number. > > But look at the process_gcf, the port is got from the packet data at >

Re: [PATCH nf-next v2 2/2] netfilter: nft_hash: support of symmetric hash

Hi, 2017-03-01 1:38 GMT+08:00 Laura Garcia Liebana : [...] > +static const struct nft_expr_ops * > +nft_hash_select_ops(const struct nft_ctx *ctx, > + const struct nlattr * const tb[]) > +{ > + u32 type; > + > + if (!tb[NFTA_HASH_TYPE]) > +

Re: [PATCH nf] netfilter: nft_hash: do not dump the auto generated seed

Hi Laura, 2017-04-08 5:19 GMT+08:00 Laura García Liébana <laura.gar...@zevenet.com>: > On Mon, Apr 3, 2017 at 10:34 AM, Liping Zhang <zlpnob...@163.com> wrote: >> >> From: Liping Zhang <zlpnob...@gmail.com> >> >> This can prevent the nft utility

[PATCH nf] netfilter: xt_CT: fix cthelper module's refcnt leak

From: Liping Zhang <zlpnob...@gmail.com> We should call module_put when the time policy is not found. Otherwise, the related cthelper module cannot be removed anymore. It is easy to reproduce by typing the following command: # iptables -t raw -A OUTPUT -p tcp -j CT --helper ftp --timeo

[PATCH nf-next] netfilter: nf_conntrack: make nf_conntrack_max as an unsigned int knob

From: Liping Zhang <zlpnob...@gmail.com> It doesn't work when we set a large value to the nf_conntrack_max, as well as the nf_conntrack_expect_max: # echo 4294967295 > /proc/sys/net/nf_conntrack_max bash: echo: write error: Invalid argument So convert to use proc_douintvec. S

Re: [PATCH nf 2/5] netfilter: nfnl_cthelper: fix incorrect helper->expect_class_max

Hi Pablo, 2017-03-21 18:27 GMT+08:00 Pablo Neira Ayuso : [...] >> + class_max = ntohl(nla_get_be32(tb[NFCTH_POLICY_SET_NUM])); >> + if (class_max == 0) >> + return -EINVAL; > > I think this patch is just fixing up this case. We should always > provide a

Re: [PATCH nf 5/5] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table

2017-03-21 18:33 GMT+08:00 Pablo Neira Ayuso <pa...@netfilter.org>: > On Sun, Mar 19, 2017 at 10:36:02PM +0800, Liping Zhang wrote: >> From: Liping Zhang <zlpnob...@gmail.com> >> >> The nf_ct_helper_hash table is protected by nf_ct_helper_mutex, while >

Re: [PATCH nf 5/5] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table

Hi Pablo, 2017-03-21 22:48 GMT+08:00 Liping Zhang <zlpnob...@gmail.com>: > 2017-03-21 18:33 GMT+08:00 Pablo Neira Ayuso <pa...@netfilter.org>: >>> +struct nfnl_cthelper { >>> + struct list_headlist; >>> + struct nf_conntrack_help

Re: [PATCH nf-next 2/3] netfilter: nf_log: don't call synchronize_rcu in nf_log_unset

Hi Florian, 2017-04-24 21:37 GMT+08:00 Florian Westphal : > nf_log_unregister() (which is what gets called in the logger backends > module exit paths) does a (required, module is removed) synchronize_rcu(). > > But nf_log_unset() is only called from pernet exit handlers. It

[PATCH nf-next] netfilter: nf_ct_ext: invoke destroy even when ext is not attached

From: Liping Zhang <zlpnob...@gmail.com> For NF_NAT_MANIP_SRC, we will insert the ct to the nat_bysource_table, then remove it from the nat_bysource_table via nat_extend->destroy. But now, the nat extension is attached on demand, so if the nat extension is not attached, we will not be

[PATCH nf] netfilter: nf_tables: can't assume lock is acquired when dumping set elems

From: Liping Zhang <zlpnob...@gmail.com> When dumping the elements related to a specified set, we may invoke the nf_tables_dump_set with the NFNL_SUBSYS_NFTABLES lock not acquired. So we should use the proper rcu operation to avoid race condition, just like other nft dump operations. Sign

[PATCH nft] src: delete the old cache when dumping is interrupted

From: Liping Zhang <zlpnob...@gmail.com> When the dumping operation is interrupted, we will restart the cache_init(), but unfortunatly, we forget to delete the old cache. So in extreme case, we will leak a huge amount of memory. Running the following commands can simulate the extrem

Re: [PATCH nf] netfilter: ctnetlink: fix incorrect nf_ct_put during hash resize

2017-05-24 6:28 GMT+08:00 Florian Westphal : > Pablo Neira Ayuso wrote: [...] >> I will append the Fixes: tag: >> >> Fixes: 89f2e21883b5 ("[NETFILTER]: ctnetlink: change table dumping not to >> require an unique ID") > > That commit looks fine to me, it seems

Re: [PATCH nf] netfilter: ctnetlink: fix incorrect nf_ct_put during hash resize

Hi Florian, 2017-05-21 8:00 GMT+08:00 Florian Westphal : [...] > Yes, you're right, seems this was added in > 93bb0ceb75be2fdfa9fc0dd1fb522d9ada515d9c (it adds the 'goto out'). I added some trace logs, and when the hash size reduced, for example, from 6 to 500, then the issue

[PATCH nf-next 2/3] netfilter: nf_ct_helper: use nf_ct_iterate_cleanup to unlink helper objs

From: Liping Zhang <zlpnob...@gmail.com> When we unlink the helper objects, we will iterate the nf_conntrack_hash, iterate the unconfirmed list, handle the hash resize situation, etc. Actually this logic is same as the nf_ct_iterate_cleanup, so we can use it to remove these copy & p

[PATCH nf-next 0/3] netfilter: handle hash resize situation in nf_ct_iterate_cleanup

From: Liping Zhang <zlpnob...@gmail.com> First, when we do nf ct cleanup, we should also handle the hash resize situation, so we will not miss the related conntracks, this is important for module removal. After we accomplish this, we can use nf_ct_iterate_cleanup to remove these copy &

[PATCH nf-next 3/3] netfilter: cttimeout: use nf_ct_iterate_cleanup to unlink timeout objs

From: Liping Zhang <zlpnob...@gmail.com> Similar to nf_conntrack_helper, we can use nf_ct_iterare_cleanup to remove these copy & paste codes. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- net/netfilter/nfnetlink_cttimeout.c | 39 + 1

[PATCH nf-next 1/3] netfilter: restart nf ct cleanup if hash resize happen

From: Liping Zhang <zlpnob...@gmail.com> Similar to commit 474803d37e7f ("netfilter: cttimeout: unlink timeout obj again when hash resize happen"), when hash resize happen, we should try to do cleanup work from the 0#bucket again, so we will never miss the conntrack entries which

Re: [PATCH nf-next 2/3] netfilter: nf_ct_helper: use nf_ct_iterate_cleanup to unlink helper objs

Hi Florian, 2017-05-21 16:15 GMT+08:00 Florian Westphal : [...] > this is broken for unconfirmed conntracks, as > other cpu can reallocate the extension area. Right, I missed this point, thanks for your reminder. > For the module removal case, we have no choice but to toss the >

Re: [PATCH nf-next 2/3] netfilter: nf_ct_helper: use nf_ct_iterate_cleanup to unlink helper objs

Hi Florian, 2017-05-21 18:31 GMT+08:00 Florian Westphal <f...@strlen.de>: > Liping Zhang <zlpnob...@gmail.com> wrote: >> Hi Florian, >> >> 2017-05-21 16:15 GMT+08:00 Florian Westphal <f...@strlen.de>: >> [...] >> > this is broken for un

[PATCH nf] netfilter: nat: use atomic bit op to clear the _SRC_NAT_DONE_BIT

From: Liping Zhang <zlpnob...@gmail.com> We need to clear the IPS_SRC_NAT_DONE_BIT to indicate that the ct has been removed from nat_bysource table. But unfortunately, we use the non-atomic bit operation: "ct->status &= ~IPS_NAT_DONE_MASK". So there's a race cond

[PATCH nf-next V2] netfilter: cttimeout: use nf_ct_iterate_cleanup_net to unlink timeout objs

From: Liping Zhang <zlpnob...@gmail.com> Similar to nf_conntrack_helper, we can use nf_ct_iterare_cleanup_net to remove these copy & paste codes. Signed-off-by: Liping Zhang <zlpnob...@gmail.com> --- V2: rebase on Florian's patch set "netfilter: conntrack: rework nf_ct

[PATCH nf-next V2] netfilter: nf_ct_helper: use nf_ct_iterate_destroy to unlink helper objs

From: Liping Zhang <zlpnob...@gmail.com> When we unlink the helper objects, we will iterate the nf_conntrack_hash, iterate the unconfirmed list, handle the hash resize situation, etc. Actually this logic is same as the nf_ct_iterate_destroy, so we can use it to remove these copy & p

Re: [PATCH nf-next 7/9] netfilter: nf_tables: allow large allocations for new sets

Hi Pablo, 2017-05-24 17:50 GMT+08:00 Pablo Neira Ayuso : [...] > - err = -ENOMEM; > - set = kzalloc(sizeof(*set) + size + udlen, GFP_KERNEL); > + alloc_size = sizeof(*set) + size + udlen; > + if (alloc_size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) >

Re: [PATCH nf-next 7/9] netfilter: nf_tables: allow large allocations for new sets

2017-05-26 18:18 GMT+08:00 Pablo Neira Ayuso <pa...@netfilter.org>: > On Fri, May 26, 2017 at 06:02:34PM +0800, Liping Zhang wrote: >> Hi Pablo, >> >> 2017-05-24 17:50 GMT+08:00 Pablo Neira Ayuso <pa...@netfilter.org>: >> [...] >> > - err =

Re: [PATCH nf-next V2] netfilter: cttimeout: use nf_ct_iterate_cleanup_net to unlink timeout objs

Hi, 2017-05-29 0:07 GMT+08:00 kbuild test robot : >net/netfilter/nfnetlink_cttimeout.c: In function 'ctnl_untimeout': >>> net/netfilter/nfnetlink_cttimeout.c:303:2: error: implicit declaration of >>> function 'nf_ct_iterate_cleanup_net' [-Werror=implicit-function-declaration]

Re: [PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

Hi Pablo, 2017-06-06 8:04 GMT+08:00 Pablo Neira Ayuso : [...] >> I remembered Pablo told me that the ct helpers "is probably one of >> the remaining subsystems not having netns support", when I sent >> patches to fix other issues. >> >> So I try to accomplish the netns

[PATCH nf] netfilter: nf_ct_dccp/sctp: fix memory leak after netns cleanup

From: Liping Zhang <zlpnob...@gmail.com> After running the following commands for a while, kmemleak reported that "1879 new suspected memory leaks" happened: # while : ; do ip netns add test ip netns delete test done unreferenced object 0x88006342fa38 (size 1024):

[PATCH nf-next RFC 3/5] netfilter: make each ct helper belong to a specific netns

From: Liping Zhang <zlpnob...@gmail.com> This is the first part to support net namespace for ct helpers. When we register a ct helper, we will store the related netns. So later, we can only find the ct helper belong to a specified netns, i.e. we will add "struct net *

[PATCH nf-next RFC 1/5] netfilter: use nf_conntrack_helpers_register when possible

From: Liping Zhang <zlpnob...@gmail.com> amanda_helper, nf_conntrack_helper_ras and nf_conntrack_helper_q931 are all arrays, so we can use nf_conntrack_helpers_register to register the ct helper, this will help us to eliminate some "goto errX" statements. Also introduce h323_

[PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

OUTPUT -p udp -j CT --helper sip-0 rmmod nf_conntrack_sip rmmod nf_conntrack_ftp rmmod nf_conntrack_tftp done Liping Zhang (5): netfilter: use nf_conntrack_helpers_register when possible netfilter: make nf_conntrack_helper_register become per-net netfilter: make each ct helper belong

<    1   2   3   4   >