Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 36bf8bc5 authored by Xin Long's avatar Xin Long Committed by Greg Kroah-Hartman
Browse files

sctp: hold transport before accessing its asoc in sctp_transport_get_next 



[ 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: 626d16f5 ("sctp: export some apis or variables for sctp_diag and reuse some for proc")
Reported-by: default avatar <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>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent d669bd62

net/sctp/proc.c

+0 −4
Original line number Original line Diff line number Diff line
@@ -337,8 +337,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;
@@ -428,8 +426,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,

net/sctp/socket.c

+15 −7
Original line number Original line Diff line number Diff line
@@ -4476,9 +4476,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;
@@ -4488,13 +4493,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 *),
@@ -4556,8 +4566,6 @@ int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *), 
	for (; !IS_ERR_OR_NULL(obj); obj = sctp_transport_get_next(net, &hti)) {

	for (; !IS_ERR_OR_NULL(obj); obj = sctp_transport_get_next(net, &hti)) {

		struct sctp_transport *transport = obj;

		struct sctp_transport *transport = obj;





		if (!sctp_transport_hold(transport))

			continue;

		err = cb(transport, p);

		err = cb(transport, p);

		sctp_transport_put(transport);

		sctp_transport_put(transport);

		if (err)

		if (err)