netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] Netfilter/IPVS fixes for net
@ 2018-05-28 23:42 Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 1/9] netfilter: nf_tables: fix NULL pointer dereference on nft_ct_helper_obj_dump() Pablo Neira Ayuso
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains Netfilter/IPVS fixes for your net tree:

1) Null pointer dereference when dumping conntrack helper configuration,
   from Taehee Yoo.

2) Missing sanitization in ebtables extension name through compat,
   from Paolo Abeni.

3) Broken fetch of tracing value, from Taehee Yoo.

4) Incorrect arithmetics in packet ratelimiting.

5) Buffer overflow in IPVS sync daemon, from Julian Anastasov.

6) Wrong argument to nla_strlcpy() in nfnetlink_{acct,cthelper},
   from Eric Dumazet.

7) Fix splat in nft_update_chain_stats().

8) Null pointer dereference from object netlink dump path, from
   Taehee Yoo.

9) Missing static_branch_inc() when enabling counters in existing
   chain, from Taehee Yoo.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Thanks.

----------------------------------------------------------------

The following changes since commit 7063efd33bb15abc0160347f89eb5aba6b7d000e:

  tuntap: fix use after free during release (2018-05-16 14:53:10 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD

for you to fetch changes up to bbb8c61f97e3a2dd91b30d3e57b7964a67569d11:

  netfilter: nf_tables: increase nft_counters_enabled in nft_chain_stats_replace() (2018-05-29 00:15:12 +0200)

----------------------------------------------------------------
Eric Dumazet (1):
      netfilter: provide correct argument to nla_strlcpy()

Julian Anastasov (1):
      ipvs: fix buffer overflow with sync daemon and service

Pablo Neira Ayuso (2):
      netfilter: nft_limit: fix packet ratelimiting
      netfilter: nf_tables: disable preemption in nft_update_chain_stats()

Paolo Abeni (1):
      netfilter: ebtables: handle string from userspace with care

Taehee Yoo (4):
      netfilter: nf_tables: fix NULL pointer dereference on nft_ct_helper_obj_dump()
      netfilter: nft_meta: fix wrong value dereference in nft_meta_set_eval
      netfilter: nf_tables: fix NULL-ptr in nf_tables_dump_obj()
      netfilter: nf_tables: increase nft_counters_enabled in nft_chain_stats_replace()

 net/bridge/netfilter/ebtables.c    |  3 ++-
 net/netfilter/ipvs/ip_vs_ctl.c     | 21 +++++++++++++++------
 net/netfilter/nf_tables_api.c      |  8 +++++---
 net/netfilter/nf_tables_core.c     |  4 ++--
 net/netfilter/nfnetlink_acct.c     |  2 +-
 net/netfilter/nfnetlink_cthelper.c |  4 ++--
 net/netfilter/nft_ct.c             | 20 ++++++++++++--------
 net/netfilter/nft_limit.c          | 38 ++++++++++++++++++++++++--------------
 net/netfilter/nft_meta.c           | 14 ++++++++------
 9 files changed, 71 insertions(+), 43 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] netfilter: nf_tables: fix NULL pointer dereference on nft_ct_helper_obj_dump()
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 2/9] netfilter: ebtables: handle string from userspace with care Pablo Neira Ayuso
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Taehee Yoo <ap420073@gmail.com>

In the nft_ct_helper_obj_dump(), always priv->helper4 is dereferenced.
But if family is ipv6, priv->helper6 should be dereferenced.

Steps to reproduces:

   #test.nft
   table ip6 filter {
	   ct helper ftp {
		   type "ftp" protocol tcp
	   }
	   chain input {
		   type filter hook input priority 4;
		   ct helper set "ftp"
	   }
   }

   %nft -f test.nft
   %nft list ruleset

we can see the below messages:

[  916.286233] kasan: GPF could be caused by NULL-ptr deref or user memory access
[  916.294777] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[  916.302613] Modules linked in: nft_objref nf_conntrack_sip nf_conntrack_snmp nf_conntrack_broadcast nf_conntrack_ftp nft_ct nf_conntrack nf_tables nfnetlink [last unloaded: nfnetlink]
[  916.318758] CPU: 1 PID: 2093 Comm: nft Not tainted 4.17.0-rc4+ #181
[  916.326772] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015
[  916.338773] RIP: 0010:strlen+0x1a/0x90
[  916.342781] RSP: 0018:ffff88010ff0f2f8 EFLAGS: 00010292
[  916.346773] RAX: dffffc0000000000 RBX: ffff880119b26ee8 RCX: ffff88010c150038
[  916.354777] RDX: 0000000000000002 RSI: ffff880119b26ee8 RDI: 0000000000000010
[  916.362773] RBP: 0000000000000010 R08: 0000000000007e88 R09: ffff88010c15003c
[  916.370773] R10: ffff88010c150037 R11: ffffed002182a007 R12: ffff88010ff04040
[  916.378779] R13: 0000000000000010 R14: ffff880119b26f30 R15: ffff88010ff04110
[  916.387265] FS:  00007f57a1997700(0000) GS:ffff88011b800000(0000) knlGS:0000000000000000
[  916.394785] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  916.402778] CR2: 00007f57a0ac80f0 CR3: 000000010ff02000 CR4: 00000000001006e0
[  916.410772] Call Trace:
[  916.414787]  nft_ct_helper_obj_dump+0x94/0x200 [nft_ct]
[  916.418779]  ? nft_ct_set_eval+0x560/0x560 [nft_ct]
[  916.426771]  ? memset+0x1f/0x40
[  916.426771]  ? __nla_reserve+0x92/0xb0
[  916.434774]  ? memcpy+0x34/0x50
[  916.434774]  nf_tables_fill_obj_info+0x484/0x860 [nf_tables]
[  916.442773]  ? __nft_release_basechain+0x600/0x600 [nf_tables]
[  916.450779]  ? lock_acquire+0x193/0x380
[  916.454771]  ? lock_acquire+0x193/0x380
[  916.458789]  ? nf_tables_dump_obj+0x148/0xcb0 [nf_tables]
[  916.462777]  nf_tables_dump_obj+0x5f0/0xcb0 [nf_tables]
[  916.470769]  ? __alloc_skb+0x30b/0x500
[  916.474779]  netlink_dump+0x752/0xb50
[  916.478775]  __netlink_dump_start+0x4d3/0x750
[  916.482784]  nf_tables_getobj+0x27a/0x930 [nf_tables]
[  916.490774]  ? nft_obj_notify+0x100/0x100 [nf_tables]
[  916.494772]  ? nf_tables_getobj+0x930/0x930 [nf_tables]
[  916.502579]  ? nf_tables_dump_flowtable_done+0x70/0x70 [nf_tables]
[  916.506774]  ? nft_obj_notify+0x100/0x100 [nf_tables]
[  916.514808]  nfnetlink_rcv_msg+0x8ab/0xa86 [nfnetlink]
[  916.518771]  ? nfnetlink_rcv_msg+0x550/0xa86 [nfnetlink]
[  916.526782]  netlink_rcv_skb+0x23e/0x360
[  916.530773]  ? nfnetlink_bind+0x200/0x200 [nfnetlink]
[  916.534778]  ? debug_check_no_locks_freed+0x280/0x280
[  916.542770]  ? netlink_ack+0x870/0x870
[  916.546786]  ? ns_capable_common+0xf4/0x130
[  916.550765]  nfnetlink_rcv+0x172/0x16c0 [nfnetlink]
[  916.554771]  ? sched_clock_local+0xe2/0x150
[  916.558774]  ? sched_clock_cpu+0x144/0x180
[  916.566575]  ? lock_acquire+0x380/0x380
[  916.570775]  ? sched_clock_local+0xe2/0x150
[  916.574765]  ? nfnetlink_net_init+0x130/0x130 [nfnetlink]
[  916.578763]  ? sched_clock_cpu+0x144/0x180
[  916.582770]  ? lock_acquire+0x193/0x380
[  916.590771]  ? lock_acquire+0x193/0x380
[  916.594766]  ? lock_acquire+0x380/0x380
[  916.598760]  ? netlink_deliver_tap+0x262/0xa60
[  916.602766]  ? lock_acquire+0x193/0x380
[  916.606766]  netlink_unicast+0x3ef/0x5a0
[  916.610771]  ? netlink_attachskb+0x630/0x630
[  916.614763]  netlink_sendmsg+0x72a/0xb00
[  916.618769]  ? netlink_unicast+0x5a0/0x5a0
[  916.626766]  ? _copy_from_user+0x92/0xc0
[  916.630773]  __sys_sendto+0x202/0x300
[  916.634772]  ? __ia32_sys_getpeername+0xb0/0xb0
[  916.638759]  ? lock_acquire+0x380/0x380
[  916.642769]  ? lock_acquire+0x193/0x380
[  916.646761]  ? finish_task_switch+0xf4/0x560
[  916.650763]  ? __schedule+0x582/0x19a0
[  916.655301]  ? __sched_text_start+0x8/0x8
[  916.655301]  ? up_read+0x1c/0x110
[  916.655301]  ? __do_page_fault+0x48b/0xaa0
[  916.655301]  ? entry_SYSCALL_64_after_hwframe+0x59/0xbe
[  916.655301]  __x64_sys_sendto+0xdd/0x1b0
[  916.655301]  do_syscall_64+0x96/0x3d0
[  916.655301]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  916.655301] RIP: 0033:0x7f57a0ff5e03
[  916.655301] RSP: 002b:00007fff6367e0a8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[  916.655301] RAX: ffffffffffffffda RBX: 00007fff6367f1e0 RCX: 00007f57a0ff5e03
[  916.655301] RDX: 0000000000000020 RSI: 00007fff6367e110 RDI: 0000000000000003
[  916.655301] RBP: 00007fff6367e100 R08: 00007f57a0ce9160 R09: 000000000000000c
[  916.655301] R10: 0000000000000000 R11: 0000000000000246 R12: 00007fff6367e110
[  916.655301] R13: 0000000000000020 R14: 00007f57a153c610 R15: 0000562417258de0
[  916.655301] Code: ff ff ff 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 fa 53 48 c1 ea 03 48 b8 00 00 00 00 00 fc ff df 48 89 fd 48 83 ec 08 <0f> b6 04 02 48 89 fa 83 e2 07 38 d0 7f
[  916.655301] RIP: strlen+0x1a/0x90 RSP: ffff88010ff0f2f8
[  916.771929] ---[ end trace 1065e048e72479fe ]---
[  916.777204] Kernel panic - not syncing: Fatal exception
[  916.778158] Kernel Offset: 0x14000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_ct.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index ea737fd789e8..5c0de704bad5 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -880,22 +880,26 @@ static int nft_ct_helper_obj_dump(struct sk_buff *skb,
 				  struct nft_object *obj, bool reset)
 {
 	const struct nft_ct_helper_obj *priv = nft_obj_data(obj);
-	const struct nf_conntrack_helper *helper = priv->helper4;
+	const struct nf_conntrack_helper *helper;
 	u16 family;
 
+	if (priv->helper4 && priv->helper6) {
+		family = NFPROTO_INET;
+		helper = priv->helper4;
+	} else if (priv->helper6) {
+		family = NFPROTO_IPV6;
+		helper = priv->helper6;
+	} else {
+		family = NFPROTO_IPV4;
+		helper = priv->helper4;
+	}
+
 	if (nla_put_string(skb, NFTA_CT_HELPER_NAME, helper->name))
 		return -1;
 
 	if (nla_put_u8(skb, NFTA_CT_HELPER_L4PROTO, priv->l4proto))
 		return -1;
 
-	if (priv->helper4 && priv->helper6)
-		family = NFPROTO_INET;
-	else if (priv->helper6)
-		family = NFPROTO_IPV6;
-	else
-		family = NFPROTO_IPV4;
-
 	if (nla_put_be16(skb, NFTA_CT_HELPER_L3PROTO, htons(family)))
 		return -1;
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/9] netfilter: ebtables: handle string from userspace with care
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 1/9] netfilter: nf_tables: fix NULL pointer dereference on nft_ct_helper_obj_dump() Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 3/9] netfilter: nft_meta: fix wrong value dereference in nft_meta_set_eval Pablo Neira Ayuso
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Paolo Abeni <pabeni@redhat.com>

strlcpy() can't be safely used on a user-space provided string,
as it can try to read beyond the buffer's end, if the latter is
not NULL terminated.

Leveraging the above, syzbot has been able to trigger the following
splat:

BUG: KASAN: stack-out-of-bounds in strlcpy include/linux/string.h:300
[inline]
BUG: KASAN: stack-out-of-bounds in compat_mtw_from_user
net/bridge/netfilter/ebtables.c:1957 [inline]
BUG: KASAN: stack-out-of-bounds in ebt_size_mwt
net/bridge/netfilter/ebtables.c:2059 [inline]
BUG: KASAN: stack-out-of-bounds in size_entry_mwt
net/bridge/netfilter/ebtables.c:2155 [inline]
BUG: KASAN: stack-out-of-bounds in compat_copy_entries+0x96c/0x14a0
net/bridge/netfilter/ebtables.c:2194
Write of size 33 at addr ffff8801b0abf888 by task syz-executor0/4504

CPU: 0 PID: 4504 Comm: syz-executor0 Not tainted 4.17.0-rc2+ #40
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  check_memory_region_inline mm/kasan/kasan.c:260 [inline]
  check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
  memcpy+0x37/0x50 mm/kasan/kasan.c:303
  strlcpy include/linux/string.h:300 [inline]
  compat_mtw_from_user net/bridge/netfilter/ebtables.c:1957 [inline]
  ebt_size_mwt net/bridge/netfilter/ebtables.c:2059 [inline]
  size_entry_mwt net/bridge/netfilter/ebtables.c:2155 [inline]
  compat_copy_entries+0x96c/0x14a0 net/bridge/netfilter/ebtables.c:2194
  compat_do_replace+0x483/0x900 net/bridge/netfilter/ebtables.c:2285
  compat_do_ebt_set_ctl+0x2ac/0x324 net/bridge/netfilter/ebtables.c:2367
  compat_nf_sockopt net/netfilter/nf_sockopt.c:144 [inline]
  compat_nf_setsockopt+0x9b/0x140 net/netfilter/nf_sockopt.c:156
  compat_ip_setsockopt+0xff/0x140 net/ipv4/ip_sockglue.c:1279
  inet_csk_compat_setsockopt+0x97/0x120 net/ipv4/inet_connection_sock.c:1041
  compat_tcp_setsockopt+0x49/0x80 net/ipv4/tcp.c:2901
  compat_sock_common_setsockopt+0xb4/0x150 net/core/sock.c:3050
  __compat_sys_setsockopt+0x1ab/0x7c0 net/compat.c:403
  __do_compat_sys_setsockopt net/compat.c:416 [inline]
  __se_compat_sys_setsockopt net/compat.c:413 [inline]
  __ia32_compat_sys_setsockopt+0xbd/0x150 net/compat.c:413
  do_syscall_32_irqs_on arch/x86/entry/common.c:323 [inline]
  do_fast_syscall_32+0x345/0xf9b arch/x86/entry/common.c:394
  entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7fb3cb9
RSP: 002b:00000000fff0c26c EFLAGS: 00000282 ORIG_RAX: 000000000000016e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000000000
RDX: 0000000000000080 RSI: 0000000020000300 RDI: 00000000000005f4
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000

The buggy address belongs to the page:
page:ffffea0006c2afc0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
flags: 0x2fffc0000000000()
raw: 02fffc0000000000 0000000000000000 0000000000000000 00000000ffffffff
raw: 0000000000000000 ffffea0006c20101 0000000000000000 0000000000000000
page dumped because: kasan: bad access detected

Fix the issue replacing the unsafe function with strscpy() and
taking care of possible errors.

Fixes: 81e675c227ec ("netfilter: ebtables: add CONFIG_COMPAT support")
Reported-and-tested-by: syzbot+4e42a04e0bc33cb6c087@syzkaller.appspotmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/bridge/netfilter/ebtables.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 28a4c3490359..6ba639f6c51d 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1954,7 +1954,8 @@ static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt,
 	int off, pad = 0;
 	unsigned int size_kern, match_size = mwt->match_size;
 
-	strlcpy(name, mwt->u.name, sizeof(name));
+	if (strscpy(name, mwt->u.name, sizeof(name)) < 0)
+		return -EINVAL;
 
 	if (state->buf_kern_start)
 		dst = state->buf_kern_start + state->buf_kern_offset;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/9] netfilter: nft_meta: fix wrong value dereference in nft_meta_set_eval
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 1/9] netfilter: nf_tables: fix NULL pointer dereference on nft_ct_helper_obj_dump() Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 2/9] netfilter: ebtables: handle string from userspace with care Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 4/9] netfilter: nft_limit: fix packet ratelimiting Pablo Neira Ayuso
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Taehee Yoo <ap420073@gmail.com>

In the nft_meta_set_eval, nftrace value is dereferenced as u32 from sreg.
But correct type is u8. so that sometimes incorrect value is dereferenced.

Steps to reproduce:

   %nft add table ip filter
   %nft add chain ip filter input { type filter hook input priority 4\; }
   %nft add rule ip filter input nftrace set 0
   %nft monitor

Sometimes, we can see trace messages.

   trace id 16767227 ip filter input packet: iif "enp2s0"
   ether saddr xx:xx:xx:xx:xx:xx ether daddr xx:xx:xx:xx:xx:xx
   ip saddr 192.168.0.1 ip daddr 255.255.255.255 ip dscp cs0
   ip ecn not-ect ip
   trace id 16767227 ip filter input rule nftrace set 0 (verdict continue)
   trace id 16767227 ip filter input verdict continue
   trace id 16767227 ip filter input

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_meta.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 8fb91940e2e7..204af9899482 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -234,7 +234,7 @@ void nft_meta_set_eval(const struct nft_expr *expr,
 	struct sk_buff *skb = pkt->skb;
 	u32 *sreg = &regs->data[meta->sreg];
 	u32 value = *sreg;
-	u8 pkt_type;
+	u8 value8;
 
 	switch (meta->key) {
 	case NFT_META_MARK:
@@ -244,15 +244,17 @@ void nft_meta_set_eval(const struct nft_expr *expr,
 		skb->priority = value;
 		break;
 	case NFT_META_PKTTYPE:
-		pkt_type = nft_reg_load8(sreg);
+		value8 = nft_reg_load8(sreg);
 
-		if (skb->pkt_type != pkt_type &&
-		    skb_pkt_type_ok(pkt_type) &&
+		if (skb->pkt_type != value8 &&
+		    skb_pkt_type_ok(value8) &&
 		    skb_pkt_type_ok(skb->pkt_type))
-			skb->pkt_type = pkt_type;
+			skb->pkt_type = value8;
 		break;
 	case NFT_META_NFTRACE:
-		skb->nf_trace = !!value;
+		value8 = nft_reg_load8(sreg);
+
+		skb->nf_trace = !!value8;
 		break;
 	default:
 		WARN_ON(1);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/9] netfilter: nft_limit: fix packet ratelimiting
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2018-05-28 23:42 ` [PATCH 3/9] netfilter: nft_meta: fix wrong value dereference in nft_meta_set_eval Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 5/9] ipvs: fix buffer overflow with sync daemon and service Pablo Neira Ayuso
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Credit calculations for the packet ratelimiting are not correct, as per
the applied ratelimit of 25/second and burst 8, a total of 33 packets
should have been accepted.  This is true in iptables(33) but not in
nftables (~65). For packet ratelimiting, use:

	div_u64(limit->nsecs, limit->rate) * limit->burst;

to calculate credit, just like in iptables' xt_limit does.

Moreover, use default burst in iptables, users are expecting similar
behaviour.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_limit.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nft_limit.c b/net/netfilter/nft_limit.c
index a9fc298ef4c3..72f13a1144dd 100644
--- a/net/netfilter/nft_limit.c
+++ b/net/netfilter/nft_limit.c
@@ -51,10 +51,13 @@ static inline bool nft_limit_eval(struct nft_limit *limit, u64 cost)
 	return !limit->invert;
 }
 
+/* Use same default as in iptables. */
+#define NFT_LIMIT_PKT_BURST_DEFAULT	5
+
 static int nft_limit_init(struct nft_limit *limit,
-			  const struct nlattr * const tb[])
+			  const struct nlattr * const tb[], bool pkts)
 {
-	u64 unit;
+	u64 unit, tokens;
 
 	if (tb[NFTA_LIMIT_RATE] == NULL ||
 	    tb[NFTA_LIMIT_UNIT] == NULL)
@@ -68,18 +71,25 @@ static int nft_limit_init(struct nft_limit *limit,
 
 	if (tb[NFTA_LIMIT_BURST])
 		limit->burst = ntohl(nla_get_be32(tb[NFTA_LIMIT_BURST]));
-	else
-		limit->burst = 0;
+
+	if (pkts && limit->burst == 0)
+		limit->burst = NFT_LIMIT_PKT_BURST_DEFAULT;
 
 	if (limit->rate + limit->burst < limit->rate)
 		return -EOVERFLOW;
 
-	/* The token bucket size limits the number of tokens can be
-	 * accumulated. tokens_max specifies the bucket size.
-	 * tokens_max = unit * (rate + burst) / rate.
-	 */
-	limit->tokens = div_u64(limit->nsecs * (limit->rate + limit->burst),
-				limit->rate);
+	if (pkts) {
+		tokens = div_u64(limit->nsecs, limit->rate) * limit->burst;
+	} else {
+		/* The token bucket size limits the number of tokens can be
+		 * accumulated. tokens_max specifies the bucket size.
+		 * tokens_max = unit * (rate + burst) / rate.
+		 */
+		tokens = div_u64(limit->nsecs * (limit->rate + limit->burst),
+				 limit->rate);
+	}
+
+	limit->tokens = tokens;
 	limit->tokens_max = limit->tokens;
 
 	if (tb[NFTA_LIMIT_FLAGS]) {
@@ -144,7 +154,7 @@ static int nft_limit_pkts_init(const struct nft_ctx *ctx,
 	struct nft_limit_pkts *priv = nft_expr_priv(expr);
 	int err;
 
-	err = nft_limit_init(&priv->limit, tb);
+	err = nft_limit_init(&priv->limit, tb, true);
 	if (err < 0)
 		return err;
 
@@ -185,7 +195,7 @@ static int nft_limit_bytes_init(const struct nft_ctx *ctx,
 {
 	struct nft_limit *priv = nft_expr_priv(expr);
 
-	return nft_limit_init(priv, tb);
+	return nft_limit_init(priv, tb, false);
 }
 
 static int nft_limit_bytes_dump(struct sk_buff *skb,
@@ -246,7 +256,7 @@ static int nft_limit_obj_pkts_init(const struct nft_ctx *ctx,
 	struct nft_limit_pkts *priv = nft_obj_data(obj);
 	int err;
 
-	err = nft_limit_init(&priv->limit, tb);
+	err = nft_limit_init(&priv->limit, tb, true);
 	if (err < 0)
 		return err;
 
@@ -289,7 +299,7 @@ static int nft_limit_obj_bytes_init(const struct nft_ctx *ctx,
 {
 	struct nft_limit *priv = nft_obj_data(obj);
 
-	return nft_limit_init(priv, tb);
+	return nft_limit_init(priv, tb, false);
 }
 
 static int nft_limit_obj_bytes_dump(struct sk_buff *skb,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/9] ipvs: fix buffer overflow with sync daemon and service
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (3 preceding siblings ...)
  2018-05-28 23:42 ` [PATCH 4/9] netfilter: nft_limit: fix packet ratelimiting Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 6/9] netfilter: provide correct argument to nla_strlcpy() Pablo Neira Ayuso
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Julian Anastasov <ja@ssi.bg>

syzkaller reports for buffer overflow for interface name
when starting sync daemons [1]

What we do is that we copy user structure into larger stack
buffer but later we search NUL past the stack buffer.
The same happens for sched_name when adding/editing virtual server.

We are restricted by IP_VS_SCHEDNAME_MAXLEN and IP_VS_IFNAME_MAXLEN
being used as size in include/uapi/linux/ip_vs.h, so they
include the space for NUL.

As using strlcpy is wrong for unsafe source, replace it with
strscpy and add checks to return EINVAL if source string is not
NUL-terminated. The incomplete strlcpy fix comes from 2.6.13.

For the netlink interface reduce the len parameter for
IPVS_DAEMON_ATTR_MCAST_IFN and IPVS_SVC_ATTR_SCHED_NAME,
so that we get proper EINVAL.

[1]
kernel BUG at lib/string.c:1052!
invalid opcode: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
    (ftrace buffer empty)
Modules linked in:
CPU: 1 PID: 373 Comm: syz-executor936 Not tainted 4.17.0-rc4+ #45
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:fortify_panic+0x13/0x20 lib/string.c:1051
RSP: 0018:ffff8801c976f800 EFLAGS: 00010282
RAX: 0000000000000022 RBX: 0000000000000040 RCX: 0000000000000000
RDX: 0000000000000022 RSI: ffffffff8160f6f1 RDI: ffffed00392edef6
RBP: ffff8801c976f800 R08: ffff8801cf4c62c0 R09: ffffed003b5e4fb0
R10: ffffed003b5e4fb0 R11: ffff8801daf27d87 R12: ffff8801c976fa20
R13: ffff8801c976fae4 R14: ffff8801c976fae0 R15: 000000000000048b
FS:  00007fd99f75e700(0000) GS:ffff8801daf00000(0000)
knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000200001c0 CR3: 00000001d6843000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  strlen include/linux/string.h:270 [inline]
  strlcpy include/linux/string.h:293 [inline]
  do_ip_vs_set_ctl+0x31c/0x1d00 net/netfilter/ipvs/ip_vs_ctl.c:2388
  nf_sockopt net/netfilter/nf_sockopt.c:106 [inline]
  nf_setsockopt+0x7d/0xd0 net/netfilter/nf_sockopt.c:115
  ip_setsockopt+0xd8/0xf0 net/ipv4/ip_sockglue.c:1253
  udp_setsockopt+0x62/0xa0 net/ipv4/udp.c:2487
  ipv6_setsockopt+0x149/0x170 net/ipv6/ipv6_sockglue.c:917
  tcp_setsockopt+0x93/0xe0 net/ipv4/tcp.c:3057
  sock_common_setsockopt+0x9a/0xe0 net/core/sock.c:3046
  __sys_setsockopt+0x1bd/0x390 net/socket.c:1903
  __do_sys_setsockopt net/socket.c:1914 [inline]
  __se_sys_setsockopt net/socket.c:1911 [inline]
  __x64_sys_setsockopt+0xbe/0x150 net/socket.c:1911
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x447369
RSP: 002b:00007fd99f75dda8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 00000000006e39e4 RCX: 0000000000447369
RDX: 000000000000048b RSI: 0000000000000000 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000018 R09: 0000000000000000
R10: 00000000200001c0 R11: 0000000000000246 R12: 00000000006e39e0
R13: 75a1ff93f0896195 R14: 6f745f3168746576 R15: 0000000000000001
Code: 08 5b 41 5c 41 5d 41 5e 41 5f 5d c3 0f 0b 48 89 df e8 d2 8f 48 fa eb
de 55 48 89 fe 48 c7 c7 60 65 64 88 48 89 e5 e8 91 dd f3 f9 <0f> 0b 90 90
90 90 90 90 90 90 90 90 90 55 48 89 e5 41 57 41 56
RIP: fortify_panic+0x13/0x20 lib/string.c:1051 RSP: ffff8801c976f800

Reported-and-tested-by: syzbot+aac887f77319868646df@syzkaller.appspotmail.com
Fixes: e4ff67513096 ("ipvs: add sync_maxlen parameter for the sync daemon")
Fixes: 4da62fc70d7c ("[IPVS]: Fix for overflows")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_ctl.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index f36098887ad0..3ecca0616d8c 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2381,8 +2381,10 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
 			struct ipvs_sync_daemon_cfg cfg;
 
 			memset(&cfg, 0, sizeof(cfg));
-			strlcpy(cfg.mcast_ifn, dm->mcast_ifn,
-				sizeof(cfg.mcast_ifn));
+			ret = -EINVAL;
+			if (strscpy(cfg.mcast_ifn, dm->mcast_ifn,
+				    sizeof(cfg.mcast_ifn)) <= 0)
+				goto out_dec;
 			cfg.syncid = dm->syncid;
 			ret = start_sync_thread(ipvs, &cfg, dm->state);
 		} else {
@@ -2420,12 +2422,19 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
 		}
 	}
 
+	if ((cmd == IP_VS_SO_SET_ADD || cmd == IP_VS_SO_SET_EDIT) &&
+	    strnlen(usvc.sched_name, IP_VS_SCHEDNAME_MAXLEN) ==
+	    IP_VS_SCHEDNAME_MAXLEN) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	/* Check for valid protocol: TCP or UDP or SCTP, even for fwmark!=0 */
 	if (usvc.protocol != IPPROTO_TCP && usvc.protocol != IPPROTO_UDP &&
 	    usvc.protocol != IPPROTO_SCTP) {
-		pr_err("set_ctl: invalid protocol: %d %pI4:%d %s\n",
+		pr_err("set_ctl: invalid protocol: %d %pI4:%d\n",
 		       usvc.protocol, &usvc.addr.ip,
-		       ntohs(usvc.port), usvc.sched_name);
+		       ntohs(usvc.port));
 		ret = -EFAULT;
 		goto out_unlock;
 	}
@@ -2847,7 +2856,7 @@ static const struct nla_policy ip_vs_cmd_policy[IPVS_CMD_ATTR_MAX + 1] = {
 static const struct nla_policy ip_vs_daemon_policy[IPVS_DAEMON_ATTR_MAX + 1] = {
 	[IPVS_DAEMON_ATTR_STATE]	= { .type = NLA_U32 },
 	[IPVS_DAEMON_ATTR_MCAST_IFN]	= { .type = NLA_NUL_STRING,
-					    .len = IP_VS_IFNAME_MAXLEN },
+					    .len = IP_VS_IFNAME_MAXLEN - 1 },
 	[IPVS_DAEMON_ATTR_SYNC_ID]	= { .type = NLA_U32 },
 	[IPVS_DAEMON_ATTR_SYNC_MAXLEN]	= { .type = NLA_U16 },
 	[IPVS_DAEMON_ATTR_MCAST_GROUP]	= { .type = NLA_U32 },
@@ -2865,7 +2874,7 @@ static const struct nla_policy ip_vs_svc_policy[IPVS_SVC_ATTR_MAX + 1] = {
 	[IPVS_SVC_ATTR_PORT]		= { .type = NLA_U16 },
 	[IPVS_SVC_ATTR_FWMARK]		= { .type = NLA_U32 },
 	[IPVS_SVC_ATTR_SCHED_NAME]	= { .type = NLA_NUL_STRING,
-					    .len = IP_VS_SCHEDNAME_MAXLEN },
+					    .len = IP_VS_SCHEDNAME_MAXLEN - 1 },
 	[IPVS_SVC_ATTR_PE_NAME]		= { .type = NLA_NUL_STRING,
 					    .len = IP_VS_PENAME_MAXLEN },
 	[IPVS_SVC_ATTR_FLAGS]		= { .type = NLA_BINARY,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/9] netfilter: provide correct argument to nla_strlcpy()
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (4 preceding siblings ...)
  2018-05-28 23:42 ` [PATCH 5/9] ipvs: fix buffer overflow with sync daemon and service Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 7/9] netfilter: nf_tables: disable preemption in nft_update_chain_stats() Pablo Neira Ayuso
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Eric Dumazet <edumazet@google.com>

Recent patch forgot to remove nla_data(), upsetting syzkaller a bit.

BUG: KASAN: slab-out-of-bounds in nla_strlcpy+0x13d/0x150 lib/nlattr.c:314
Read of size 1 at addr ffff8801ad1f4fdd by task syz-executor189/4509

CPU: 1 PID: 4509 Comm: syz-executor189 Not tainted 4.17.0-rc6+ #62
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x1b9/0x294 lib/dump_stack.c:113
 print_address_description+0x6c/0x20b mm/kasan/report.c:256
 kasan_report_error mm/kasan/report.c:354 [inline]
 kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
 __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
 nla_strlcpy+0x13d/0x150 lib/nlattr.c:314
 nfnl_acct_new+0x574/0xc50 net/netfilter/nfnetlink_acct.c:118
 nfnetlink_rcv_msg+0xdb5/0xff0 net/netfilter/nfnetlink.c:212
 netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2448
 nfnetlink_rcv+0x1fe/0x1ba0 net/netfilter/nfnetlink.c:513
 netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
 netlink_unicast+0x58b/0x740 net/netlink/af_netlink.c:1336
 netlink_sendmsg+0x9f0/0xfa0 net/netlink/af_netlink.c:1901
 sock_sendmsg_nosec net/socket.c:629 [inline]
 sock_sendmsg+0xd5/0x120 net/socket.c:639
 sock_write_iter+0x35a/0x5a0 net/socket.c:908
 call_write_iter include/linux/fs.h:1784 [inline]
 new_sync_write fs/read_write.c:474 [inline]
 __vfs_write+0x64d/0x960 fs/read_write.c:487
 vfs_write+0x1f8/0x560 fs/read_write.c:549
 ksys_write+0xf9/0x250 fs/read_write.c:598
 __do_sys_write fs/read_write.c:610 [inline]
 __se_sys_write fs/read_write.c:607 [inline]
 __x64_sys_write+0x73/0xb0 fs/read_write.c:607

Fixes: 4e09fc873d92 ("netfilter: prefer nla_strlcpy for dealing with NLA_STRING attributes")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Florian Westphal <fw@strlen.de>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_acct.c     | 2 +-
 net/netfilter/nfnetlink_cthelper.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 6ddf89183e7b..a0e5adf0b3b6 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -115,7 +115,7 @@ static int nfnl_acct_new(struct net *net, struct sock *nfnl,
 		nfacct->flags = flags;
 	}
 
-	nla_strlcpy(nfacct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
+	nla_strlcpy(nfacct->name, tb[NFACCT_NAME], NFACCT_NAME_MAX);
 
 	if (tb[NFACCT_BYTES]) {
 		atomic64_set(&nfacct->bytes,
diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index fa026b269b36..cb5b5f207777 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -150,7 +150,7 @@ nfnl_cthelper_expect_policy(struct nf_conntrack_expect_policy *expect_policy,
 		return -EINVAL;
 
 	nla_strlcpy(expect_policy->name,
-		    nla_data(tb[NFCTH_POLICY_NAME]), NF_CT_HELPER_NAME_LEN);
+		    tb[NFCTH_POLICY_NAME], NF_CT_HELPER_NAME_LEN);
 	expect_policy->max_expected =
 		ntohl(nla_get_be32(tb[NFCTH_POLICY_EXPECT_MAX]));
 	if (expect_policy->max_expected > NF_CT_EXPECT_MAX_CNT)
@@ -235,7 +235,7 @@ nfnl_cthelper_create(const struct nlattr * const tb[],
 		goto err1;
 
 	nla_strlcpy(helper->name,
-		    nla_data(tb[NFCTH_NAME]), NF_CT_HELPER_NAME_LEN);
+		    tb[NFCTH_NAME], NF_CT_HELPER_NAME_LEN);
 	size = ntohl(nla_get_be32(tb[NFCTH_PRIV_DATA_LEN]));
 	if (size > FIELD_SIZEOF(struct nf_conn_help, data)) {
 		ret = -ENOMEM;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/9] netfilter: nf_tables: disable preemption in nft_update_chain_stats()
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (5 preceding siblings ...)
  2018-05-28 23:42 ` [PATCH 6/9] netfilter: provide correct argument to nla_strlcpy() Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 8/9] netfilter: nf_tables: fix NULL-ptr in nf_tables_dump_obj() Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

This patch fixes the following splat.

[118709.054937] BUG: using smp_processor_id() in preemptible [00000000] code: test/1571
[118709.054970] caller is nft_update_chain_stats.isra.4+0x53/0x97 [nf_tables]
[118709.054980] CPU: 2 PID: 1571 Comm: test Not tainted 4.17.0-rc6+ #335
[...]
[118709.054992] Call Trace:
[118709.055011]  dump_stack+0x5f/0x86
[118709.055026]  check_preemption_disabled+0xd4/0xe4

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 942702a2776f..40e744572283 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -126,15 +126,15 @@ static noinline void nft_update_chain_stats(const struct nft_chain *chain,
 	if (!base_chain->stats)
 		return;
 
+	local_bh_disable();
 	stats = this_cpu_ptr(rcu_dereference(base_chain->stats));
 	if (stats) {
-		local_bh_disable();
 		u64_stats_update_begin(&stats->syncp);
 		stats->pkts++;
 		stats->bytes += pkt->skb->len;
 		u64_stats_update_end(&stats->syncp);
-		local_bh_enable();
 	}
+	local_bh_enable();
 }
 
 struct nft_jumpstack {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/9] netfilter: nf_tables: fix NULL-ptr in nf_tables_dump_obj()
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (6 preceding siblings ...)
  2018-05-28 23:42 ` [PATCH 7/9] netfilter: nf_tables: disable preemption in nft_update_chain_stats() Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-28 23:42 ` [PATCH 9/9] netfilter: nf_tables: increase nft_counters_enabled in nft_chain_stats_replace() Pablo Neira Ayuso
  2018-05-29  2:39 ` [PATCH 0/9] Netfilter/IPVS fixes for net David Miller
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Taehee Yoo <ap420073@gmail.com>

The table field in nft_obj_filter is not an array. In order to check
tablename, we should check if the pointer is set.

Test commands:

   %nft add table ip filter
   %nft add counter ip filter ct1
   %nft reset counters

Splat looks like:

[  306.510504] kasan: CONFIG_KASAN_INLINE enabled
[  306.516184] kasan: GPF could be caused by NULL-ptr deref or user memory access
[  306.524775] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[  306.528284] Modules linked in: nft_objref nft_counter nf_tables nfnetlink ip_tables x_tables
[  306.528284] CPU: 0 PID: 1488 Comm: nft Not tainted 4.17.0-rc4+ #17
[  306.528284] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015
[  306.528284] RIP: 0010:nf_tables_dump_obj+0x52c/0xa70 [nf_tables]
[  306.528284] RSP: 0018:ffff8800b6cb7520 EFLAGS: 00010246
[  306.528284] RAX: 0000000000000000 RBX: ffff8800b6c49820 RCX: 0000000000000000
[  306.528284] RDX: 0000000000000000 RSI: dffffc0000000000 RDI: ffffed0016d96e9a
[  306.528284] RBP: ffff8800b6cb75c0 R08: ffffed00236fce7c R09: ffffed00236fce7b
[  306.528284] R10: ffffffff9f6241e8 R11: ffffed00236fce7c R12: ffff880111365108
[  306.528284] R13: 0000000000000000 R14: ffff8800b6c49860 R15: ffff8800b6c49860
[  306.528284] FS:  00007f838b007700(0000) GS:ffff88011b600000(0000) knlGS:0000000000000000
[  306.528284] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  306.528284] CR2: 00007ffeafabcf78 CR3: 00000000b6cbe000 CR4: 00000000001006f0
[  306.528284] Call Trace:
[  306.528284]  netlink_dump+0x470/0xa20
[  306.528284]  __netlink_dump_start+0x5ae/0x690
[  306.528284]  ? nf_tables_getobj+0x1b3/0x740 [nf_tables]
[  306.528284]  nf_tables_getobj+0x2f5/0x740 [nf_tables]
[  306.528284]  ? nft_obj_notify+0x100/0x100 [nf_tables]
[  306.528284]  ? nf_tables_getobj+0x740/0x740 [nf_tables]
[  306.528284]  ? nf_tables_dump_flowtable_done+0x70/0x70 [nf_tables]
[  306.528284]  ? nft_obj_notify+0x100/0x100 [nf_tables]
[  306.528284]  nfnetlink_rcv_msg+0x8ff/0x932 [nfnetlink]
[  306.528284]  ? nfnetlink_rcv_msg+0x216/0x932 [nfnetlink]
[  306.528284]  netlink_rcv_skb+0x1c9/0x2f0
[  306.528284]  ? nfnetlink_bind+0x1d0/0x1d0 [nfnetlink]
[  306.528284]  ? debug_check_no_locks_freed+0x270/0x270
[  306.528284]  ? netlink_ack+0x7a0/0x7a0
[  306.528284]  ? ns_capable_common+0x6e/0x110
[ ... ]

Fixes: e46abbcc05aa8 ("netfilter: nf_tables: Allow table names of up to 255 chars")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 91e80aa852d6..2bdc8767aa40 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4706,7 +4706,7 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
 			if (idx > s_idx)
 				memset(&cb->args[1], 0,
 				       sizeof(cb->args) - sizeof(cb->args[0]));
-			if (filter && filter->table[0] &&
+			if (filter && filter->table &&
 			    strcmp(filter->table, table->name))
 				goto cont;
 			if (filter &&
@@ -5380,7 +5380,7 @@ static int nf_tables_dump_flowtable(struct sk_buff *skb,
 			if (idx > s_idx)
 				memset(&cb->args[1], 0,
 				       sizeof(cb->args) - sizeof(cb->args[0]));
-			if (filter && filter->table[0] &&
+			if (filter && filter->table &&
 			    strcmp(filter->table, table->name))
 				goto cont;
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 9/9] netfilter: nf_tables: increase nft_counters_enabled in nft_chain_stats_replace()
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (7 preceding siblings ...)
  2018-05-28 23:42 ` [PATCH 8/9] netfilter: nf_tables: fix NULL-ptr in nf_tables_dump_obj() Pablo Neira Ayuso
@ 2018-05-28 23:42 ` Pablo Neira Ayuso
  2018-05-29  2:39 ` [PATCH 0/9] Netfilter/IPVS fixes for net David Miller
  9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 23:42 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Taehee Yoo <ap420073@gmail.com>

When a chain is updated, a counter can be attached. if so,
the nft_counters_enabled should be increased.

test commands:

   %nft add table ip filter
   %nft add chain ip filter input { type filter hook input priority 4\; }
   %iptables-compat -Z input
   %nft delete chain ip filter input

we can see below messages.

[  286.443720] jump label: negative count!
[  286.448278] WARNING: CPU: 0 PID: 1459 at kernel/jump_label.c:197 __static_key_slow_dec_cpuslocked+0x6f/0xf0
[  286.449144] Modules linked in: nf_tables nfnetlink ip_tables x_tables
[  286.449144] CPU: 0 PID: 1459 Comm: nft Tainted: G        W         4.17.0-rc2+ #12
[  286.449144] RIP: 0010:__static_key_slow_dec_cpuslocked+0x6f/0xf0
[  286.449144] RSP: 0018:ffff88010e5176f0 EFLAGS: 00010286
[  286.449144] RAX: 000000000000001b RBX: ffffffffc0179500 RCX: ffffffffb8a82522
[  286.449144] RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffff88011b7e5eac
[  286.449144] RBP: 0000000000000000 R08: ffffed00236fce5c R09: ffffed00236fce5b
[  286.449144] R10: ffffffffc0179503 R11: ffffed00236fce5c R12: 0000000000000000
[  286.449144] R13: ffff88011a28e448 R14: ffff88011a28e470 R15: dffffc0000000000
[  286.449144] FS:  00007f0384328700(0000) GS:ffff88011b600000(0000) knlGS:0000000000000000
[  286.449144] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  286.449144] CR2: 00007f038394bf10 CR3: 0000000104a86000 CR4: 00000000001006f0
[  286.449144] Call Trace:
[  286.449144]  static_key_slow_dec+0x6a/0x70
[  286.449144]  nf_tables_chain_destroy+0x19d/0x210 [nf_tables]
[  286.449144]  nf_tables_commit+0x1891/0x1c50 [nf_tables]
[  286.449144]  nfnetlink_rcv+0x1148/0x13d0 [nfnetlink]
[ ... ]

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 2bdc8767aa40..501e48a7965b 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1298,8 +1298,10 @@ static void nft_chain_stats_replace(struct nft_base_chain *chain,
 		rcu_assign_pointer(chain->stats, newstats);
 		synchronize_rcu();
 		free_percpu(oldstats);
-	} else
+	} else {
 		rcu_assign_pointer(chain->stats, newstats);
+		static_branch_inc(&nft_counters_enabled);
+	}
 }
 
 static void nf_tables_chain_destroy(struct nft_ctx *ctx)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/9] Netfilter/IPVS fixes for net
  2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (8 preceding siblings ...)
  2018-05-28 23:42 ` [PATCH 9/9] netfilter: nf_tables: increase nft_counters_enabled in nft_chain_stats_replace() Pablo Neira Ayuso
@ 2018-05-29  2:39 ` David Miller
  9 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2018-05-29  2:39 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 29 May 2018 01:42:12 +0200

> The following patchset contains Netfilter/IPVS fixes for your net tree:
> 
> 1) Null pointer dereference when dumping conntrack helper configuration,
>    from Taehee Yoo.
> 
> 2) Missing sanitization in ebtables extension name through compat,
>    from Paolo Abeni.
> 
> 3) Broken fetch of tracing value, from Taehee Yoo.
> 
> 4) Incorrect arithmetics in packet ratelimiting.
> 
> 5) Buffer overflow in IPVS sync daemon, from Julian Anastasov.
> 
> 6) Wrong argument to nla_strlcpy() in nfnetlink_{acct,cthelper},
>    from Eric Dumazet.
> 
> 7) Fix splat in nft_update_chain_stats().
> 
> 8) Null pointer dereference from object netlink dump path, from
>    Taehee Yoo.
> 
> 9) Missing static_branch_inc() when enabling counters in existing
>    chain, from Taehee Yoo.
> 
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-05-29  2:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-28 23:42 [PATCH 0/9] Netfilter/IPVS fixes for net Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 1/9] netfilter: nf_tables: fix NULL pointer dereference on nft_ct_helper_obj_dump() Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 2/9] netfilter: ebtables: handle string from userspace with care Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 3/9] netfilter: nft_meta: fix wrong value dereference in nft_meta_set_eval Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 4/9] netfilter: nft_limit: fix packet ratelimiting Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 5/9] ipvs: fix buffer overflow with sync daemon and service Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 6/9] netfilter: provide correct argument to nla_strlcpy() Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 7/9] netfilter: nf_tables: disable preemption in nft_update_chain_stats() Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 8/9] netfilter: nf_tables: fix NULL-ptr in nf_tables_dump_obj() Pablo Neira Ayuso
2018-05-28 23:42 ` [PATCH 9/9] netfilter: nf_tables: increase nft_counters_enabled in nft_chain_stats_replace() Pablo Neira Ayuso
2018-05-29  2:39 ` [PATCH 0/9] Netfilter/IPVS fixes for net David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).