Thread (2 messages) 2 messages, 2 authors, 2018-11-21

Re: [PATCH net] sctp: hold transport before accessing its asoc in sctp_epaddr_lookup_transport

From: Neil Horman <nhorman@tuxdriver.com>
Date: 2018-11-21 00:29:38
Also in: linux-sctp

On Tue, Nov 20, 2018 at 07:12:10PM +0800, Xin Long wrote:
quoted hunk ↗ jump to hunk
Without holding transport to dereference its asoc, a use after
free panic can be caused in sctp_epaddr_lookup_transport. Note
that a sock lock can't protect these transports that belong to
other socks.

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_epaddr_lookup_transport.

Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/input.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 69584e9..c2c0816 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -972,9 +972,15 @@ struct sctp_transport *sctp_epaddr_lookup_transport(
 	list = rhltable_lookup(&sctp_transport_hashtable, &arg,
 			       sctp_hash_params);
 
-	rhl_for_each_entry_rcu(t, tmp, list, node)
-		if (ep == t->asoc->ep)
+	rhl_for_each_entry_rcu(t, tmp, list, node) {
+		if (!sctp_transport_hold(t))
+			continue;
+		if (ep == t->asoc->ep) {
+			sctp_transport_put(t);
 			return t;
+		}
+		sctp_transport_put(t);
+	}
 
 	return NULL;
 }
-- 
2.1.0
Same as the other patch, it seems like we shouldn't need to hold the transport
here as long as we either have a hold on the parent association already (which
we should), and or, properly quiesce the destruction of an assocation.

Neil
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help