REGRESSION in nfnetlink on 3.18.x (bisected)

REGRESSION in nfnetlink on 3.18.x (bisected)

[Andre Tomt]

On at least Ubuntu 14.04 LTS and Ubuntu 14.10 "conntrack -E" has started 
failing with Linux 3.18.x. conntrack -L still works.

14.04 and 14.10 ships conntrack-utils version 1.4.1, but 1.4.2 does not 
work either.

It fails with:
> # conntrack -E
> conntrack v1.4.2 (conntrack-tools): Can't open handler

strace shows:
> bind(3, {sa_family=AF_NETLINK, pid=0, groups=00000000}, 12) = 0
> getsockname(3, {sa_family=AF_NETLINK, pid=14092, groups=00000000}, [12]) = 0
> bind(3, {sa_family=AF_NETLINK, pid=14092, groups=00000007}, 12) = -1 EINVAL (Invalid argument)

Reverting 97840cb67ff5ac8add836684f011fd838518d698 - netfilter: 
nfnetlink: fix insufficient validation in nfnetlink_bind

makes everything work again on my systems.

I'm testing with
> # modprobe nfnetlink
> # modprobe nf_conntrack_netlink
> # modprobe nf_conntrack_ipv4
> # conntrack -E

Re: REGRESSION in nfnetlink on 3.18.x (bisected)

[Andre Tomt]

Copying change author

On 21. des. 2014 00:33, Andre Tomt wrote:
> On at least Ubuntu 14.04 LTS and Ubuntu 14.10 "conntrack -E" has started
> failing with Linux 3.18.x. conntrack -L still works.
>
> 14.04 and 14.10 ships conntrack-utils version 1.4.1, but 1.4.2 does not
> work either.
>
> It fails with:
>> # conntrack -E
>> conntrack v1.4.2 (conntrack-tools): Can't open handler
>
> strace shows:
>> bind(3, {sa_family=AF_NETLINK, pid=0, groups=00000000}, 12) = 0
>> getsockname(3, {sa_family=AF_NETLINK, pid=14092, groups=00000000},
>> [12]) = 0
>> bind(3, {sa_family=AF_NETLINK, pid=14092, groups=00000007}, 12) = -1
>> EINVAL (Invalid argument)
>
> Reverting 97840cb67ff5ac8add836684f011fd838518d698 - netfilter:
> nfnetlink: fix insufficient validation in nfnetlink_bind
>
> makes everything work again on my systems.
>
> I'm testing with
>> # modprobe nfnetlink
>> # modprobe nf_conntrack_netlink
>> # modprobe nf_conntrack_ipv4
>> # conntrack -E
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: REGRESSION in nfnetlink on 3.18.x (bisected)

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

On Sun, Dec 21, 2014 at 12:33:51AM +0100, Andre Tomt wrote:
> On at least Ubuntu 14.04 LTS and Ubuntu 14.10 "conntrack -E" has
> started failing with Linux 3.18.x. conntrack -L still works.
> 
> 14.04 and 14.10 ships conntrack-utils version 1.4.1, but 1.4.2 does
> not work either.
> 
> It fails with:
> ># conntrack -E
> >conntrack v1.4.2 (conntrack-tools): Can't open handler
> 
> strace shows:
> >bind(3, {sa_family=AF_NETLINK, pid=0, groups=00000000}, 12) = 0
> >getsockname(3, {sa_family=AF_NETLINK, pid=14092, groups=00000000}, [12]) = 0
> >bind(3, {sa_family=AF_NETLINK, pid=14092, groups=00000007}, 12) = -1 EINVAL (Invalid argument)
> 
> Reverting 97840cb67ff5ac8add836684f011fd838518d698 - netfilter:
> nfnetlink: fix insufficient validation in nfnetlink_bind

Could you give a test to this patch? Thanks.

[-- Attachment #2: 0001-netlink-fix-wrong-subscription-bitmask-to-group-mapp.patch --]
[-- Type: text/x-diff, Size: 1827 bytes --]

>From f4f65150fd2129607a7bd25f007c258045237c8c Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Sun, 21 Dec 2014 21:48:36 +0100
Subject: [PATCH nf] netlink: fix wrong subscription bitmask to group mapping in
 binding callbacks

The subscription bitmask passed via struct sockaddr_nl is converted to
the group number when calling the netlink_bind() and netlink_unbind()
callbacks.

The conversion is however incorrect since bitmask (1 << 0) needs to be
mapped to group number 1. Note that you cannot specify the group number 0
(usually known as _NONE) from setsockopt() using NETLINK_ADD_MEMBERSHIP
since this is rejected through -EINVAL.

This problem became noticeable since 97840cb ("netfilter: nfnetlink:
fix insufficient validation in nfnetlink_bind") when binding to bitmask
(1 << 0) in ctnetlink.

Reported-by: Andre Tomt <andre@tomt.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netlink/af_netlink.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 074cf3e..cbcf73b 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1420,7 +1420,7 @@ static void netlink_unbind(int group, long unsigned int groups,
 
 	for (undo = 0; undo < group; undo++)
 		if (test_bit(undo, &groups))
-			nlk->netlink_unbind(undo);
+			nlk->netlink_unbind(undo + 1);
 }
 
 static int netlink_bind(struct socket *sock, struct sockaddr *addr,
@@ -1458,7 +1458,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 		for (group = 0; group < nlk->ngroups; group++) {
 			if (!test_bit(group, &groups))
 				continue;
-			err = nlk->netlink_bind(group);
+			err = nlk->netlink_bind(group + 1);
 			if (!err)
 				continue;
 			netlink_unbind(group, groups, nlk);
-- 
1.7.10.4

Re: REGRESSION in nfnetlink on 3.18.x (bisected)

[Andre Tomt]

On 22. des. 2014 12:56, Pablo Neira Ayuso wrote:
> On Sun, Dec 21, 2014 at 12:33:51AM +0100, Andre Tomt wrote:
>> On at least Ubuntu 14.04 LTS and Ubuntu 14.10 "conntrack -E" has
>> started failing with Linux 3.18.x. conntrack -L still works.
>>
>> 14.04 and 14.10 ships conntrack-utils version 1.4.1, but 1.4.2 does
>> not work either.
>>
>> It fails with:
>>> # conntrack -E
>>> conntrack v1.4.2 (conntrack-tools): Can't open handler
>>
>> strace shows:
>>> bind(3, {sa_family=AF_NETLINK, pid=0, groups=00000000}, 12) = 0
>>> getsockname(3, {sa_family=AF_NETLINK, pid=14092, groups=00000000}, [12]) = 0
>>> bind(3, {sa_family=AF_NETLINK, pid=14092, groups=00000007}, 12) = -1 EINVAL (Invalid argument)
>>
>> Reverting 97840cb67ff5ac8add836684f011fd838518d698 - netfilter:
>> nfnetlink: fix insufficient validation in nfnetlink_bind
>
> Could you give a test to this patch? Thanks.
>

Initial testing looks good with this patch applied on top of 3.18.1
I will give it a spin on some more systems tomorrow.

Thanks

Re: REGRESSION in nfnetlink on 3.18.x (bisected)

[Andre Tomt]

On 23. des. 2014 00:23, Andre Tomt wrote:
> On 22. des. 2014 12:56, Pablo Neira Ayuso wrote:
>> Could you give a test to this patch? Thanks.
>>
>
> Initial testing looks good with this patch applied on top of 3.18.1
> I will give it a spin on some more systems tomorrow.

No news is good news :-)

~10 3.18.x systems in various roles have had this fix for two weeks with 
no issues.

Re: REGRESSION in nfnetlink on 3.18.x (bisected)

[Pablo Neira Ayuso]

On Wed, Jan 07, 2015 at 11:03:10PM +0100, Andre Tomt wrote:
> On 23. des. 2014 00:23, Andre Tomt wrote:
> >On 22. des. 2014 12:56, Pablo Neira Ayuso wrote:
> >>Could you give a test to this patch? Thanks.
> >>
> >
> >Initial testing looks good with this patch applied on top of 3.18.1
> >I will give it a spin on some more systems tomorrow.
> 
> No news is good news :-)
> 
> ~10 3.18.x systems in various roles have had this fix for two weeks
> with no issues.

Thanks a lot for testing. I already sent the patch to David:

http://patchwork.ozlabs.org/patch/426205/