Re: [PATCH] sctp: Fix mangled IPv4 addresses on a IPv6 listening socket

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 ::ffff: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 <jgunthorpe@xxxxxxxxxxxxxxxxxxxx>
---
  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 <daniel@xxxxxxxxxxxxx>

  	addr->v6.sin6_addr.s6_addr32[0] = 0;
  	addr->v6.sin6_addr.s6_addr32[1] = 0;
  	addr->v6.sin6_addr.s6_addr32[2] = htonl(0x0000ffff);


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




[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux