From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:58996 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727716AbeIMSoa (ORCPT ); Thu, 13 Sep 2018 14:44:30 -0400 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, syzbot+fe62a0c9aa6a85c6de16@syzkaller.appspotmail.com, Xin Long , Neil Horman , Marcelo Ricardo Leitner , "David S. Miller" Subject: [PATCH 4.9 10/78] sctp: hold transport before accessing its asoc in sctp_transport_get_next Date: Thu, 13 Sep 2018 15:30:57 +0200 Message-Id: <20180913131806.402048634@linuxfoundation.org> In-Reply-To: <20180913131805.732342940@linuxfoundation.org> References: <20180913131805.732342940@linuxfoundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: 4.9-stable review patch. If anyone has any objections, please let me know. ------------------ From: Xin Long [ 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 Acked-by: Neil Horman Acked-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- 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 @@ -337,8 +337,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; @@ -428,8 +426,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 @@ -4476,9 +4476,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; @@ -4488,13 +4493,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 *), @@ -4556,8 +4566,6 @@ int sctp_for_each_transport(int (*cb)(st for (; !IS_ERR_OR_NULL(obj); obj = sctp_transport_get_next(net, &hti)) { struct sctp_transport *transport = obj; - if (!sctp_transport_hold(transport)) - continue; err = cb(transport, p); sctp_transport_put(transport); if (err)