CPU load on queued_spin_lock_slowpath

2018-02-05 Thread Tugrul Erdogan
Hi All,

My server had a locking problem with the logs located below. I can not
reproduce this erroneous situation again but I think that there is an
active vulnerability at my server because of this error.

My server's kernel version is v4.6.4.

What can be the cause of this error or do you have any opinion about
how can I reproduce this logs again? Thanks for your helps.

Best regards,
Tugrul

Feb  5 13:20:42 serv kernel: []
queued_spin_lock_slowpath+0xb/0xf
Feb  5 13:20:42 serv kernel: [] _raw_spin_lock_bh+0x2b/0x30
Feb  5 13:20:42 serv kernel: []
connlimit_mt+0x114/0x30 [xt_connlimit]
Feb  5 13:20:42 serv kernel: [] ?
hashlimit_mt+0x2b7/0x71 [xt_hashlimit]
Feb  5 13:20:42 serv kernel: [] ?
_raw_spin_unlock_bh+0x1e/0x20
Feb  5 13:20:42 serv kernel: []
ipt_do_table+0x25f/0x710 [ipt_tables]
Feb  5 13:20:42 serv kernel: [] ?
ipt_do_table+0x332/0x710 [ipt_tables]
Feb  5 13:20:42 serv kernel: [] ? tcp_packet+0x39d/0x9a0
Feb  5 13:20:42 serv kernel: [] ?
dev_hard_start_xmit+0x22f/0x3e0
Feb  5 13:20:42 serv kernel: []
iptable_mangle_hook+0x37/0x110 [iptable_mangle]
Feb  5 13:20:42 serv kernel: [] nf_iterate+0x5d/0x70
Feb  5 13:20:42 serv kernel: [] nf_hook_slow+0x5d/0x70
Feb  5 13:20:42 serv kernel: [] ip_output+0xdb/0xf0
Feb  5 13:20:42 serv kernel: [] ? __ip_local_out+0xa2/0x110
Feb  5 13:20:42 serv kernel: [] ?
ip_fragment.constprop.51+0x80/0x80
Feb  5 13:20:42 serv kernel: [] ip_local_out+0x35/0x40
Feb  5 13:20:42 serv kernel: []
synproxy_send_tcp.isra.8+0xca/0xf0 [ipt_SYNPROXY]
Feb  5 13:20:42 serv kernel: []
synproxy_recv_client_ack+0x200/0x340 [ipt_SYNPROXY]
Feb  5 13:20:42 serv kernel: []
synproxy_tg4+0x11c/0x308 [ipt_SYNPROXY]
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[nf:flow-offload-hw-v2 6/6] net/netfilter/nf_flow_table_inet.o:undefined reference to `nf_flow_table_init'

2018-02-05 Thread kbuild test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git 
flow-offload-hw-v2
head:   d2a66b6aae8b1294d8cb550520485eeb03fa1619
commit: d2a66b6aae8b1294d8cb550520485eeb03fa1619 [6/6] netfilter: 
nf_flow_table: add hardware offload support
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout d2a66b6aae8b1294d8cb550520485eeb03fa1619
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   `.exit.data' referenced in section `.exit.text' of drivers/tty/n_hdlc.o: 
defined in discarded section `.exit.data' of drivers/tty/n_hdlc.o
   `.exit.data' referenced in section `.exit.text' of drivers/tty/n_hdlc.o: 
defined in discarded section `.exit.data' of drivers/tty/n_hdlc.o
   `.exit.data' referenced in section `.exit.text' of drivers/tty/n_hdlc.o: 
defined in discarded section `.exit.data' of drivers/tty/n_hdlc.o
   `.exit.data' referenced in section `.exit.text' of drivers/tty/n_hdlc.o: 
defined in discarded section `.exit.data' of drivers/tty/n_hdlc.o
>> net/netfilter/nf_flow_table_inet.o:(.data+0x20): undefined reference to 
>> `nf_flow_table_init'
>> net/ipv4/netfilter/nf_flow_table_ipv4.o:(.data+0x20): undefined reference to 
>> `nf_flow_table_init'
>> net/ipv6/netfilter/nf_flow_table_ipv6.o:(.data+0x20): undefined reference to 
>> `nf_flow_table_init'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip


[nf:flow-offload-hw-v2 6/6] net/netfilter/nf_flow_table_inet.o:undefined reference to `nf_flow_table_init'

2018-02-05 Thread kbuild test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git 
flow-offload-hw-v2
head:   d2a66b6aae8b1294d8cb550520485eeb03fa1619
commit: d2a66b6aae8b1294d8cb550520485eeb03fa1619 [6/6] netfilter: 
nf_flow_table: add hardware offload support
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout d2a66b6aae8b1294d8cb550520485eeb03fa1619
# save the attached .config to linux build tree
make.cross ARCH=alpha 

All errors (new ones prefixed by >>):

>> net/netfilter/nf_flow_table_inet.o:(.data.rel+0x20): undefined reference to 
>> `nf_flow_table_init'
>> net/ipv4/netfilter/nf_flow_table_ipv4.o:(.data.rel+0x20): undefined 
>> reference to `nf_flow_table_init'
>> net/ipv6/netfilter/nf_flow_table_ipv6.o:(.data.rel+0x20): undefined 
>> reference to `nf_flow_table_init'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip


[PATCH nf 3/3] netfilter: nf_tables: fix flowtable free

2018-02-05 Thread Pablo Neira Ayuso
Every flow_offload entry is added into the table twice. Because of this,
rhashtable_free_and_destroy can't be used, since it would call kfree for
each flow_offload object twice.

This patch adds a call to nf_flow_table_iterate_cleanup() to schedule
removal of entries, then there is an explicitly invocation of the
garbage collector to clean up resources.

Based on patch from Felix Fietkau.

Signed-off-by: Pablo Neira Ayuso 
---
@Felix: This is an alternate path to fix what you're reporting, it's based
on your patch 1/2, but sets on the DYING flag, then there's an explicit
call to the garbage collector. The reason for this is that rule of thumb
is that entries should be always removed by the garbage collector,
either by calling explicitly or implicitly. I think this simplifies the
upcoming hardware offload support and it will allow us to introduce
batching support from the garbage collector. Given adding/removing
entries to hardware can be slow, we can amortize the cost by batching
several add/delete commands.

 include/net/netfilter/nf_flow_table.h   |  2 ++
 net/ipv4/netfilter/nf_flow_table_ipv4.c |  1 +
 net/ipv6/netfilter/nf_flow_table_ipv6.c |  1 +
 net/netfilter/nf_flow_table.c   | 25 +++--
 net/netfilter/nf_flow_table_inet.c  |  1 +
 net/netfilter/nf_tables_api.c   |  9 ++---
 6 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h 
b/include/net/netfilter/nf_flow_table.h
index ed49cd169ecf..020ae903066f 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -14,6 +14,7 @@ struct nf_flowtable_type {
struct list_headlist;
int family;
void(*gc)(struct work_struct *work);
+   void(*free)(struct nf_flowtable *ft);
const struct rhashtable_params  *params;
nf_hookfn   *hook;
struct module   *owner;
@@ -98,6 +99,7 @@ int nf_flow_table_iterate(struct nf_flowtable *flow_table,
 
 void nf_flow_table_cleanup(struct net *net, struct net_device *dev);
 
+void nf_flow_table_free(struct nf_flowtable *flow_table);
 void nf_flow_offload_work_gc(struct work_struct *work);
 extern const struct rhashtable_params nf_flow_offload_rhash_params;
 
diff --git a/net/ipv4/netfilter/nf_flow_table_ipv4.c 
b/net/ipv4/netfilter/nf_flow_table_ipv4.c
index b2d01eb25f2c..25d2975da156 100644
--- a/net/ipv4/netfilter/nf_flow_table_ipv4.c
+++ b/net/ipv4/netfilter/nf_flow_table_ipv4.c
@@ -260,6 +260,7 @@ static struct nf_flowtable_type flowtable_ipv4 = {
.family = NFPROTO_IPV4,
.params = _flow_offload_rhash_params,
.gc = nf_flow_offload_work_gc,
+   .free   = nf_flow_table_free,
.hook   = nf_flow_offload_ip_hook,
.owner  = THIS_MODULE,
 };
diff --git a/net/ipv6/netfilter/nf_flow_table_ipv6.c 
b/net/ipv6/netfilter/nf_flow_table_ipv6.c
index fff21602875a..d346705d6ee6 100644
--- a/net/ipv6/netfilter/nf_flow_table_ipv6.c
+++ b/net/ipv6/netfilter/nf_flow_table_ipv6.c
@@ -253,6 +253,7 @@ static struct nf_flowtable_type flowtable_ipv6 = {
.family = NFPROTO_IPV6,
.params = _flow_offload_rhash_params,
.gc = nf_flow_offload_work_gc,
+   .free   = nf_flow_table_free,
.hook   = nf_flow_offload_ipv6_hook,
.owner  = THIS_MODULE,
 };
diff --git a/net/netfilter/nf_flow_table.c b/net/netfilter/nf_flow_table.c
index 04c08f6b9015..c17f1af42daa 100644
--- a/net/netfilter/nf_flow_table.c
+++ b/net/netfilter/nf_flow_table.c
@@ -232,19 +232,16 @@ static inline bool nf_flow_is_dying(const struct 
flow_offload *flow)
return flow->flags & FLOW_OFFLOAD_DYING;
 }
 
-void nf_flow_offload_work_gc(struct work_struct *work)
+static int nf_flow_offload_gc_step(struct nf_flowtable *flow_table)
 {
struct flow_offload_tuple_rhash *tuplehash;
-   struct nf_flowtable *flow_table;
struct rhashtable_iter hti;
struct flow_offload *flow;
int err;
 
-   flow_table = container_of(work, struct nf_flowtable, gc_work.work);
-
err = rhashtable_walk_init(_table->rhashtable, , GFP_KERNEL);
if (err)
-   goto schedule;
+   return 0;
 
rhashtable_walk_start();
 
@@ -270,7 +267,16 @@ void nf_flow_offload_work_gc(struct work_struct *work)
 out:
rhashtable_walk_stop();
rhashtable_walk_exit();
-schedule:
+
+   return 1;
+}
+
+void nf_flow_offload_work_gc(struct work_struct *work)
+{
+   struct nf_flowtable *flow_table;
+
+   flow_table = container_of(work, struct nf_flowtable, gc_work.work);
+   nf_flow_offload_gc_step(flow_table);

[PATCH nf 2/3] netfilter: nft_flow_offload: move flowtable cleanup routines to nf_flow_table

2018-02-05 Thread Pablo Neira Ayuso
Move the flowtable cleanup routines to nf_flow_table and expose the
nf_flow_table_cleanup() helper function.

Signed-off-by: Pablo Neira Ayuso 
---
This is another dependency for the fix in patch 3/3.

 include/net/netfilter/nf_flow_table.h |  3 +++
 net/netfilter/nf_flow_table.c | 24 
 net/netfilter/nft_flow_offload.c  | 19 +--
 3 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h 
b/include/net/netfilter/nf_flow_table.h
index b22b22082733..ed49cd169ecf 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -95,6 +95,9 @@ struct flow_offload_tuple_rhash *flow_offload_lookup(struct 
nf_flowtable *flow_t
 int nf_flow_table_iterate(struct nf_flowtable *flow_table,
  void (*iter)(struct flow_offload *flow, void *data),
  void *data);
+
+void nf_flow_table_cleanup(struct net *net, struct net_device *dev);
+
 void nf_flow_offload_work_gc(struct work_struct *work);
 extern const struct rhashtable_params nf_flow_offload_rhash_params;
 
diff --git a/net/netfilter/nf_flow_table.c b/net/netfilter/nf_flow_table.c
index 2f5099cb85b8..04c08f6b9015 100644
--- a/net/netfilter/nf_flow_table.c
+++ b/net/netfilter/nf_flow_table.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -425,5 +426,28 @@ int nf_flow_dnat_port(const struct flow_offload *flow,
 }
 EXPORT_SYMBOL_GPL(nf_flow_dnat_port);
 
+static void nf_flow_table_do_cleanup(struct flow_offload *flow, void *data)
+{
+   struct net_device *dev = data;
+
+   if (dev && flow->tuplehash[0].tuple.iifidx != dev->ifindex)
+   return;
+
+   flow_offload_dead(flow);
+}
+
+static void nf_flow_table_iterate_cleanup(struct nf_flowtable *flowtable,
+ void *data)
+{
+   nf_flow_table_iterate(flowtable, nf_flow_table_do_cleanup, data);
+   flush_delayed_work(>gc_work);
+}
+
+void nf_flow_table_cleanup(struct net *net, struct net_device *dev)
+{
+   nft_flow_table_iterate(net, nf_flow_table_iterate_cleanup, dev);
+}
+EXPORT_SYMBOL_GPL(nf_flow_table_cleanup);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Pablo Neira Ayuso ");
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index e5c45c7ac02a..b65829b2be22 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -194,23 +194,6 @@ static struct nft_expr_type nft_flow_offload_type 
__read_mostly = {
.owner  = THIS_MODULE,
 };
 
-static void flow_offload_iterate_cleanup(struct flow_offload *flow, void *data)
-{
-   struct net_device *dev = data;
-
-   if (dev && flow->tuplehash[0].tuple.iifidx != dev->ifindex)
-   return;
-
-   flow_offload_dead(flow);
-}
-
-static void nft_flow_offload_iterate_cleanup(struct nf_flowtable *flowtable,
-void *data)
-{
-   nf_flow_table_iterate(flowtable, flow_offload_iterate_cleanup, data);
-   flush_delayed_work(>gc_work);
-}
-
 static int flow_offload_netdev_event(struct notifier_block *this,
 unsigned long event, void *ptr)
 {
@@ -219,7 +202,7 @@ static int flow_offload_netdev_event(struct notifier_block 
*this,
if (event != NETDEV_DOWN)
return NOTIFY_DONE;
 
-   nft_flow_table_iterate(dev_net(dev), nft_flow_offload_iterate_cleanup, 
dev);
+   nf_flow_table_cleanup(dev_net(dev), dev);
 
return NOTIFY_DONE;
 }
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH nf 1/3] netfilter: nft_flow_offload: no need to flush entries on module removal

2018-02-05 Thread Pablo Neira Ayuso
nft_flow_offload module removal does not require to flush existing
flowtables, it is valid to remove this module while keeping flowtables
around.

Signed-off-by: Pablo Neira Ayuso 
---
This patch is a dependency for bugfix in patch 3/3. PATCH 2/3 moves
flowtable cleanup to the core, this patch saves us from exporting a
function that we would need to unexport just thereafter. So remove it
in first place to avoid unnecessary code churning.

 net/netfilter/nft_flow_offload.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 1739ff8ca21f..e5c45c7ac02a 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -247,14 +247,8 @@ static int __init nft_flow_offload_module_init(void)
 
 static void __exit nft_flow_offload_module_exit(void)
 {
-   struct net *net;
-
nft_unregister_expr(_flow_offload_type);
unregister_netdevice_notifier(_offload_netdev_notifier);
-   rtnl_lock();
-   for_each_net(net)
-   nft_flow_table_iterate(net, nft_flow_offload_iterate_cleanup, 
NULL);
-   rtnl_unlock();
 }
 
 module_init(nft_flow_offload_module_init);
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: IPv6 Parameter problem with no ICMPv6 response ?

2018-02-05 Thread David McCullough

Pablo Neira Ayuso wrote the following:
> On Mon, Feb 05, 2018 at 01:16:08PM +0100, Pablo Neira Ayuso wrote:
> > On Mon, Feb 05, 2018 at 01:58:26PM +1000, David McCullough wrote:
> > > 
> > > Hi devel,
> > > 
> > > I am looking for some feedback on IPv6 behaviour with/without netfilter in
> > > the path.  We are in process of some IPv6 certification at a lab.
> > > 
> > > RFC2460 has a bunch of conditions under which certain ICMPv6 responses
> > > should be sent.  This is even commented in the code.
> > > 
> > > linux/net/ipv6/reassembly.c:255
> > > /* Check if the fragment is rounded to 8 bytes.
> > >  * Required by the RFC.
> > >  */
> > > if (end & 0x7) {
> > > /* RFC2460 says always send parameter problem in
> > >  * this case. -DaveM
> > >  */
> > > __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > > IPSTATS_MIB_INHDRERRORS);
> > > icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
> > >   offsetof(struct ipv6hdr, 
> > > payload_len));   
> > > return -1;
> > > }
> > > 
> > > linux/net/ipv6/netfilter/nf_conntrack_reasm.c:259
> > > /* Check if the fragment is rounded to 8 bytes.
> > >  * Required by the RFC.
> > >  */
> > > if (end & 0x7) {
> > > /* RFC2460 says always send parameter problem in
> > >  * this case. -DaveM
> > >  */
> > > pr_debug("end of fragment not rounded to 8 
> > > bytes.\n");
> > > return -1;  
> > > }
> > > 
> > > The behaviour of the non-netfilter code is what the certification is 
> > > expecting.
> > > We are using conntracking though and I can see no way to avoid the above
> > > netfilter code from silently dropping the packet and not responding 
> > > correctly.
> > > 
> > > We experiemented with the patch below and it provided the appropriate
> > > responses but we were not sure this is the best approach.  Happy to send 
> > > in
> > > a proper patch if this looks ok.
> > 
> > Probably you're refering to this fix?
> > 
> > commit 83f1999caeb14e15df205e80d210699951733287
> > Author: Subash Abhinov Kasiviswanathan 
> > Date:   Fri Jan 12 17:36:27 2018 -0700
> > 
> > netfilter: ipv6: nf_defrag: Pass on packets to stack per RFC2460
> 
> You will also need this follow up amendment on top of it:
> 
> commit ea23d5e3bf340e413b8e05c13da233c99c64142b
> Author: Subash Abhinov Kasiviswanathan 
> Date:   Wed Jan 31 04:50:01 2018 -0700
> 
> netfilter: ipv6: nf_defrag: Kill frag queue on RFC2460 failure

Awesome,  thanks, wasn't aware of the patch,   will check it out,

Cheers,
Davidm

-- 
David McCullough,  david.mccullo...@accelerated.com,   Ph: 0410 560 763
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Patch net v2] xt_RATEEST: acquire xt_rateest_mutex for hash insert

2018-02-05 Thread Eric Dumazet
On Mon, 2018-02-05 at 14:41 -0800, Cong Wang wrote:
> rateest_hash is supposed to be protected by xt_rateest_mutex,
> and, as suggested by Eric, lookup and insert should be atomic,
> so we should acquire the xt_rateest_mutex once for both.
> 
> So introduce a non-locking helper for internal use and keep the
> locking one for external.
> 
> Reported-by: 
> Fixes: 5859034d7eb8 ("[NETFILTER]: x_tables: add RATEEST target")
> Cc: Pablo Neira Ayuso 
> Cc: Eric Dumazet 
> Signed-off-by: Cong Wang 
> ---

Reviewed-by: Eric Dumazet 

Thanks !

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Patch net v2] xt_RATEEST: acquire xt_rateest_mutex for hash insert

2018-02-05 Thread Florian Westphal
Cong Wang  wrote:
> rateest_hash is supposed to be protected by xt_rateest_mutex,
> and, as suggested by Eric, lookup and insert should be atomic,
> so we should acquire the xt_rateest_mutex once for both.
> 
> So introduce a non-locking helper for internal use and keep the
> locking one for external.

Looks good, thanks Cong.

Reviewed-by: Florian Westphal 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Patch net v2] xt_RATEEST: acquire xt_rateest_mutex for hash insert

2018-02-05 Thread Cong Wang
rateest_hash is supposed to be protected by xt_rateest_mutex,
and, as suggested by Eric, lookup and insert should be atomic,
so we should acquire the xt_rateest_mutex once for both.

So introduce a non-locking helper for internal use and keep the
locking one for external.

Reported-by: 
Fixes: 5859034d7eb8 ("[NETFILTER]: x_tables: add RATEEST target")
Cc: Pablo Neira Ayuso 
Cc: Eric Dumazet 
Signed-off-by: Cong Wang 
---
 net/netfilter/xt_RATEEST.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c
index 498b54fd04d7..141c295191f6 100644
--- a/net/netfilter/xt_RATEEST.c
+++ b/net/netfilter/xt_RATEEST.c
@@ -39,23 +39,31 @@ static void xt_rateest_hash_insert(struct xt_rateest *est)
hlist_add_head(>list, _hash[h]);
 }
 
-struct xt_rateest *xt_rateest_lookup(const char *name)
+static struct xt_rateest *__xt_rateest_lookup(const char *name)
 {
struct xt_rateest *est;
unsigned int h;
 
h = xt_rateest_hash(name);
-   mutex_lock(_rateest_mutex);
hlist_for_each_entry(est, _hash[h], list) {
if (strcmp(est->name, name) == 0) {
est->refcnt++;
-   mutex_unlock(_rateest_mutex);
return est;
}
}
-   mutex_unlock(_rateest_mutex);
+
return NULL;
 }
+
+struct xt_rateest *xt_rateest_lookup(const char *name)
+{
+   struct xt_rateest *est;
+
+   mutex_lock(_rateest_mutex);
+   est = __xt_rateest_lookup(name);
+   mutex_unlock(_rateest_mutex);
+   return est;
+}
 EXPORT_SYMBOL_GPL(xt_rateest_lookup);
 
 void xt_rateest_put(struct xt_rateest *est)
@@ -100,8 +108,10 @@ static int xt_rateest_tg_checkentry(const struct 
xt_tgchk_param *par)
 
net_get_random_once(_rnd, sizeof(jhash_rnd));
 
-   est = xt_rateest_lookup(info->name);
+   mutex_lock(_rateest_mutex);
+   est = __xt_rateest_lookup(info->name);
if (est) {
+   mutex_unlock(_rateest_mutex);
/*
 * If estimator parameters are specified, they must match the
 * existing estimator.
@@ -139,11 +149,13 @@ static int xt_rateest_tg_checkentry(const struct 
xt_tgchk_param *par)
 
info->est = est;
xt_rateest_hash_insert(est);
+   mutex_unlock(_rateest_mutex);
return 0;
 
 err2:
kfree(est);
 err1:
+   mutex_unlock(_rateest_mutex);
return ret;
 }
 
-- 
2.13.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] netfilter: nf_tables: fix flowtable resource leak

2018-02-05 Thread Felix Fietkau
When cleaning up flowtable entries, associated dst and ct entries need
to be released as well

Signed-off-by: Felix Fietkau 
---
 net/netfilter/nf_flow_table.c | 19 +--
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nf_flow_table.c b/net/netfilter/nf_flow_table.c
index 20f86091ab98..9925bd3f93c4 100644
--- a/net/netfilter/nf_flow_table.c
+++ b/net/netfilter/nf_flow_table.c
@@ -124,7 +124,7 @@ void flow_offload_free(struct flow_offload *flow)
dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst_cache);
dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_cache);
e = container_of(flow, struct flow_offload_entry, flow);
-   kfree(e);
+   kfree_rcu(e, rcu_head);
 }
 EXPORT_SYMBOL_GPL(flow_offload_free);
 
@@ -161,7 +161,9 @@ void flow_offload_del(struct nf_flowtable *flow_table,
   *flow_table->type->params);
 
e = container_of(flow, struct flow_offload_entry, flow);
-   kfree_rcu(e, rcu_head);
+   nf_ct_delete(e->ct, 0, 0);
+   nf_ct_put(e->ct);
+   flow_offload_free(flow);
 }
 EXPORT_SYMBOL_GPL(flow_offload_del);
 
@@ -174,15 +176,6 @@ flow_offload_lookup(struct nf_flowtable *flow_table,
 }
 EXPORT_SYMBOL_GPL(flow_offload_lookup);
 
-static void nf_flow_release_ct(const struct flow_offload *flow)
-{
-   struct flow_offload_entry *e;
-
-   e = container_of(flow, struct flow_offload_entry, flow);
-   nf_ct_delete(e->ct, 0, 0);
-   nf_ct_put(e->ct);
-}
-
 int nf_flow_table_iterate(struct nf_flowtable *flow_table,
  void (*iter)(struct flow_offload *flow, void *data),
  void *data)
@@ -276,10 +269,8 @@ void nf_flow_offload_work_gc(struct work_struct *work)
flow = container_of(tuplehash, struct flow_offload, 
tuplehash[0]);
 
if (nf_flow_has_expired(flow) ||
-   nf_flow_is_dying(flow)) {
+   nf_flow_is_dying(flow))
flow_offload_del(flow_table, flow);
-   nf_flow_release_ct(flow);
-   }
}
 out:
rhashtable_walk_stop();
-- 
2.14.2

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] netfilter: nf_tables: fix flowtable free

2018-02-05 Thread Felix Fietkau
Every flow_offload entry is added into the table twice. Because of this,
rhashtable_free_and_destroy can't be used, since it would call kfree for
each flow_offload object twice.

Signed-off-by: Felix Fietkau 
---
 include/net/netfilter/nf_flow_table.h   |  2 ++
 net/ipv4/netfilter/nf_flow_table_ipv4.c |  1 +
 net/ipv6/netfilter/nf_flow_table_ipv6.c |  1 +
 net/netfilter/nf_flow_table.c   | 15 +++
 net/netfilter/nf_flow_table_inet.c  |  1 +
 net/netfilter/nf_tables_api.c   |  8 +---
 6 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h 
b/include/net/netfilter/nf_flow_table.h
index b22b22082733..67c61bda6a46 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -14,6 +14,7 @@ struct nf_flowtable_type {
struct list_headlist;
int family;
void(*gc)(struct work_struct *work);
+   void(*free)(struct nf_flowtable *table);
const struct rhashtable_params  *params;
nf_hookfn   *hook;
struct module   *owner;
@@ -95,6 +96,7 @@ struct flow_offload_tuple_rhash *flow_offload_lookup(struct 
nf_flowtable *flow_t
 int nf_flow_table_iterate(struct nf_flowtable *flow_table,
  void (*iter)(struct flow_offload *flow, void *data),
  void *data);
+void nf_flow_table_free(struct nf_flowtable *flow_table);
 void nf_flow_offload_work_gc(struct work_struct *work);
 extern const struct rhashtable_params nf_flow_offload_rhash_params;
 
diff --git a/net/ipv4/netfilter/nf_flow_table_ipv4.c 
b/net/ipv4/netfilter/nf_flow_table_ipv4.c
index b2d01eb25f2c..25d2975da156 100644
--- a/net/ipv4/netfilter/nf_flow_table_ipv4.c
+++ b/net/ipv4/netfilter/nf_flow_table_ipv4.c
@@ -260,6 +260,7 @@ static struct nf_flowtable_type flowtable_ipv4 = {
.family = NFPROTO_IPV4,
.params = _flow_offload_rhash_params,
.gc = nf_flow_offload_work_gc,
+   .free   = nf_flow_table_free,
.hook   = nf_flow_offload_ip_hook,
.owner  = THIS_MODULE,
 };
diff --git a/net/ipv6/netfilter/nf_flow_table_ipv6.c 
b/net/ipv6/netfilter/nf_flow_table_ipv6.c
index fff21602875a..d346705d6ee6 100644
--- a/net/ipv6/netfilter/nf_flow_table_ipv6.c
+++ b/net/ipv6/netfilter/nf_flow_table_ipv6.c
@@ -253,6 +253,7 @@ static struct nf_flowtable_type flowtable_ipv6 = {
.family = NFPROTO_IPV6,
.params = _flow_offload_rhash_params,
.gc = nf_flow_offload_work_gc,
+   .free   = nf_flow_table_free,
.hook   = nf_flow_offload_ipv6_hook,
.owner  = THIS_MODULE,
 };
diff --git a/net/netfilter/nf_flow_table.c b/net/netfilter/nf_flow_table.c
index 2f5099cb85b8..20f86091ab98 100644
--- a/net/netfilter/nf_flow_table.c
+++ b/net/netfilter/nf_flow_table.c
@@ -221,6 +221,21 @@ int nf_flow_table_iterate(struct nf_flowtable *flow_table,
 }
 EXPORT_SYMBOL_GPL(nf_flow_table_iterate);
 
+static void
+__nf_flow_offload_free(struct flow_offload *flow, void *data)
+{
+   struct nf_flowtable *flow_table = data;
+
+   flow_offload_del(flow_table, flow);
+}
+
+void nf_flow_table_free(struct nf_flowtable *flow_table)
+{
+   nf_flow_table_iterate(flow_table, __nf_flow_offload_free, flow_table);
+   rhashtable_destroy(_table->rhashtable);
+}
+EXPORT_SYMBOL_GPL(nf_flow_table_free);
+
 static inline bool nf_flow_has_expired(const struct flow_offload *flow)
 {
return (__s32)(flow->timeout - (u32)jiffies) <= 0;
diff --git a/net/netfilter/nf_flow_table_inet.c 
b/net/netfilter/nf_flow_table_inet.c
index 281209aeba8f..375a1881d93d 100644
--- a/net/netfilter/nf_flow_table_inet.c
+++ b/net/netfilter/nf_flow_table_inet.c
@@ -24,6 +24,7 @@ static struct nf_flowtable_type flowtable_inet = {
.family = NFPROTO_INET,
.params = _flow_offload_rhash_params,
.gc = nf_flow_offload_work_gc,
+   .free   = nf_flow_table_free,
.hook   = nf_flow_offload_inet_hook,
.owner  = THIS_MODULE,
 };
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0791813a1e7d..a6c4747c7d5d 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5399,17 +5399,11 @@ static void nf_tables_flowtable_notify(struct nft_ctx 
*ctx,
nfnetlink_set_err(ctx->net, ctx->portid, NFNLGRP_NFTABLES, -ENOBUFS);
 }
 
-static void nft_flowtable_destroy(void *ptr, void *arg)
-{
-   kfree(ptr);
-}
-
 static void nf_tables_flowtable_destroy(struct nft_flowtable *flowtable)
 {
cancel_delayed_work_sync(>data.gc_work);
kfree(flowtable->name);
-   rhashtable_free_and_destroy(>data.rhashtable,
-   

Re: KASAN: slab-out-of-bounds Read in clusterip_tg_check

2018-02-05 Thread Florian Westphal
syzbot  wrote:

#syz-fix: netfilter: ipt_CLUSTERIP: fix out-of-bounds accesses in 
clusterip_tg_check()
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: IPv6 Parameter problem with no ICMPv6 response ?

2018-02-05 Thread Pablo Neira Ayuso
On Mon, Feb 05, 2018 at 01:16:08PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Feb 05, 2018 at 01:58:26PM +1000, David McCullough wrote:
> > 
> > Hi devel,
> > 
> > I am looking for some feedback on IPv6 behaviour with/without netfilter in
> > the path.  We are in process of some IPv6 certification at a lab.
> > 
> > RFC2460 has a bunch of conditions under which certain ICMPv6 responses
> > should be sent.  This is even commented in the code.
> > 
> > linux/net/ipv6/reassembly.c:255
> > /* Check if the fragment is rounded to 8 bytes.
> >  * Required by the RFC.
> >  */
> > if (end & 0x7) {
> > /* RFC2460 says always send parameter problem in
> >  * this case. -DaveM
> >  */
> > __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > IPSTATS_MIB_INHDRERRORS);
> > icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
> >   offsetof(struct ipv6hdr, 
> > payload_len));   
> > return -1;
> > }
> > 
> > linux/net/ipv6/netfilter/nf_conntrack_reasm.c:259
> > /* Check if the fragment is rounded to 8 bytes.
> >  * Required by the RFC.
> >  */
> > if (end & 0x7) {
> > /* RFC2460 says always send parameter problem in
> >  * this case. -DaveM
> >  */
> > pr_debug("end of fragment not rounded to 8 
> > bytes.\n");
> > return -1;  
> > }
> > 
> > The behaviour of the non-netfilter code is what the certification is 
> > expecting.
> > We are using conntracking though and I can see no way to avoid the above
> > netfilter code from silently dropping the packet and not responding 
> > correctly.
> > 
> > We experiemented with the patch below and it provided the appropriate
> > responses but we were not sure this is the best approach.  Happy to send in
> > a proper patch if this looks ok.
> 
> Probably you're refering to this fix?
> 
> commit 83f1999caeb14e15df205e80d210699951733287
> Author: Subash Abhinov Kasiviswanathan 
> Date:   Fri Jan 12 17:36:27 2018 -0700
> 
> netfilter: ipv6: nf_defrag: Pass on packets to stack per RFC2460

You will also need this follow up amendment on top of it:

commit ea23d5e3bf340e413b8e05c13da233c99c64142b
Author: Subash Abhinov Kasiviswanathan 
Date:   Wed Jan 31 04:50:01 2018 -0700

netfilter: ipv6: nf_defrag: Kill frag queue on RFC2460 failure

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: IPv6 Parameter problem with no ICMPv6 response ?

2018-02-05 Thread Pablo Neira Ayuso
On Mon, Feb 05, 2018 at 01:58:26PM +1000, David McCullough wrote:
> 
> Hi devel,
> 
> I am looking for some feedback on IPv6 behaviour with/without netfilter in
> the path.  We are in process of some IPv6 certification at a lab.
> 
> RFC2460 has a bunch of conditions under which certain ICMPv6 responses
> should be sent.  This is even commented in the code.
> 
> linux/net/ipv6/reassembly.c:255
> /* Check if the fragment is rounded to 8 bytes.
>  * Required by the RFC.
>  */
> if (end & 0x7) {
> /* RFC2460 says always send parameter problem in
>  * this case. -DaveM
>  */
> __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> IPSTATS_MIB_INHDRERRORS);
> icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
>   offsetof(struct ipv6hdr, 
> payload_len));   
> return -1;
> }
> 
> linux/net/ipv6/netfilter/nf_conntrack_reasm.c:259
> /* Check if the fragment is rounded to 8 bytes.
>  * Required by the RFC.
>  */
> if (end & 0x7) {
> /* RFC2460 says always send parameter problem in
>  * this case. -DaveM
>  */
> pr_debug("end of fragment not rounded to 8 bytes.\n");
> return -1;  
> }
> 
> The behaviour of the non-netfilter code is what the certification is 
> expecting.
> We are using conntracking though and I can see no way to avoid the above
> netfilter code from silently dropping the packet and not responding correctly.
> 
> We experiemented with the patch below and it provided the appropriate
> responses but we were not sure this is the best approach.  Happy to send in
> a proper patch if this looks ok.

Probably you're refering to this fix?

commit 83f1999caeb14e15df205e80d210699951733287
Author: Subash Abhinov Kasiviswanathan 
Date:   Fri Jan 12 17:36:27 2018 -0700

netfilter: ipv6: nf_defrag: Pass on packets to stack per RFC2460

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html