Hi Laura,
2017-04-08 5:19 GMT+08:00 Laura García Liébana :
> On Mon, Apr 3, 2017 at 10:34 AM, Liping Zhang wrote:
>>
>> From: Liping Zhang
>>
>> This can prevent the nft utility from printing out the auto generated
>> seed to the user, which is unnecessary a
From: Liping Zhang
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 --timeout xxx
Signed-off-by: Liping
From: Liping Zhang
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.
Signed-off-by: Liping Zhang
Hi Pablo,
2017-04-09 5:16 GMT+08:00 Pablo Neira Ayuso :
> 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,
>> err = -EEXIST;
>> ct =
From: Liping Zhang
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
---
include/net/netfilter/nf_conntrack_helper.h | 2 ++
net/netfilter/nf_conntrack_helper.c | 6
From: Liping Zhang
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 shoule add a refcnt to
fix this is
From: Liping Zhang
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 //--> succeed!
So add reference co
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 2017-04-08 19:21, Arushi Singhal wrote:
>>
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, right?
Yes, but in pac
From: Liping Zhang
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:
CPU0CPU1
ctnetli
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; bit < __IPS_MAX_BIT; bit+
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 duplicated?
I see. I misunderstood
From: Liping Zhang
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:
CPU0CPU1
ctnetli
go, I'd appreciate.
Feng, since you spotted this issue earlier, can you send a new patch to do this?
With a new patch name: "netfilter: xt_CT: fix refcnt leak on error path".
Also you can add my:
Signed-off-by: Liping Zhang
--
To unsubscribe from this list: send the line &quo
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 hash.seed_set, instead of rel
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;/* Internal use. */
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) \
>> for (pos = hl
Hi Pablo,
2017-04-14 7:25 GMT+08:00 Pablo Neira Ayuso :
> On Thu, Apr 13, 2017 at 08:53:47PM +0800, Liping Zhang wrote:
>> From: Liping Zhang
>>
>> User can update the ct->status via nfnetlink, but using a non-atomic
>> operation "ct->status |= s
Hi Jarno,
2017-04-14 7:05 GMT+08:00 Jarno Rajahalme :
[...]
> + h = nf_conntrack_find_get(net, zone, &tuple);
> + if (h) {
> + struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
> +
> + if (nf_ct_is_confirmed(ct))
If the _ct_
From: Liping Zhang
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 will be generated in the kernel.
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 fine. Thanks!
I will re
From: Liping Zhang
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 inet udp
nfct v1.4.3
Hi Florian,
2017-04-15 18:26 GMT+08:00 Florian Westphal :
[...]
> +#ifdef CONFIG_NF_CONNTRACK_EVENTS
> + case NFT_CT_EVENTMASK: {
> + struct nf_conntrack_ecache *e = nf_ct_ecache_find(ct);
> + u16 ctmask = nft_reg_load16(®s->data[priv->sreg]);
Hmm, I find that in
From: Liping Zhang
We forget to free dummy elements when deleting the set. So when I was
running nft-test.py, I saw many kmemleak warnings:
kmemleak: 1344 new suspected memory leaks ...
# cat /sys/kernel/debug/kmemleak
unreferenced object 0x8800631345c8 (size 32):
comm "nft&
From: Liping Zhang
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
lock
From: Liping Zhang
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 ctnetlink
becomes unsafe, and
From: Liping Zhang
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 unnecessary nf_conntrack_expect_lock will cause
From: Liping Zhang
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_read_lock held
From: Liping Zhang
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
---
net/netfilter/nf_conntrack_netlink.c | 21 +++--
1 f
From: Liping Zhang
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 added
to the dynamic set "ta
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 doesn't
> free any memo
From: Liping Zhang
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 notified when the ct
Hi Feng,
2017-05-05 8:55 GMT+08:00 :
[...]
> +static void
> +nf_ct_flush_expect(const struct module *me)
> +{
> + struct nf_conntrack_expect *exp;
> + const struct hlist_node *next;
> + u32 i;
> +
> + if (!me)
> + return;
> +
> + /* Get rid of expectati
From: Liping Zhang
We cannot setup nat info if the ct has been confirmed already, else,
different cpu may race to handle the same ct. In extreme situation,
we may hit the "BUG_ON(nf_nat_initialized(ct, maniptype))" in the
nf_nat_setup_info.
Also running the following commands will
From: Liping Zhang
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 //--> oops, succeed!
BUG: unable
From: Liping Zhang
And convert module_put invocation to nf_conntrack_helper_put, this is
prepared for the followup patch, which will add a refcnt for cthelper,
so we can reject the deleting request when cthelper is in use.
Signed-off-by: Liping Zhang
---
V2: update nft_ct helper obj
include
From: Liping Zhang
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 should add a refcnt to
fix this is
From: Liping Zhang
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 extreme case:
# nft add table t
From: Liping Zhang
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.
Signed-off-by: Liping Zhang
From: Liping Zhang
If nf_conntrack_htable_size was adjusted by the user during the ct
dump operation, we may invoke nf_ct_put twice for the same ct, i.e.
the "last" ct. This will cause the ct will be freed but still linked
in hash buckets.
It's very easy to reproduce the
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 would happen.
A
From: Liping Zhang
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 & paste codes.
Signed-off-by: Li
From: Liping Zhang
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 & paste codes, which are use
From: Liping Zhang
Similar to nf_conntrack_helper, we can use nf_ct_iterare_cleanup to
remove these copy & paste codes.
Signed-off-by: Liping Zhang
---
net/netfilter/nfnetlink_cttimeout.c | 39 +
1 file changed, 5 insertions(+), 34 deletions(-)
diff -
From: Liping Zhang
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 we are intrested i
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
> unconfirmed conn
Hi Florian,
2017-05-21 18:31 GMT+08:00 Florian Westphal :
> Liping Zhang wrote:
>> Hi Florian,
>>
>> 2017-05-21 16:15 GMT+08:00 Florian Westphal :
>> [...]
>> > this is broken for unconfirmed conntracks, as
>> > other cpu can reallocate the ext
From: Liping Zhang
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 condition that we may clear the _DY
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 to make sure to put
> "last" only onc
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))
> + set =
2017-05-26 18:18 GMT+08:00 Pablo Neira Ayuso :
> 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 :
>> [...]
>> > - err = -ENOMEM;
>> > - set = kz
From: Liping Zhang
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 & paste codes.
Signed-off-by: Li
From: Liping Zhang
Similar to nf_conntrack_helper, we can use nf_ct_iterare_cleanup_net to
remove these copy & paste codes.
Signed-off-by: Liping Zhang
---
V2: rebase on Florian's patch set "netfilter: conntrack: rework nf_ct_iterate,
part 1."
net/netfilter/nfnetli
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]
> nf_ct_ite
From: Liping Zhang
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):
comm "ip"
From: Liping Zhang
First, we should make the global nfnl_cthelper_list become per-net,
so different netns's user cthelpers will be linked to the different
global lists.
Second, when we do the netns cleanup work, we may invoke the
nfnl_cthelper_net_exit and nf_conntrack_helper_put in diff
From: Liping Zhang
In order to support net namespace for these built-in cthelpers, we
must kmemdup the nf_conntrack_helper and the related _expect_policy
before we insert them to the nf_ct_helper_hash. Then free them after
unregistration. These are all done by helper_register/unregister.
But
From: Liping Zhang
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 *" parameter to these
ct_helper_find fun
From: Liping Zhang
Now we add "struct net *" parameter to the nf_conntrack_helper_register/
unregister function, and make the kernel built-in ct helpers to use
pernet subsys operation.
Also note, after this patch, we only support ct helper register in
&init_net netns, but the fo
From: Liping Zhang
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_helper_init/exit helper f
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
Hi Florian & Pablo,
2017-06-05 0:07 GMT+08:00 Florian Westphal :
> Liping Zhang wrote:
>> This patch set aims to add net namespace support for the ct helper,
>> it is a little large, but I try my best to split them to a relative
>> smaller patches, which will help to rev
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 support for ct helpers.
>> (
From: Liping Zhang
"struct nf_loginfo li;" is a local variable, so we should set the flags
to 0 explicitly, else, packets maybe truncated unexpectedly when copied
to the userspace.
Fixes: 7643507fe8b5 ("netfilter: xt_NFLOG: nflog-range does not truncate
packets")
Cc: Vishwa
301 - 364 of 364 matches
Mail list logo
