Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

sctp: hold transport instead of assoc when lookup assoc in rx path

Prior to this patch, in rx path, before calling lock_sock, it needed to
hold assoc when got it by __sctp_lookup_association, in case other place
would free/put assoc.

But in __sctp_lookup_association, it lookup and hold transport, then got
assoc by transport->assoc, then hold assoc and put transport. It means
it didn't hold transport, yet it was returned and later on directly
assigned to chunk->transport.

Without the protection of sock lock, the transport may be freed/put by
other places, which would cause a use-after-free issue.

This patch is to fix this issue by holding transport instead of assoc.
As holding transport can make sure to access assoc is also safe, and
actually it looks up assoc by searching transport rhashtable, to hold
transport here makes more sense.

Note that the function will be renamed later on on another patch.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Xin Long and committed by
David S. Miller
dae399d7 7c17fcc7

+18 -18
+1 -1
include/net/sctp/sctp.h
··· 152 152 struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *, 153 153 struct sctphdr *, struct sctp_association **, 154 154 struct sctp_transport **); 155 - void sctp_err_finish(struct sock *, struct sctp_association *); 155 + void sctp_err_finish(struct sock *, struct sctp_transport *); 156 156 void sctp_icmp_frag_needed(struct sock *, struct sctp_association *, 157 157 struct sctp_transport *t, __u32 pmtu); 158 158 void sctp_icmp_redirect(struct sock *, struct sctp_transport *,
+16 -16
net/sctp/input.c
··· 181 181 * bound to another interface, via SO_BINDTODEVICE, treat it as OOTB 182 182 */ 183 183 if (sk->sk_bound_dev_if && (sk->sk_bound_dev_if != af->skb_iif(skb))) { 184 - if (asoc) { 185 - sctp_association_put(asoc); 184 + if (transport) { 185 + sctp_transport_put(transport); 186 186 asoc = NULL; 187 + transport = NULL; 187 188 } else { 188 189 sctp_endpoint_put(ep); 189 190 ep = NULL; ··· 270 269 bh_unlock_sock(sk); 271 270 272 271 /* Release the asoc/ep ref we took in the lookup calls. */ 273 - if (asoc) 274 - sctp_association_put(asoc); 272 + if (transport) 273 + sctp_transport_put(transport); 275 274 else 276 275 sctp_endpoint_put(ep); 277 276 ··· 284 283 285 284 discard_release: 286 285 /* Release the asoc/ep ref we took in the lookup calls. */ 287 - if (asoc) 288 - sctp_association_put(asoc); 286 + if (transport) 287 + sctp_transport_put(transport); 289 288 else 290 289 sctp_endpoint_put(ep); 291 290 ··· 301 300 { 302 301 struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk; 303 302 struct sctp_inq *inqueue = &chunk->rcvr->inqueue; 303 + struct sctp_transport *t = chunk->transport; 304 304 struct sctp_ep_common *rcvr = NULL; 305 305 int backloged = 0; 306 306 ··· 353 351 done: 354 352 /* Release the refs we took in sctp_add_backlog */ 355 353 if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type) 356 - sctp_association_put(sctp_assoc(rcvr)); 354 + sctp_transport_put(t); 357 355 else if (SCTP_EP_TYPE_SOCKET == rcvr->type) 358 356 sctp_endpoint_put(sctp_ep(rcvr)); 359 357 else ··· 365 363 static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb) 366 364 { 367 365 struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk; 366 + struct sctp_transport *t = chunk->transport; 368 367 struct sctp_ep_common *rcvr = chunk->rcvr; 369 368 int ret; 370 369 ··· 376 373 * from us 377 374 */ 378 375 if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type) 379 - sctp_association_hold(sctp_assoc(rcvr)); 376 + sctp_transport_hold(t); 380 377 else if (SCTP_EP_TYPE_SOCKET == rcvr->type) 381 378 sctp_endpoint_hold(sctp_ep(rcvr)); 382 379 else ··· 540 537 return sk; 541 538 542 539 out: 543 - sctp_association_put(asoc); 540 + sctp_transport_put(transport); 544 541 return NULL; 545 542 } 546 543 547 544 /* Common cleanup code for icmp/icmpv6 error handler. */ 548 - void sctp_err_finish(struct sock *sk, struct sctp_association *asoc) 545 + void sctp_err_finish(struct sock *sk, struct sctp_transport *t) 549 546 { 550 547 bh_unlock_sock(sk); 551 - sctp_association_put(asoc); 548 + sctp_transport_put(t); 552 549 } 553 550 554 551 /* ··· 644 641 } 645 642 646 643 out_unlock: 647 - sctp_err_finish(sk, asoc); 644 + sctp_err_finish(sk, transport); 648 645 } 649 646 650 647 /* ··· 955 952 goto out; 956 953 957 954 asoc = t->asoc; 958 - sctp_association_hold(asoc); 959 955 *pt = t; 960 - 961 - sctp_transport_put(t); 962 956 963 957 out: 964 958 return asoc; ··· 986 986 struct sctp_transport *transport; 987 987 988 988 if ((asoc = sctp_lookup_association(net, laddr, paddr, &transport))) { 989 - sctp_association_put(asoc); 989 + sctp_transport_put(transport); 990 990 return 1; 991 991 } 992 992
+1 -1
net/sctp/ipv6.c
··· 198 198 } 199 199 200 200 out_unlock: 201 - sctp_err_finish(sk, asoc); 201 + sctp_err_finish(sk, transport); 202 202 out: 203 203 if (likely(idev != NULL)) 204 204 in6_dev_put(idev);