From bab1be79a5169ac748d8292b20c86d874022d7ba Mon Sep 17 00:00:00 2001 From: Xin Long Date: Mon, 27 Aug 2018 18:38:31 +0800 Subject: 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: 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 Acked-by: Neil Horman Acked-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sctp/proc.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'net/sctp/proc.c') diff --git a/net/sctp/proc.c b/net/sctp/proc.c index ef5c9a82d4e8..4d6f1c8d6659 100644 --- a/net/sctp/proc.c +++ b/net/sctp/proc.c @@ -264,8 +264,6 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v) } transport = (struct sctp_transport *)v; - if (!sctp_transport_hold(transport)) - return 0; assoc = transport->asoc; epb = &assoc->base; sk = epb->sk; @@ -322,8 +320,6 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) } 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, -- cgit From 834539e69a5fe2aab33cc777ccfd4a4fcc5b9770 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Mon, 27 Aug 2018 18:40:18 +0800 Subject: sctp: remove useless start_fail from sctp_ht_iter in proc After changing rhashtable_walk_start to return void, start_fail would never be set other value than 0, and the checking for start_fail is pointless, so remove it. Fixes: 97a6ec4ac021 ("rhashtable: Change rhashtable_walk_start to return void") Signed-off-by: Xin Long Acked-by: Neil Horman Acked-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sctp/proc.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'net/sctp/proc.c') diff --git a/net/sctp/proc.c b/net/sctp/proc.c index 4d6f1c8d6659..a644292f9faf 100644 --- a/net/sctp/proc.c +++ b/net/sctp/proc.c @@ -215,7 +215,6 @@ static const struct seq_operations sctp_eps_ops = { struct sctp_ht_iter { struct seq_net_private p; struct rhashtable_iter hti; - int start_fail; }; static void *sctp_transport_seq_start(struct seq_file *seq, loff_t *pos) @@ -224,7 +223,6 @@ static void *sctp_transport_seq_start(struct seq_file *seq, loff_t *pos) sctp_transport_walk_start(&iter->hti); - iter->start_fail = 0; return sctp_transport_get_idx(seq_file_net(seq), &iter->hti, *pos); } @@ -232,8 +230,6 @@ static void sctp_transport_seq_stop(struct seq_file *seq, void *v) { struct sctp_ht_iter *iter = seq->private; - if (iter->start_fail) - return; sctp_transport_walk_stop(&iter->hti); } -- cgit