netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Netfilter fixes for net
@ 2014-11-20 12:30 Pablo Neira Ayuso
  2014-11-20 12:30 ` [PATCH 1/2] netfilter: nfnetlink: fix insufficient validation in nfnetlink_bind Pablo Neira Ayuso
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2014-11-20 12:30 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains two bugfixes for your net tree, they are:

1) Validate netlink group from nfnetlink to avoid an out of bound array
   access. This should only happen with superuser priviledges though.
   Discovered by Andrey Ryabinin using trinity.

2) Don't push ethernet header before calling the netfilter output hook
   for multicast traffic, this breaks ebtables since it expects to see
   skb->data pointing to the network header, patch from Linus Luessing.

You can pull these changes from:

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

Thanks!

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

The following changes since commit feb91a02ccb09661507f170b2a444aec94f307f9:

  ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs (2014-11-16 16:55:06 -0500)

are available in the git repository at:

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

for you to fetch changes up to f0b4eeced518c632210ef2aea44fc92cc9e86cce:

  bridge: fix netfilter/NF_BR_LOCAL_OUT for own, locally generated queries (2014-11-17 12:38:02 +0100)

----------------------------------------------------------------
Linus Lüssing (1):
      bridge: fix netfilter/NF_BR_LOCAL_OUT for own, locally generated queries

Pablo Neira Ayuso (1):
      netfilter: nfnetlink: fix insufficient validation in nfnetlink_bind

 net/bridge/br_multicast.c |    3 +--
 net/netfilter/nfnetlink.c |   12 +++++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

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

* [PATCH 1/2] netfilter: nfnetlink: fix insufficient validation in nfnetlink_bind
  2014-11-20 12:30 [PATCH 0/2] Netfilter fixes for net Pablo Neira Ayuso
@ 2014-11-20 12:30 ` Pablo Neira Ayuso
  2014-11-20 12:30 ` [PATCH 2/2] bridge: fix netfilter/NF_BR_LOCAL_OUT for own, locally generated queries Pablo Neira Ayuso
  2014-11-21  5:12 ` [PATCH 0/2] Netfilter fixes for net David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2014-11-20 12:30 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Make sure the netlink group exists, otherwise you can trigger an out
of bound array memory access from the netlink_bind() path. This splat
can only be triggered only by superuser.

[  180.203600] UBSan: Undefined behaviour in ../net/netfilter/nfnetlink.c:467:28
[  180.204249] index 9 is out of range for type 'int [9]'
[  180.204697] CPU: 0 PID: 1771 Comm: trinity-main Not tainted 3.18.0-rc4-mm1+ #122
[  180.205365] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
+04/01/2014
[  180.206498]  0000000000000018 0000000000000000 0000000000000009 ffff88007bdf7da8
[  180.207220]  ffffffff82b0ef5f 0000000000000092 ffffffff845ae2e0 ffff88007bdf7db8
[  180.207887]  ffffffff8199e489 ffff88007bdf7e18 ffffffff8199ea22 0000003900000000
[  180.208639] Call Trace:
[  180.208857] dump_stack (lib/dump_stack.c:52)
[  180.209370] ubsan_epilogue (lib/ubsan.c:174)
[  180.209849] __ubsan_handle_out_of_bounds (lib/ubsan.c:400)
[  180.210512] nfnetlink_bind (net/netfilter/nfnetlink.c:467)
[  180.210986] netlink_bind (net/netlink/af_netlink.c:1483)
[  180.211495] SYSC_bind (net/socket.c:1541)

Moreover, define the missing nf_tables and nf_acct multicast groups too.

Reported-by: Andrey Ryabinin <a.ryabinin@samsung.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 6c5a915..13c2e17 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -47,6 +47,8 @@ static const int nfnl_group2type[NFNLGRP_MAX+1] = {
 	[NFNLGRP_CONNTRACK_EXP_NEW]	= NFNL_SUBSYS_CTNETLINK_EXP,
 	[NFNLGRP_CONNTRACK_EXP_UPDATE]	= NFNL_SUBSYS_CTNETLINK_EXP,
 	[NFNLGRP_CONNTRACK_EXP_DESTROY] = NFNL_SUBSYS_CTNETLINK_EXP,
+	[NFNLGRP_NFTABLES]		= NFNL_SUBSYS_NFTABLES,
+	[NFNLGRP_ACCT_QUOTA]		= NFNL_SUBSYS_ACCT,
 };
 
 void nfnl_lock(__u8 subsys_id)
@@ -464,7 +466,12 @@ static void nfnetlink_rcv(struct sk_buff *skb)
 static int nfnetlink_bind(int group)
 {
 	const struct nfnetlink_subsystem *ss;
-	int type = nfnl_group2type[group];
+	int type;
+
+	if (group <= NFNLGRP_NONE || group > NFNLGRP_MAX)
+		return -EINVAL;
+
+	type = nfnl_group2type[group];
 
 	rcu_read_lock();
 	ss = nfnetlink_get_subsys(type);
@@ -514,6 +521,9 @@ static int __init nfnetlink_init(void)
 {
 	int i;
 
+	for (i = NFNLGRP_NONE + 1; i <= NFNLGRP_MAX; i++)
+		BUG_ON(nfnl_group2type[i] == NFNL_SUBSYS_NONE);
+
 	for (i=0; i<NFNL_SUBSYS_COUNT; i++)
 		mutex_init(&table[i].mutex);
 
-- 
1.7.10.4

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

* [PATCH 2/2] bridge: fix netfilter/NF_BR_LOCAL_OUT for own, locally generated queries
  2014-11-20 12:30 [PATCH 0/2] Netfilter fixes for net Pablo Neira Ayuso
  2014-11-20 12:30 ` [PATCH 1/2] netfilter: nfnetlink: fix insufficient validation in nfnetlink_bind Pablo Neira Ayuso
@ 2014-11-20 12:30 ` Pablo Neira Ayuso
  2014-11-21  5:12 ` [PATCH 0/2] Netfilter fixes for net David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2014-11-20 12:30 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Linus Lüssing <linus.luessing@web.de>

Ebtables on the OUTPUT chain (NF_BR_LOCAL_OUT) would not work as expected
for both locally generated IGMP and MLD queries. The IP header specific
filter options are off by 14 Bytes for netfilter (actual output on
interfaces is fine).

NF_HOOK() expects the skb->data to point to the IP header, not the
ethernet one (while dev_queue_xmit() does not). Luckily there is an
br_dev_queue_push_xmit() helper function already - let's just use that.

Introduced by eb1d16414339a6e113d89e2cca2556005d7ce919
("bridge: Add core IGMP snooping support")

Ebtables example:

$ ebtables -I OUTPUT -p IPv6 -o eth1 --logical-out br0 \
	--log --log-level 6 --log-ip6 --log-prefix="~EBT: " -j DROP

before (broken):

~EBT:  IN= OUT=eth1 MAC source = 02:04:64:a4:39:c2 \
	MAC dest = 33:33:00:00:00:01 proto = 0x86dd IPv6 \
	SRC=64a4:39c2:86dd:6000:0000:0020:0001:fe80 IPv6 \
	DST=0000:0000:0000:0004:64ff:fea4:39c2:ff02, \
	IPv6 priority=0x3, Next Header=2

after (working):

~EBT:  IN= OUT=eth1 MAC source = 02:04:64:a4:39:c2 \
	MAC dest = 33:33:00:00:00:01 proto = 0x86dd IPv6 \
	SRC=fe80:0000:0000:0000:0004:64ff:fea4:39c2 IPv6 \
	DST=ff02:0000:0000:0000:0000:0000:0000:0001, \
	IPv6 priority=0x0, Next Header=0

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/bridge/br_multicast.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 648d79c..c465876 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -813,10 +813,9 @@ static void __br_multicast_send_query(struct net_bridge *br,
 		return;
 
 	if (port) {
-		__skb_push(skb, sizeof(struct ethhdr));
 		skb->dev = port->dev;
 		NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
-			dev_queue_xmit);
+			br_dev_queue_push_xmit);
 	} else {
 		br_multicast_select_own_querier(br, ip, skb);
 		netif_rx(skb);
-- 
1.7.10.4

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

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

* Re: [PATCH 0/2] Netfilter fixes for net
  2014-11-20 12:30 [PATCH 0/2] Netfilter fixes for net Pablo Neira Ayuso
  2014-11-20 12:30 ` [PATCH 1/2] netfilter: nfnetlink: fix insufficient validation in nfnetlink_bind Pablo Neira Ayuso
  2014-11-20 12:30 ` [PATCH 2/2] bridge: fix netfilter/NF_BR_LOCAL_OUT for own, locally generated queries Pablo Neira Ayuso
@ 2014-11-21  5:12 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2014-11-21  5:12 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 20 Nov 2014 13:30:49 +0100

> The following patchset contains two bugfixes for your net tree, they are:
> 
> 1) Validate netlink group from nfnetlink to avoid an out of bound array
>    access. This should only happen with superuser priviledges though.
>    Discovered by Andrey Ryabinin using trinity.
> 
> 2) Don't push ethernet header before calling the netfilter output hook
>    for multicast traffic, this breaks ebtables since it expects to see
>    skb->data pointing to the network header, patch from Linus Luessing.
> 
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thanks Pablo.

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

end of thread, other threads:[~2014-11-21  5:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-20 12:30 [PATCH 0/2] Netfilter fixes for net Pablo Neira Ayuso
2014-11-20 12:30 ` [PATCH 1/2] netfilter: nfnetlink: fix insufficient validation in nfnetlink_bind Pablo Neira Ayuso
2014-11-20 12:30 ` [PATCH 2/2] bridge: fix netfilter/NF_BR_LOCAL_OUT for own, locally generated queries Pablo Neira Ayuso
2014-11-21  5:12 ` [PATCH 0/2] Netfilter 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).