From eabf786dc9c015d4ee05338eb165c81f518e9074 Mon Sep 17 00:00:00 2001 From: tuexen Date: Sun, 14 Jul 2019 12:04:39 +0000 Subject: [PATCH] When calling sctp_initialize_auth_params(), the inp must have at least a read lock. To avoid more complex locking dances, just call it in sctp_aloc_assoc() when the write lock is still held. Reported by: syzbot+08a486f7e6966f1c3cfb@syzkaller.appspotmail.com MFC after: 1 week --- sys/netinet/sctp_input.c | 4 ++-- sys/netinet/sctp_output.c | 6 ++---- sys/netinet/sctp_pcb.c | 7 +++++-- sys/netinet/sctp_pcb.h | 6 +++++- sys/netinet/sctp_usrreq.c | 13 ++++--------- sys/netinet6/sctp6_usrreq.c | 7 ++----- 6 files changed, 20 insertions(+), 23 deletions(-) diff --git a/sys/netinet/sctp_input.c b/sys/netinet/sctp_input.c index 86656a7f7eb2..845fd9afc1b8 100644 --- a/sys/netinet/sctp_input.c +++ b/sys/netinet/sctp_input.c @@ -2155,8 +2155,8 @@ sctp_process_cookie_new(struct mbuf *m, int iphlen, int offset, ntohl(initack_cp->init.initiate_tag), vrf_id, ntohs(initack_cp->init.num_outbound_streams), port, - (struct thread *)NULL - ); + (struct thread *)NULL, + SCTP_DONT_INITIALIZE_AUTH_PARAMS); if (stcb == NULL) { struct mbuf *op_err; diff --git a/sys/netinet/sctp_output.c b/sys/netinet/sctp_output.c index 9a81c40332de..28969ba40ff7 100644 --- a/sys/netinet/sctp_output.c +++ b/sys/netinet/sctp_output.c @@ -12769,7 +12769,8 @@ sctp_lower_sosend(struct socket *so, stcb = sctp_aloc_assoc(inp, addr, &error, 0, vrf_id, inp->sctp_ep.pre_open_stream_count, inp->sctp_ep.port, - p); + p, + SCTP_INITIALIZE_AUTH_PARAMS); if (stcb == NULL) { /* Error is setup for us in the call */ goto out_unlocked; @@ -12798,9 +12799,6 @@ sctp_lower_sosend(struct socket *so, SCTP_SET_STATE(stcb, SCTP_STATE_COOKIE_WAIT); (void)SCTP_GETTIME_TIMEVAL(&asoc->time_entered); - /* initialize authentication params for the assoc */ - sctp_initialize_auth_params(inp, stcb); - if (control) { if (sctp_process_cmsgs_for_init(stcb, control, &error)) { sctp_free_assoc(inp, stcb, SCTP_PCBFREE_FORCE, diff --git a/sys/netinet/sctp_pcb.c b/sys/netinet/sctp_pcb.c index e29142658bd1..42bf4e544de1 100644 --- a/sys/netinet/sctp_pcb.c +++ b/sys/netinet/sctp_pcb.c @@ -4190,8 +4190,8 @@ struct sctp_tcb * sctp_aloc_assoc(struct sctp_inpcb *inp, struct sockaddr *firstaddr, int *error, uint32_t override_tag, uint32_t vrf_id, uint16_t o_streams, uint16_t port, - struct thread *p -) + struct thread *p, + int initialize_auth_params) { /* note the p argument is only valid in unbound sockets */ @@ -4421,6 +4421,9 @@ sctp_aloc_assoc(struct sctp_inpcb *inp, struct sockaddr *firstaddr, inp->sctp_hashmark)]; LIST_INSERT_HEAD(head, stcb, sctp_tcbhash); } + if (initialize_auth_params == SCTP_INITIALIZE_AUTH_PARAMS) { + sctp_initialize_auth_params(inp, stcb); + } SCTP_INP_WUNLOCK(inp); SCTPDBG(SCTP_DEBUG_PCB1, "Association %p now allocated\n", (void *)stcb); return (stcb); diff --git a/sys/netinet/sctp_pcb.h b/sys/netinet/sctp_pcb.h index 5b41ae8a6cff..0f5aca88ed47 100644 --- a/sys/netinet/sctp_pcb.h +++ b/sys/netinet/sctp_pcb.h @@ -578,9 +578,13 @@ int sctp_is_address_on_local_host(struct sockaddr *addr, uint32_t vrf_id); void sctp_inpcb_free(struct sctp_inpcb *, int, int); +#define SCTP_DONT_INITIALIZE_AUTH_PARAMS 0 +#define SCTP_INITIALIZE_AUTH_PARAMS 1 + struct sctp_tcb * sctp_aloc_assoc(struct sctp_inpcb *, struct sockaddr *, - int *, uint32_t, uint32_t, uint16_t, uint16_t, struct thread *); + int *, uint32_t, uint32_t, uint16_t, uint16_t, struct thread *, + int); int sctp_free_assoc(struct sctp_inpcb *, struct sctp_tcb *, int, int); diff --git a/sys/netinet/sctp_usrreq.c b/sys/netinet/sctp_usrreq.c index cff7dd769485..edc03b3bf987 100644 --- a/sys/netinet/sctp_usrreq.c +++ b/sys/netinet/sctp_usrreq.c @@ -1443,8 +1443,8 @@ sctp_do_connect_x(struct socket *so, struct sctp_inpcb *inp, void *optval, stcb = sctp_aloc_assoc(inp, sa, &error, 0, vrf_id, inp->sctp_ep.pre_open_stream_count, inp->sctp_ep.port, - (struct thread *)p - ); + (struct thread *)p, + SCTP_INITIALIZE_AUTH_PARAMS); if (stcb == NULL) { /* Gak! no memory */ goto out_now; @@ -1480,9 +1480,6 @@ sctp_do_connect_x(struct socket *so, struct sctp_inpcb *inp, void *optval, a_id = (sctp_assoc_t *)optval; *a_id = sctp_get_associd(stcb); - /* initialize authentication parameters for the assoc */ - sctp_initialize_auth_params(inp, stcb); - if (delay) { /* doing delayed connection */ stcb->asoc.delayed_connection = 1; @@ -7025,7 +7022,8 @@ sctp_connect(struct socket *so, struct sockaddr *addr, struct thread *p) /* We are GOOD to go */ stcb = sctp_aloc_assoc(inp, addr, &error, 0, vrf_id, inp->sctp_ep.pre_open_stream_count, - inp->sctp_ep.port, p); + inp->sctp_ep.port, p, + SCTP_INITIALIZE_AUTH_PARAMS); if (stcb == NULL) { /* Gak! no memory */ goto out_now; @@ -7038,9 +7036,6 @@ sctp_connect(struct socket *so, struct sockaddr *addr, struct thread *p) SCTP_SET_STATE(stcb, SCTP_STATE_COOKIE_WAIT); (void)SCTP_GETTIME_TIMEVAL(&stcb->asoc.time_entered); - /* initialize authentication parameters for the assoc */ - sctp_initialize_auth_params(inp, stcb); - sctp_send_initiate(inp, stcb, SCTP_SO_LOCKED); SCTP_TCB_UNLOCK(stcb); out_now: diff --git a/sys/netinet6/sctp6_usrreq.c b/sys/netinet6/sctp6_usrreq.c index 33cea2399ca7..0b7742a01d1d 100644 --- a/sys/netinet6/sctp6_usrreq.c +++ b/sys/netinet6/sctp6_usrreq.c @@ -908,7 +908,8 @@ sctp6_connect(struct socket *so, struct sockaddr *addr, struct thread *p) /* We are GOOD to go */ stcb = sctp_aloc_assoc(inp, addr, &error, 0, vrf_id, inp->sctp_ep.pre_open_stream_count, - inp->sctp_ep.port, p); + inp->sctp_ep.port, p, + SCTP_INITIALIZE_AUTH_PARAMS); SCTP_ASOC_CREATE_UNLOCK(inp); if (stcb == NULL) { /* Gak! no memory */ @@ -921,10 +922,6 @@ sctp6_connect(struct socket *so, struct sockaddr *addr, struct thread *p) } SCTP_SET_STATE(stcb, SCTP_STATE_COOKIE_WAIT); (void)SCTP_GETTIME_TIMEVAL(&stcb->asoc.time_entered); - - /* initialize authentication parameters for the assoc */ - sctp_initialize_auth_params(inp, stcb); - sctp_send_initiate(inp, stcb, SCTP_SO_LOCKED); SCTP_TCB_UNLOCK(stcb); return (error);