Commit bab1be79 authored by Xin Long's avatar Xin Long Committed by David S. Miller

sctp: hold transport before accessing its asoc in sctp_transport_get_next

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: 626d16f5 ("sctp: export some apis or variables for sctp_diag and reuse some for proc")
Reported-by: syzbot+fe62a0c9aa6a85c6de16@syzkaller.appspotmail.com
Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
Acked-by: default avatarNeil Horman <nhorman@tuxdriver.com>
Acked-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 050cdc6c
...@@ -264,8 +264,6 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v) ...@@ -264,8 +264,6 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v)
} }
transport = (struct sctp_transport *)v; transport = (struct sctp_transport *)v;
if (!sctp_transport_hold(transport))
return 0;
assoc = transport->asoc; assoc = transport->asoc;
epb = &assoc->base; epb = &assoc->base;
sk = epb->sk; sk = epb->sk;
...@@ -322,8 +320,6 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) ...@@ -322,8 +320,6 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
} }
transport = (struct sctp_transport *)v; transport = (struct sctp_transport *)v;
if (!sctp_transport_hold(transport))
return 0;
assoc = transport->asoc; assoc = transport->asoc;
list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list, list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list,
......
...@@ -5005,9 +5005,14 @@ struct sctp_transport *sctp_transport_get_next(struct net *net, ...@@ -5005,9 +5005,14 @@ struct sctp_transport *sctp_transport_get_next(struct net *net,
break; break;
} }
if (!sctp_transport_hold(t))
continue;
if (net_eq(sock_net(t->asoc->base.sk), net) && if (net_eq(sock_net(t->asoc->base.sk), net) &&
t->asoc->peer.primary_path == t) t->asoc->peer.primary_path == t)
break; break;
sctp_transport_put(t);
} }
return t; return t;
...@@ -5017,13 +5022,18 @@ struct sctp_transport *sctp_transport_get_idx(struct net *net, ...@@ -5017,13 +5022,18 @@ struct sctp_transport *sctp_transport_get_idx(struct net *net,
struct rhashtable_iter *iter, struct rhashtable_iter *iter,
int pos) int pos)
{ {
void *obj = SEQ_START_TOKEN; struct sctp_transport *t;
while (pos && (obj = sctp_transport_get_next(net, iter)) && if (!pos)
!IS_ERR(obj)) return SEQ_START_TOKEN;
pos--;
return obj; while ((t = sctp_transport_get_next(net, iter)) && !IS_ERR(t)) {
if (!--pos)
break;
sctp_transport_put(t);
}
return t;
} }
int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *),
...@@ -5082,8 +5092,6 @@ int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *), ...@@ -5082,8 +5092,6 @@ int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
tsp = sctp_transport_get_idx(net, &hti, *pos + 1); tsp = sctp_transport_get_idx(net, &hti, *pos + 1);
for (; !IS_ERR_OR_NULL(tsp); tsp = sctp_transport_get_next(net, &hti)) { for (; !IS_ERR_OR_NULL(tsp); tsp = sctp_transport_get_next(net, &hti)) {
if (!sctp_transport_hold(tsp))
continue;
ret = cb(tsp, p); ret = cb(tsp, p);
if (ret) if (ret)
break; break;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment