RE: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
From: Jason Gunthorpe Sent: 27 May 2015 18:05 On Wed, May 27, 2015 at 04:41:18PM +, David Laight wrote: The code will be sleeping in kernel_accept() and later calls kernel_getpeername(). The code is used for both TCP and SCTP and this part is common (using the TCP semantics). getpeername uses a different flow, it calls into inet6_getname which will always return the AF_INET6 version. Ok, that explains why I hadn't seen the problem. It also means I don't have to worry about it. David -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
On 05/27/2015 11:06 AM, David Laight wrote: From: Jason Gunthorpe ... Fixes: 299ee123e198 (sctp: Fixup v4mapped behaviour to comply with Sock API) ... This bugfix should be a candidate for -stable Anyone know off-hand which kernel releases are affected? I'm going to have to note this in the release notes for one of our products. Ever heard of git-describe(1) ? ;) git describe 299ee123e19889d511092347f5fc14db0f10e3a6 v3.16-rc7-1525-g299ee12 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
From: Daniel Borkmann Sent: 27 May 2015 10:34 ... Fixes: 299ee123e198 (sctp: Fixup v4mapped behaviour to comply with Sock API) ... This bugfix should be a candidate for -stable Anyone know off-hand which kernel releases are affected? I'm going to have to note this in the release notes for one of our products. Ever heard of git-describe(1) ? ;) git describe 299ee123e19889d511092347f5fc14db0f10e3a6 v3.16-rc7-1525-g299ee12 Probably last time I asked the same question :-) Since I don't spend all day fighting git, I don't know all the sub-commands. In any case it looks like I can escape by turning off SCTP_I_WANT_MAPPED_V4_ADDR for kernels 3.17 through 4.0. David -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
From: Jason Gunthorpe jguntho...@obsidianresearch.com Date: Tue, 26 May 2015 17:30:17 -0600 sctp_v4_map_v6 was subtly writing and reading from members of a union in a way the clobbered data it needed to read before it read it. Zeroing the v6 flowinfo overwrites the v4 sin_addr with 0, meaning that every place that calls sctp_v4_map_v6 gets :::0.0.0.0 as the result. Reorder things to guarantee correct behaviour no matter what the union layout is. This impacts user space clients that open an IPv6 SCTP socket and receive IPv4 connections. Prior to 299ee user space would see a sockaddr with AF_INET and a correct address, after 299ee the sockaddr is AF_INET6, but the address is wrong. Fixes: 299ee123e198 (sctp: Fixup v4mapped behaviour to comply with Sock API) Signed-off-by: Jason Gunthorpe jguntho...@obsidianresearch.com Applied and queued up for -stable, thakns. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
From: Jason Gunthorpe Sent: 27 May 2015 16:32 On Wed, May 27, 2015 at 10:11:22AM +, David Laight wrote: In any case it looks like I can escape by turning off SCTP_I_WANT_MAPPED_V4_ADDR for kernels 3.17 through 4.0. Just be aware that option is unusable on kernels without 299ee. I fixed everything wrong I saw, but that doesn't mean it works 100%. Honestly, I don't think anyone has ever used it. I'm now confused. I've just done a test using a 4.0.0-rc1 kernel. I'm binding an IPv6 listening socket and then connecting to it from 127.0.0.1. I don't know it I'm being given an IPv4 format address or a v6mapped one (I shorten the latter before tracing it) - but it contains 127.0.0.1 (not 0.0.0.0). (That is without changing any socket options.) The kernel code I have seems to default 'v4mapped = 1' which means it should (if I read the code correctly) hit sctp_v4_map_v6(). But I'm not seeing the corruption. Does 127.0.0.1 go through a different path that means the address is already in IPv6 format? Testing without using the loopback address is rather harder (for automated test scripts). (If anyone suggests that network namespaces might help, they can then explain how a single kernel entity is going to choose between different namespaces on an individual connection basis. Think of something that is receiving data on one connection, decoding the requests, re-encapsulation the data in a different protocol and then sending the data onwards.) David -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
On Wed, May 27, 2015 at 10:11:22AM +, David Laight wrote: In any case it looks like I can escape by turning off SCTP_I_WANT_MAPPED_V4_ADDR for kernels 3.17 through 4.0. Just be aware that option is unusable on kernels without 299ee. I fixed everything wrong I saw, but that doesn't mean it works 100%. Honestly, I don't think anyone has ever used it. Jason -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
On Tue, May 26, 2015 at 05:30:17PM -0600, Jason Gunthorpe wrote:
sctp_v4_map_v6 was subtly writing and reading from members
of a union in a way the clobbered data it needed to read before
it read it.
Zeroing the v6 flowinfo overwrites the v4 sin_addr with 0, meaning
that every place that calls sctp_v4_map_v6 gets :::0.0.0.0 as the
result.
Reorder things to guarantee correct behaviour no matter what the
union layout is.
This impacts user space clients that open an IPv6 SCTP socket and
receive IPv4 connections. Prior to 299ee user space would see a
sockaddr with AF_INET and a correct address, after 299ee the sockaddr
is AF_INET6, but the address is wrong.
Fixes: 299ee123e198 (sctp: Fixup v4mapped behaviour to comply with Sock API)
Signed-off-by: Jason Gunthorpe jguntho...@obsidianresearch.com
---
include/net/sctp/sctp.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
This bugfix should be a candidate for -stable
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 856f01cb51dd..230775f5952a 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -571,11 +571,14 @@ static inline void sctp_v6_map_v4(union sctp_addr *addr)
/* Map v4 address to v4-mapped v6 address */
static inline void sctp_v4_map_v6(union sctp_addr *addr)
{
+ __be16 port;
+
+ port = addr-v4.sin_port;
+ addr-v6.sin6_addr.s6_addr32[3] = addr-v4.sin_addr.s_addr;
+ addr-v6.sin6_port = port;
addr-v6.sin6_family = AF_INET6;
addr-v6.sin6_flowinfo = 0;
addr-v6.sin6_scope_id = 0;
- addr-v6.sin6_port = addr-v4.sin_port;
- addr-v6.sin6_addr.s6_addr32[3] = addr-v4.sin_addr.s_addr;
addr-v6.sin6_addr.s6_addr32[0] = 0;
addr-v6.sin6_addr.s6_addr32[1] = 0;
addr-v6.sin6_addr.s6_addr32[2] = htonl(0x);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-sctp in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Nice catch
Acked-by: Neil Horman nhor...@tuxdriver.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
From: Jason Gunthorpe Sent: 27 May 2015 17:32 On Wed, May 27, 2015 at 04:16:44PM +, David Laight wrote: From: Jason Gunthorpe Sent: 27 May 2015 16:32 On Wed, May 27, 2015 at 10:11:22AM +, David Laight wrote: In any case it looks like I can escape by turning off SCTP_I_WANT_MAPPED_V4_ADDR for kernels 3.17 through 4.0. Just be aware that option is unusable on kernels without 299ee. I fixed everything wrong I saw, but that doesn't mean it works 100%. Honestly, I don't think anyone has ever used it. I'm now confused. I've just done a test using a 4.0.0-rc1 kernel. I'm binding an IPv6 listening socket and then connecting to it from 127.0.0.1. I don't know it I'm being given an IPv4 format address or a v6mapped one (I shorten the latter before tracing it) - but it contains 127.0.0.1 (not 0.0.0.0). (That is without changing any socket options.) I don't know what your test does, but I used the same basic idea with loopback to find this issue. You should confirm the kernel is returning a AF_INET6 socket type, if it is AF_INET then there is a path I missed in 299ee and I should fix it.. The code will be sleeping in kernel_accept() and later calls kernel_getpeername(). The code is used for both TCP and SCTP and this part is common (using the TCP semantics). I'll look at the actual raw address format tomorrow. I suspect it is already AF_INET6 before getting to the code that would badly translate it. David -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
On Wed, May 27, 2015 at 04:41:18PM +, David Laight wrote: The code will be sleeping in kernel_accept() and later calls kernel_getpeername(). The code is used for both TCP and SCTP and this part is common (using the TCP semantics). getpeername uses a different flow, it calls into inet6_getname which will always return the AF_INET6 version. The call to sctp_v6_addr_to_user after is to support the v6-v4 coversion when SCTP_I_WANT_MAPPED_V4_ADDR=0, it will never do the broken v4-v6 conversion. Jason -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
On Wed, May 27, 2015 at 04:16:44PM +, David Laight wrote:
From: Jason Gunthorpe
Sent: 27 May 2015 16:32
On Wed, May 27, 2015 at 10:11:22AM +, David Laight wrote:
In any case it looks like I can escape by turning off
SCTP_I_WANT_MAPPED_V4_ADDR for kernels 3.17 through 4.0.
Just be aware that option is unusable on kernels without 299ee.
I fixed everything wrong I saw, but that doesn't mean it works
100%. Honestly, I don't think anyone has ever used it.
I'm now confused.
I've just done a test using a 4.0.0-rc1 kernel.
I'm binding an IPv6 listening socket and then connecting to it
from 127.0.0.1.
I don't know it I'm being given an IPv4 format address or a
v6mapped one (I shorten the latter before tracing it) - but
it contains 127.0.0.1 (not 0.0.0.0).
(That is without changing any socket options.)
I don't know what your test does, but I used the same basic idea with
loopback to find this issue. You should confirm the kernel is
returning a AF_INET6 socket type, if it is AF_INET then there is a
path I missed in 299ee and I should fix it..
Specifically, the corruption I confirmed was from a recvmsg call with
MSG_NOTIFICATION set indicating a new connection has happened on a
many to many socket.
strace sayth:
socket(PF_INET6, SOCK_SEQPACKET|SOCK_CLOEXEC, IPPROTO_SCTP) = 7
recvmsg(7, {msg_name(28)={sa_family=AF_INET6, sin6_port=htons(9090),
inet_pton(AF_INET6, :::0.0.0.0, sin6_addr), sin6_flowinfo=0,
sin6_scope_id=0}, msg_iov(1)=[{\1\200\0\0\24\0\0\0\4\0\0\0\0\0\0\0\17%\0\0,
1024}], msg_controllen=0, msg_flags=MSG_EOR|MSG_MORE}, MSG_DONTWAIT) = 20
Jason
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket
On 05/27/2015 01:30 AM, Jason Gunthorpe wrote:
sctp_v4_map_v6 was subtly writing and reading from members
of a union in a way the clobbered data it needed to read before
s/the/that/
it read it.
Zeroing the v6 flowinfo overwrites the v4 sin_addr with 0, meaning
that every place that calls sctp_v4_map_v6 gets :::0.0.0.0 as the
result.
Reorder things to guarantee correct behaviour no matter what the
union layout is.
This impacts user space clients that open an IPv6 SCTP socket and
receive IPv4 connections. Prior to 299ee user space would see a
sockaddr with AF_INET and a correct address, after 299ee the sockaddr
is AF_INET6, but the address is wrong.
Fixes: 299ee123e198 (sctp: Fixup v4mapped behaviour to comply with Sock API)
Signed-off-by: Jason Gunthorpe jguntho...@obsidianresearch.com
---
include/net/sctp/sctp.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
This bugfix should be a candidate for -stable
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 856f01cb51dd..230775f5952a 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -571,11 +571,14 @@ static inline void sctp_v6_map_v4(union sctp_addr *addr)
/* Map v4 address to v4-mapped v6 address */
static inline void sctp_v4_map_v6(union sctp_addr *addr)
{
+ __be16 port;
+
+ port = addr-v4.sin_port;
+ addr-v6.sin6_addr.s6_addr32[3] = addr-v4.sin_addr.s_addr;
+ addr-v6.sin6_port = port;
addr-v6.sin6_family = AF_INET6;
addr-v6.sin6_flowinfo = 0;
addr-v6.sin6_scope_id = 0;
- addr-v6.sin6_port = addr-v4.sin_port;
- addr-v6.sin6_addr.s6_addr32[3] = addr-v4.sin_addr.s_addr;
Change looks good to me. You actually would only need to address
the issue where the v4.sin_addr is copied before you overwrite/zero
the flowinfo as only these two overlap in the union. Given that
sockaddr_in and sockaddr_in6 are exported to uapi, they are immutable,
but I see the point that union sctp_addr is kernel private - bad
things happen if v4/v6 don't overlap the way anymore as they'd do
now. ;)
Anyways:
Acked-by: Daniel Borkmann dan...@iogearbox.net
addr-v6.sin6_addr.s6_addr32[0] = 0;
addr-v6.sin6_addr.s6_addr32[1] = 0;
addr-v6.sin6_addr.s6_addr32[2] = htonl(0x);
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
