From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner Subject: Re: [PATCHv2 net] sctp: hold transport before accessing its asoc in sctp_hash_transport Date: Fri, 30 Nov 2018 10:05:15 -0200 Message-ID: <20181130120515.GM3601@localhost.localdomain> References: <92d9c2b54a60494142909f1fbb8c187aa317ab60.1543474168.git.lucien.xin@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: network dev , linux-sctp@vger.kernel.org, davem@davemloft.net, Neil Horman To: Xin Long Return-path: Received: from mail-qt1-f196.google.com ([209.85.160.196]:44497 "EHLO mail-qt1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726555AbeK3XOX (ORCPT ); Fri, 30 Nov 2018 18:14:23 -0500 Content-Disposition: inline In-Reply-To: <92d9c2b54a60494142909f1fbb8c187aa317ab60.1543474168.git.lucien.xin@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Nov 29, 2018 at 02:49:28PM +0800, Xin Long wrote: > In sctp_hash_transport, it dereferences a transport's asoc only under > rcu_read_lock. Without holding the transport, its asoc could be freed > already, which leads to a use-after-free panic. > > A similar fix as Commit bab1be79a516 ("sctp: hold transport before > accessing its asoc in sctp_transport_get_next") is needed to hold > the transport before accessing its asoc in sctp_hash_transport. > > Note that as rhlist keeps the lists to a small size, this extra > atomic operation won't cause a noticeable latency on inserting > a transport. Yet it's not in a datapath. > > v1->v2: > - improve the changelog. > > Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport") > Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com > Signed-off-by: Xin Long Acked-by: Marcelo Ricardo Leitner > --- > net/sctp/input.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/net/sctp/input.c b/net/sctp/input.c > index ce7351c..c2c0816 100644 > --- a/net/sctp/input.c > +++ b/net/sctp/input.c > @@ -896,11 +896,16 @@ int sctp_hash_transport(struct sctp_transport *t) > list = rhltable_lookup(&sctp_transport_hashtable, &arg, > sctp_hash_params); > > - rhl_for_each_entry_rcu(transport, tmp, list, node) > + rhl_for_each_entry_rcu(transport, tmp, list, node) { > + if (!sctp_transport_hold(transport)) > + continue; > if (transport->asoc->ep == t->asoc->ep) { > + sctp_transport_put(transport); > rcu_read_unlock(); > return -EEXIST; > } > + sctp_transport_put(transport); > + } > rcu_read_unlock(); > > err = rhltable_insert_key(&sctp_transport_hashtable, &arg, > -- > 2.1.0 >