lkml.org 
[lkml]   [2018]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 4.18 033/197] sctp: hold transport before accessing its asoc in sctp_transport_get_next
Date
4.18-stable review patch.  If anyone has any objections, please let me know.

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

From: Xin Long <lucien.xin@gmail.com>

[ Upstream commit bab1be79a5169ac748d8292b20c86d874022d7ba ]

As Marcelo noticed, in sctp_transport_get_next, it is iterating over
transports but then also accessing the association directly, without
checking any refcnts before that, which can cause an use-after-free
Read.

So fix it by holding transport before accessing the association. With
that, sctp_transport_hold calls can be removed in the later places.

Fixes: 626d16f50f39 ("sctp: export some apis or variables for sctp_diag and reuse some for proc")
Reported-by: syzbot+fe62a0c9aa6a85c6de16@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/sctp/proc.c | 4 ----
net/sctp/socket.c | 22 +++++++++++++++-------
2 files changed, 15 insertions(+), 11 deletions(-)

--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -260,8 +260,6 @@ static int sctp_assocs_seq_show(struct s
}

transport = (struct sctp_transport *)v;
- if (!sctp_transport_hold(transport))
- return 0;
assoc = transport->asoc;
epb = &assoc->base;
sk = epb->sk;
@@ -318,8 +316,6 @@ static int sctp_remaddr_seq_show(struct
}

transport = (struct sctp_transport *)v;
- if (!sctp_transport_hold(transport))
- return 0;
assoc = transport->asoc;

list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list,
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4881,9 +4881,14 @@ struct sctp_transport *sctp_transport_ge
break;
}

+ if (!sctp_transport_hold(t))
+ continue;
+
if (net_eq(sock_net(t->asoc->base.sk), net) &&
t->asoc->peer.primary_path == t)
break;
+
+ sctp_transport_put(t);
}

return t;
@@ -4893,13 +4898,18 @@ struct sctp_transport *sctp_transport_ge
struct rhashtable_iter *iter,
int pos)
{
- void *obj = SEQ_START_TOKEN;
+ struct sctp_transport *t;
+
+ if (!pos)
+ return SEQ_START_TOKEN;

- while (pos && (obj = sctp_transport_get_next(net, iter)) &&
- !IS_ERR(obj))
- pos--;
+ while ((t = sctp_transport_get_next(net, iter)) && !IS_ERR(t)) {
+ if (!--pos)
+ break;
+ sctp_transport_put(t);
+ }

- return obj;
+ return t;
}

int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *),
@@ -4958,8 +4968,6 @@ again:

tsp = sctp_transport_get_idx(net, &hti, *pos + 1);
for (; !IS_ERR_OR_NULL(tsp); tsp = sctp_transport_get_next(net, &hti)) {
- if (!sctp_transport_hold(tsp))
- continue;
ret = cb(tsp, p);
if (ret)
break;

\
 
 \ /
  Last update: 2018-09-13 16:11    [W:0.540 / U:3.932 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site