All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Jin Meng <meng.a.jin@nokia-sbell.com>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	Xin Long <lucien.xin@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.9 07/32] sctp: fix possibly using a bad saddr with a given dst
Date: Sat, 11 Apr 2020 14:08:46 +0200	[thread overview]
Message-ID: <20200411115419.504610902@linuxfoundation.org> (raw)
In-Reply-To: <20200411115418.455500023@linuxfoundation.org>

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

[ Upstream commit 582eea230536a6f104097dd46205822005d5fe3a ]

Under certain circumstances, depending on the order of addresses on the
interfaces, it could be that sctp_v[46]_get_dst() would return a dst
with a mismatched struct flowi.

For example, if when walking through the bind addresses and the first
one is not a match, it saves the dst as a fallback (added in
410f03831c07), but not the flowi. Then if the next one is also not a
match, the previous dst will be returned but with the flowi information
for the 2nd address, which is wrong.

The fix is to use a locally stored flowi that can be used for such
attempts, and copy it to the parameter only in case it is a possible
match, together with the corresponding dst entry.

The patch updates IPv6 code mostly just to be in sync. Even though the issue
is also present there, it fallback is not expected to work with IPv6.

Fixes: 410f03831c07 ("sctp: add routing output fallback")
Reported-by: Jin Meng <meng.a.jin@nokia-sbell.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Tested-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/sctp/ipv6.c     |   20 ++++++++++++++------
 net/sctp/protocol.c |   28 +++++++++++++++++++---------
 2 files changed, 33 insertions(+), 15 deletions(-)

--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -235,7 +235,8 @@ static void sctp_v6_get_dst(struct sctp_
 {
 	struct sctp_association *asoc = t->asoc;
 	struct dst_entry *dst = NULL;
-	struct flowi6 *fl6 = &fl->u.ip6;
+	struct flowi _fl;
+	struct flowi6 *fl6 = &_fl.u.ip6;
 	struct sctp_bind_addr *bp;
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sctp_sockaddr_entry *laddr;
@@ -245,7 +246,7 @@ static void sctp_v6_get_dst(struct sctp_
 	__u8 matchlen = 0;
 	sctp_scope_t scope;
 
-	memset(fl6, 0, sizeof(struct flowi6));
+	memset(&_fl, 0, sizeof(_fl));
 	fl6->daddr = daddr->v6.sin6_addr;
 	fl6->fl6_dport = daddr->v6.sin6_port;
 	fl6->flowi6_proto = IPPROTO_SCTP;
@@ -269,8 +270,11 @@ static void sctp_v6_get_dst(struct sctp_
 	rcu_read_unlock();
 
 	dst = ip6_dst_lookup_flow(sk, fl6, final_p);
-	if (!asoc || saddr)
+	if (!asoc || saddr) {
+		t->dst = dst;
+		memcpy(fl, &_fl, sizeof(_fl));
 		goto out;
+	}
 
 	bp = &asoc->base.bind_addr;
 	scope = sctp_scope(daddr);
@@ -293,6 +297,8 @@ static void sctp_v6_get_dst(struct sctp_
 			if ((laddr->a.sa.sa_family == AF_INET6) &&
 			    (sctp_v6_cmp_addr(&dst_saddr, &laddr->a))) {
 				rcu_read_unlock();
+				t->dst = dst;
+				memcpy(fl, &_fl, sizeof(_fl));
 				goto out;
 			}
 		}
@@ -331,6 +337,8 @@ static void sctp_v6_get_dst(struct sctp_
 			if (!IS_ERR_OR_NULL(dst))
 				dst_release(dst);
 			dst = bdst;
+			t->dst = dst;
+			memcpy(fl, &_fl, sizeof(_fl));
 			break;
 		}
 
@@ -344,6 +352,8 @@ static void sctp_v6_get_dst(struct sctp_
 			dst_release(dst);
 		dst = bdst;
 		matchlen = bmatchlen;
+		t->dst = dst;
+		memcpy(fl, &_fl, sizeof(_fl));
 	}
 	rcu_read_unlock();
 
@@ -352,14 +362,12 @@ out:
 		struct rt6_info *rt;
 
 		rt = (struct rt6_info *)dst;
-		t->dst = dst;
 		t->dst_cookie = rt6_get_cookie(rt);
 		pr_debug("rt6_dst:%pI6/%d rt6_src:%pI6\n",
 			 &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
-			 &fl6->saddr);
+			 &fl->u.ip6.saddr);
 	} else {
 		t->dst = NULL;
-
 		pr_debug("no route\n");
 	}
 }
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -430,14 +430,15 @@ static void sctp_v4_get_dst(struct sctp_
 {
 	struct sctp_association *asoc = t->asoc;
 	struct rtable *rt;
-	struct flowi4 *fl4 = &fl->u.ip4;
+	struct flowi _fl;
+	struct flowi4 *fl4 = &_fl.u.ip4;
 	struct sctp_bind_addr *bp;
 	struct sctp_sockaddr_entry *laddr;
 	struct dst_entry *dst = NULL;
 	union sctp_addr *daddr = &t->ipaddr;
 	union sctp_addr dst_saddr;
 
-	memset(fl4, 0x0, sizeof(struct flowi4));
+	memset(&_fl, 0x0, sizeof(_fl));
 	fl4->daddr  = daddr->v4.sin_addr.s_addr;
 	fl4->fl4_dport = daddr->v4.sin_port;
 	fl4->flowi4_proto = IPPROTO_SCTP;
@@ -455,8 +456,11 @@ static void sctp_v4_get_dst(struct sctp_
 		 &fl4->saddr);
 
 	rt = ip_route_output_key(sock_net(sk), fl4);
-	if (!IS_ERR(rt))
+	if (!IS_ERR(rt)) {
 		dst = &rt->dst;
+		t->dst = dst;
+		memcpy(fl, &_fl, sizeof(_fl));
+	}
 
 	/* If there is no association or if a source address is passed, no
 	 * more validation is required.
@@ -519,27 +523,33 @@ static void sctp_v4_get_dst(struct sctp_
 		odev = __ip_dev_find(sock_net(sk), laddr->a.v4.sin_addr.s_addr,
 				     false);
 		if (!odev || odev->ifindex != fl4->flowi4_oif) {
-			if (!dst)
+			if (!dst) {
 				dst = &rt->dst;
-			else
+				t->dst = dst;
+				memcpy(fl, &_fl, sizeof(_fl));
+			} else {
 				dst_release(&rt->dst);
+			}
 			continue;
 		}
 
 		dst_release(dst);
 		dst = &rt->dst;
+		t->dst = dst;
+		memcpy(fl, &_fl, sizeof(_fl));
 		break;
 	}
 
 out_unlock:
 	rcu_read_unlock();
 out:
-	t->dst = dst;
-	if (dst)
+	if (dst) {
 		pr_debug("rt_dst:%pI4, rt_src:%pI4\n",
-			 &fl4->daddr, &fl4->saddr);
-	else
+			 &fl->u.ip4.daddr, &fl->u.ip4.saddr);
+	} else {
+		t->dst = NULL;
 		pr_debug("no route\n");
+	}
 }
 
 /* For v4, the source address is cached in the route entry(dst). So no need



  parent reply	other threads:[~2020-04-11 12:28 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-11 12:08 [PATCH 4.9 00/32] 4.9.219-rc1 review Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 01/32] l2tp: ensure sessions are freed after their PPPOL2TP socket Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 02/32] l2tp: fix race between l2tp_session_delete() and l2tp_tunnel_closeall() Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 03/32] drm/bochs: downgrade pci_request_region failure from error to warning Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 04/32] ipv4: fix a RCU-list lock in fib_triestat_seq_show Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 05/32] net, ip_tunnel: fix interface lookup with no key Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 06/32] sctp: fix refcount bug in sctp_wfree Greg Kroah-Hartman
2020-04-11 12:08 ` Greg Kroah-Hartman [this message]
2020-04-11 12:08 ` [PATCH 4.9 08/32] drm/etnaviv: replace MMU flush marker with flush sequence Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 09/32] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 10/32] blk-mq: Allow blocking queue tag iter callbacks Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 11/32] coresight: do not use the BIT() macro in the UAPI header Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 12/32] net: dsa: tag_brcm: Fix skb->fwd_offload_mark location Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 13/32] padata: always acquire cpu_hotplug_lock before pinst->lock Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 14/32] mm: mempolicy: require at least one nodeid for MPOL_PREFERRED Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 15/32] net: dsa: bcm_sf2: Ensure correct sub-node is parsed Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 16/32] net: stmmac: dwmac1000: fix out-of-bounds mac address reg setting Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 17/32] slcan: Dont transmit uninitialized stack data in padding Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 18/32] net: phy: micrel: kszphy_resume(): add delay after genphy_resume() before accessing PHY registers Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 19/32] random: always use batched entropy for get_random_u{32,64} Greg Kroah-Hartman
2020-04-11 12:08 ` [PATCH 4.9 20/32] tools/accounting/getdelays.c: fix netlink attribute length Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 21/32] ASoC: jz4740-i2s: Fix divider written at incorrect offset in register Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 22/32] IB/hfi1: Call kobject_put() when kobject_init_and_add() fails Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 23/32] IB/hfi1: Fix memory leaks in sysfs registration and unregistration Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 24/32] ceph: remove the extra slashes in the server path Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 25/32] ceph: canonicalize server path in place Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 26/32] Bluetooth: RFCOMM: fix ODEBUG bug in rfcomm_dev_ioctl Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 27/32] RDMA/cm: Update num_paths in cma_resolve_iboe_route error flow Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 28/32] clk: qcom: rcg: Return failure for RCG update Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 29/32] drm/msm: stop abusing dma_map/unmap for cache Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 30/32] arm64: Fix size of __early_cpu_boot_status Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 31/32] usb: dwc3: dont set gadget->is_otg flag Greg Kroah-Hartman
2020-04-11 12:09 ` [PATCH 4.9 32/32] drm_dp_mst_topology: fix broken drm_dp_sideband_parse_remote_dpcd_read() Greg Kroah-Hartman
2020-04-11 20:38 ` [PATCH 4.9 00/32] 4.9.219-rc1 review Guenter Roeck
2020-04-12  9:58 ` Naresh Kamboju
2020-04-14 10:35 ` Jon Hunter
2020-04-14 10:35   ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200411115419.504610902@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=marcelo.leitner@gmail.com \
    --cc=meng.a.jin@nokia-sbell.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.