From f84af33138a827dbdf7ee0df2ed82377b43cd1cc Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:10 +0800 Subject: [PATCH 1/9] sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg This patch is to move the codes for checking and sending on one asoc after this asoc has been found or created into sctp_sendmsg_to_asoc. Note that 'err != -ESRCH' check is for the case that asoc is freed when waiting for tx buffer in sctp_sendmsg_to_asoc. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 230 ++++++++++++++++++++-------------------------- 1 file changed, 99 insertions(+), 131 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index bf271f8c2dc9..183129e2bc68 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1606,6 +1606,100 @@ static int sctp_error(struct sock *sk, int flags, int err) static int sctp_msghdr_parse(const struct msghdr *msg, struct sctp_cmsgs *cmsgs); +static int sctp_sendmsg_to_asoc(struct sctp_association *asoc, + struct msghdr *msg, size_t msg_len, + struct sctp_transport *transport, + struct sctp_sndrcvinfo *sinfo) +{ + struct sock *sk = asoc->base.sk; + struct net *net = sock_net(sk); + struct sctp_datamsg *datamsg; + bool wait_connect = false; + struct sctp_chunk *chunk; + long timeo; + int err; + + if (sinfo->sinfo_stream >= asoc->stream.outcnt) { + err = -EINVAL; + goto err; + } + + if (unlikely(!asoc->stream.out[sinfo->sinfo_stream].ext)) { + err = sctp_stream_init_ext(&asoc->stream, sinfo->sinfo_stream); + if (err) + goto err; + } + + if (sctp_sk(sk)->disable_fragments && msg_len > asoc->frag_point) { + err = -EMSGSIZE; + goto err; + } + + if (sctp_state(asoc, CLOSED)) { + err = sctp_primitive_ASSOCIATE(net, asoc, NULL); + if (err) + goto err; + + if (sctp_sk(sk)->strm_interleave) { + timeo = sock_sndtimeo(sk, 0); + err = sctp_wait_for_connect(asoc, &timeo); + if (err) + goto err; + } else { + wait_connect = true; + } + + pr_debug("%s: we associated primitively\n", __func__); + } + + if (asoc->pmtu_pending) + sctp_assoc_pending_pmtu(asoc); + + if (sctp_wspace(asoc) < msg_len) + sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc)); + + if (!sctp_wspace(asoc)) { + timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT); + err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len); + if (err) + goto err; + } + + datamsg = sctp_datamsg_from_user(asoc, sinfo, &msg->msg_iter); + if (IS_ERR(datamsg)) { + err = PTR_ERR(datamsg); + goto err; + } + + asoc->force_delay = !!(msg->msg_flags & MSG_MORE); + + list_for_each_entry(chunk, &datamsg->chunks, frag_list) { + sctp_chunk_hold(chunk); + sctp_set_owner_w(chunk); + chunk->transport = transport; + } + + err = sctp_primitive_SEND(net, asoc, datamsg); + if (err) { + sctp_datamsg_free(datamsg); + goto err; + } + + pr_debug("%s: we sent primitively\n", __func__); + + sctp_datamsg_put(datamsg); + + if (unlikely(wait_connect)) { + timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT); + sctp_wait_for_connect(asoc, &timeo); + } + + err = msg_len; + +err: + return err; +} + static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) { struct net *net = sock_net(sk); @@ -1622,11 +1716,8 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) sctp_assoc_t associd = 0; struct sctp_cmsgs cmsgs = { NULL }; enum sctp_scope scope; - bool fill_sinfo_ttl = false, wait_connect = false; - struct sctp_datamsg *datamsg; - int msg_flags = msg->msg_flags; + bool fill_sinfo_ttl = false; __u16 sinfo_flags = 0; - long timeo; int err; err = 0; @@ -1923,49 +2014,6 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) goto out_free; } - if (asoc->pmtu_pending) - sctp_assoc_pending_pmtu(asoc); - - /* If fragmentation is disabled and the message length exceeds the - * association fragmentation point, return EMSGSIZE. The I-D - * does not specify what this error is, but this looks like - * a great fit. - */ - if (sctp_sk(sk)->disable_fragments && (msg_len > asoc->frag_point)) { - err = -EMSGSIZE; - goto out_free; - } - - /* Check for invalid stream. */ - if (sinfo->sinfo_stream >= asoc->stream.outcnt) { - err = -EINVAL; - goto out_free; - } - - /* Allocate sctp_stream_out_ext if not already done */ - if (unlikely(!asoc->stream.out[sinfo->sinfo_stream].ext)) { - err = sctp_stream_init_ext(&asoc->stream, sinfo->sinfo_stream); - if (err) - goto out_free; - } - - if (sctp_wspace(asoc) < msg_len) - sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc)); - - timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT); - if (!sctp_wspace(asoc)) { - /* sk can be changed by peel off when waiting for buf. */ - err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len); - if (err) { - if (err == -ESRCH) { - /* asoc is already dead. */ - new_asoc = NULL; - err = -EPIPE; - } - goto out_free; - } - } - /* If an address is passed with the sendto/sendmsg call, it is used * to override the primary destination address in the TCP model, or * when SCTP_ADDR_OVER flag is set in the UDP model. @@ -1980,96 +2028,16 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) } else chunk_tp = NULL; - /* Auto-connect, if we aren't connected already. */ - if (sctp_state(asoc, CLOSED)) { - err = sctp_primitive_ASSOCIATE(net, asoc, NULL); - if (err < 0) - goto out_free; - - /* If stream interleave is enabled, wait_connect has to be - * done earlier than data enqueue, as it needs to make data - * or idata according to asoc->intl_enable which is set - * after connection is done. - */ - if (sctp_sk(asoc->base.sk)->strm_interleave) { - timeo = sock_sndtimeo(sk, 0); - err = sctp_wait_for_connect(asoc, &timeo); - if (err) - goto out_unlock; - } else { - wait_connect = true; - } - - pr_debug("%s: we associated primitively\n", __func__); - } - - /* Break the message into multiple chunks of maximum size. */ - datamsg = sctp_datamsg_from_user(asoc, sinfo, &msg->msg_iter); - if (IS_ERR(datamsg)) { - err = PTR_ERR(datamsg); - goto out_free; - } - asoc->force_delay = !!(msg->msg_flags & MSG_MORE); - - /* Now send the (possibly) fragmented message. */ - list_for_each_entry(chunk, &datamsg->chunks, frag_list) { - sctp_chunk_hold(chunk); - - /* Do accounting for the write space. */ - sctp_set_owner_w(chunk); - - chunk->transport = chunk_tp; - } - - /* Send it to the lower layers. Note: all chunks - * must either fail or succeed. The lower layer - * works that way today. Keep it that way or this - * breaks. - */ - err = sctp_primitive_SEND(net, asoc, datamsg); - /* Did the lower layer accept the chunk? */ - if (err) { - sctp_datamsg_free(datamsg); - goto out_free; - } - - pr_debug("%s: we sent primitively\n", __func__); - - sctp_datamsg_put(datamsg); - err = msg_len; - - if (unlikely(wait_connect)) { - timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT); - sctp_wait_for_connect(asoc, &timeo); - } - - /* If we are already past ASSOCIATE, the lower - * layers are responsible for association cleanup. - */ - goto out_unlock; + /* Send msg to the asoc */ + err = sctp_sendmsg_to_asoc(asoc, msg, msg_len, chunk_tp, sinfo); out_free: - if (new_asoc) + if (err < 0 && err != -ESRCH && new_asoc) sctp_association_free(asoc); out_unlock: release_sock(sk); - out_nounlock: - return sctp_error(sk, msg_flags, err); - -#if 0 -do_sock_err: - if (msg_len) - err = msg_len; - else - err = sock_error(sk); - goto out; - -do_interrupted: - if (msg_len) - err = msg_len; - goto out; -#endif /* 0 */ + return sctp_error(sk, msg->msg_flags, err); } /* This is an extended version of skb_pull() that removes the data from the From 2bfd80f9edbfc18c6247be929df3b348329141a0 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:11 +0800 Subject: [PATCH 2/9] sctp: factor out sctp_sendmsg_new_asoc from sctp_sendmsg This patch is to move the codes for creating a new asoc if no asoc was found into sctp_sendmsg_new_asoc. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 201 ++++++++++++++++++++-------------------------- 1 file changed, 86 insertions(+), 115 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 183129e2bc68..58bb55dce8f6 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1606,6 +1606,87 @@ static int sctp_error(struct sock *sk, int flags, int err) static int sctp_msghdr_parse(const struct msghdr *msg, struct sctp_cmsgs *cmsgs); +static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, + struct sctp_cmsgs *cmsgs, + union sctp_addr *daddr, + struct sctp_transport **tp) +{ + struct sctp_endpoint *ep = sctp_sk(sk)->ep; + struct net *net = sock_net(sk); + struct sctp_association *asoc; + enum sctp_scope scope; + int err = -EINVAL; + + *tp = NULL; + + if (sflags & (SCTP_EOF | SCTP_ABORT)) + return -EINVAL; + + if (sctp_style(sk, TCP) && (sctp_sstate(sk, ESTABLISHED) || + sctp_sstate(sk, CLOSING))) + return -EADDRNOTAVAIL; + + if (sctp_endpoint_is_peeled_off(ep, daddr)) + return -EADDRNOTAVAIL; + + if (!ep->base.bind_addr.port) { + if (sctp_autobind(sk)) + return -EAGAIN; + } else { + if (ep->base.bind_addr.port < inet_prot_sock(net) && + !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE)) + return -EACCES; + } + + scope = sctp_scope(daddr); + + asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL); + if (!asoc) + return -ENOMEM; + + if (sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL) < 0) { + err = -ENOMEM; + goto free; + } + + if (cmsgs->init) { + struct sctp_initmsg *init = cmsgs->init; + + if (init->sinit_num_ostreams) { + __u16 outcnt = init->sinit_num_ostreams; + + asoc->c.sinit_num_ostreams = outcnt; + /* outcnt has been changed, need to re-init stream */ + err = sctp_stream_init(&asoc->stream, outcnt, 0, + GFP_KERNEL); + if (err) + goto free; + } + + if (init->sinit_max_instreams) + asoc->c.sinit_max_instreams = init->sinit_max_instreams; + + if (init->sinit_max_attempts) + asoc->max_init_attempts = init->sinit_max_attempts; + + if (init->sinit_max_init_timeo) + asoc->max_init_timeo = + msecs_to_jiffies(init->sinit_max_init_timeo); + } + + *tp = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL, SCTP_UNKNOWN); + if (!*tp) { + err = -ENOMEM; + goto free; + } + + return 0; + +free: + sctp_association_free(asoc); + return err; +} + static int sctp_sendmsg_to_asoc(struct sctp_association *asoc, struct msghdr *msg, size_t msg_len, struct sctp_transport *transport, @@ -1715,7 +1796,6 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) struct sctp_initmsg *sinit; sctp_assoc_t associd = 0; struct sctp_cmsgs cmsgs = { NULL }; - enum sctp_scope scope; bool fill_sinfo_ttl = false; __u16 sinfo_flags = 0; int err; @@ -1817,20 +1897,6 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) if (msg_name) { /* Look for a matching association on the endpoint. */ asoc = sctp_endpoint_lookup_assoc(ep, &to, &transport); - - /* If we could not find a matching association on the - * endpoint, make sure that it is not a TCP-style - * socket that already has an association or there is - * no peeled-off association on another socket. - */ - if (!asoc && - ((sctp_style(sk, TCP) && - (sctp_sstate(sk, ESTABLISHED) || - sctp_sstate(sk, CLOSING))) || - sctp_endpoint_is_peeled_off(ep, &to))) { - err = -EADDRNOTAVAIL; - goto out_unlock; - } } else { asoc = sctp_id2assoc(sk, associd); if (!asoc) { @@ -1879,108 +1945,13 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) /* Do we need to create the association? */ if (!asoc) { - pr_debug("%s: there is no association yet\n", __func__); - - if (sinfo_flags & (SCTP_EOF | SCTP_ABORT)) { - err = -EINVAL; + err = sctp_sendmsg_new_asoc(sk, sinfo_flags, &cmsgs, &to, + &transport); + if (err) goto out_unlock; - } - /* Check for invalid stream against the stream counts, - * either the default or the user specified stream counts. - */ - if (sinfo) { - if (!sinit || !sinit->sinit_num_ostreams) { - /* Check against the defaults. */ - if (sinfo->sinfo_stream >= - sp->initmsg.sinit_num_ostreams) { - err = -EINVAL; - goto out_unlock; - } - } else { - /* Check against the requested. */ - if (sinfo->sinfo_stream >= - sinit->sinit_num_ostreams) { - err = -EINVAL; - goto out_unlock; - } - } - } - - /* - * API 3.1.2 bind() - UDP Style Syntax - * If a bind() or sctp_bindx() is not called prior to a - * sendmsg() call that initiates a new association, the - * system picks an ephemeral port and will choose an address - * set equivalent to binding with a wildcard address. - */ - if (!ep->base.bind_addr.port) { - if (sctp_autobind(sk)) { - err = -EAGAIN; - goto out_unlock; - } - } else { - /* - * If an unprivileged user inherits a one-to-many - * style socket with open associations on a privileged - * port, it MAY be permitted to accept new associations, - * but it SHOULD NOT be permitted to open new - * associations. - */ - if (ep->base.bind_addr.port < inet_prot_sock(net) && - !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE)) { - err = -EACCES; - goto out_unlock; - } - } - - scope = sctp_scope(&to); - new_asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL); - if (!new_asoc) { - err = -ENOMEM; - goto out_unlock; - } - asoc = new_asoc; - err = sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL); - if (err < 0) { - err = -ENOMEM; - goto out_free; - } - - /* If the SCTP_INIT ancillary data is specified, set all - * the association init values accordingly. - */ - if (sinit) { - if (sinit->sinit_num_ostreams) { - __u16 outcnt = sinit->sinit_num_ostreams; - - asoc->c.sinit_num_ostreams = outcnt; - /* outcnt has been changed, so re-init stream */ - err = sctp_stream_init(&asoc->stream, outcnt, 0, - GFP_KERNEL); - if (err) - goto out_free; - } - if (sinit->sinit_max_instreams) { - asoc->c.sinit_max_instreams = - sinit->sinit_max_instreams; - } - if (sinit->sinit_max_attempts) { - asoc->max_init_attempts - = sinit->sinit_max_attempts; - } - if (sinit->sinit_max_init_timeo) { - asoc->max_init_timeo = - msecs_to_jiffies(sinit->sinit_max_init_timeo); - } - } - - /* Prime the peer's transport structures. */ - transport = sctp_assoc_add_peer(asoc, &to, GFP_KERNEL, SCTP_UNKNOWN); - if (!transport) { - err = -ENOMEM; - goto out_free; - } + asoc = transport->asoc; + new_asoc = asoc; } /* ASSERT: we have a valid association at this point. */ From c2666de1fde3a02583c2c4d4af4bc54f7252e891 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:12 +0800 Subject: [PATCH 3/9] sctp: factor out sctp_sendmsg_check_sflags from sctp_sendmsg This patch is to move the codes for checking sinfo_flags on one asoc after this asoc has been found into sctp_sendmsg_check_sflags. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 72 +++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 58bb55dce8f6..93cff9963614 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1687,6 +1687,39 @@ free: return err; } +static int sctp_sendmsg_check_sflags(struct sctp_association *asoc, + __u16 sflags, struct msghdr *msg, + size_t msg_len) +{ + struct sock *sk = asoc->base.sk; + struct net *net = sock_net(sk); + + if (sctp_state(asoc, CLOSED) && sctp_style(sk, TCP)) + return -EPIPE; + + if (sflags & SCTP_EOF) { + pr_debug("%s: shutting down association:%p\n", __func__, asoc); + sctp_primitive_SHUTDOWN(net, asoc, NULL); + + return 0; + } + + if (sflags & SCTP_ABORT) { + struct sctp_chunk *chunk; + + chunk = sctp_make_abort_user(asoc, msg, msg_len); + if (!chunk) + return -ENOMEM; + + pr_debug("%s: aborting association:%p\n", __func__, asoc); + sctp_primitive_ABORT(net, asoc, chunk); + + return 0; + } + + return 1; +} + static int sctp_sendmsg_to_asoc(struct sctp_association *asoc, struct msghdr *msg, size_t msg_len, struct sctp_transport *transport, @@ -1783,12 +1816,10 @@ err: static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) { - struct net *net = sock_net(sk); struct sctp_sock *sp; struct sctp_endpoint *ep; struct sctp_association *new_asoc = NULL, *asoc = NULL; struct sctp_transport *transport, *chunk_tp; - struct sctp_chunk *chunk; union sctp_addr to; struct sockaddr *msg_name = NULL; struct sctp_sndrcvinfo default_sinfo; @@ -1906,41 +1937,10 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) } if (asoc) { - pr_debug("%s: just looked up association:%p\n", __func__, asoc); - - /* We cannot send a message on a TCP-style SCTP_SS_ESTABLISHED - * socket that has an association in CLOSED state. This can - * happen when an accepted socket has an association that is - * already CLOSED. - */ - if (sctp_state(asoc, CLOSED) && sctp_style(sk, TCP)) { - err = -EPIPE; + err = sctp_sendmsg_check_sflags(asoc, sinfo_flags, msg, + msg_len); + if (err <= 0) goto out_unlock; - } - - if (sinfo_flags & SCTP_EOF) { - pr_debug("%s: shutting down association:%p\n", - __func__, asoc); - - sctp_primitive_SHUTDOWN(net, asoc, NULL); - err = 0; - goto out_unlock; - } - if (sinfo_flags & SCTP_ABORT) { - - chunk = sctp_make_abort_user(asoc, msg, msg_len); - if (!chunk) { - err = -ENOMEM; - goto out_unlock; - } - - pr_debug("%s: aborting association:%p\n", - __func__, asoc); - - sctp_primitive_ABORT(net, asoc, chunk); - err = 0; - goto out_unlock; - } } /* Do we need to create the association? */ From becef9b1e249c849ca15889586e02668d6d723a2 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:13 +0800 Subject: [PATCH 4/9] sctp: factor out sctp_sendmsg_get_daddr from sctp_sendmsg This patch is to move the codes for trying to get daddr from msg->msg_name into sctp_sendmsg_get_daddr. Note that after adding 'daddr', 'to' and 'msg_name' can be deleted. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 58 +++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 93cff9963614..68691d2b3167 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1814,14 +1814,35 @@ err: return err; } +static union sctp_addr *sctp_sendmsg_get_daddr(struct sock *sk, + const struct msghdr *msg, + struct sctp_cmsgs *cmsgs) +{ + union sctp_addr *daddr = NULL; + int err; + + if (!sctp_style(sk, UDP_HIGH_BANDWIDTH) && msg->msg_name) { + int len = msg->msg_namelen; + + if (len > sizeof(*daddr)) + len = sizeof(*daddr); + + daddr = (union sctp_addr *)msg->msg_name; + + err = sctp_verify_addr(sk, daddr, len); + if (err) + return ERR_PTR(err); + } + + return daddr; +} + static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) { struct sctp_sock *sp; struct sctp_endpoint *ep; struct sctp_association *new_asoc = NULL, *asoc = NULL; struct sctp_transport *transport, *chunk_tp; - union sctp_addr to; - struct sockaddr *msg_name = NULL; struct sctp_sndrcvinfo default_sinfo; struct sctp_sndrcvinfo *sinfo; struct sctp_initmsg *sinit; @@ -1829,6 +1850,7 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) struct sctp_cmsgs cmsgs = { NULL }; bool fill_sinfo_ttl = false; __u16 sinfo_flags = 0; + union sctp_addr *daddr; int err; err = 0; @@ -1851,23 +1873,11 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) goto out_nounlock; } - /* Fetch the destination address for this packet. This - * address only selects the association--it is not necessarily - * the address we will send to. - * For a peeled-off socket, msg_name is ignored. - */ - if (!sctp_style(sk, UDP_HIGH_BANDWIDTH) && msg->msg_name) { - int msg_namelen = msg->msg_namelen; - - err = sctp_verify_addr(sk, (union sctp_addr *)msg->msg_name, - msg_namelen); - if (err) - return err; - - if (msg_namelen > sizeof(to)) - msg_namelen = sizeof(to); - memcpy(&to, msg->msg_name, msg_namelen); - msg_name = msg->msg_name; + /* Get daddr from msg */ + daddr = sctp_sendmsg_get_daddr(sk, msg, &cmsgs); + if (IS_ERR(daddr)) { + err = PTR_ERR(daddr); + goto out_nounlock; } sinit = cmsgs.init; @@ -1925,9 +1935,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) lock_sock(sk); /* If a msg_name has been specified, assume this is to be used. */ - if (msg_name) { + if (daddr) { /* Look for a matching association on the endpoint. */ - asoc = sctp_endpoint_lookup_assoc(ep, &to, &transport); + asoc = sctp_endpoint_lookup_assoc(ep, daddr, &transport); } else { asoc = sctp_id2assoc(sk, associd); if (!asoc) { @@ -1945,7 +1955,7 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) /* Do we need to create the association? */ if (!asoc) { - err = sctp_sendmsg_new_asoc(sk, sinfo_flags, &cmsgs, &to, + err = sctp_sendmsg_new_asoc(sk, sinfo_flags, &cmsgs, daddr, &transport); if (err) goto out_unlock; @@ -1989,9 +1999,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) * to override the primary destination address in the TCP model, or * when SCTP_ADDR_OVER flag is set in the UDP model. */ - if ((sctp_style(sk, TCP) && msg_name) || + if ((sctp_style(sk, TCP) && daddr) || (sinfo_flags & SCTP_ADDR_OVER)) { - chunk_tp = sctp_assoc_lookup_paddr(asoc, &to); + chunk_tp = sctp_assoc_lookup_paddr(asoc, daddr); if (!chunk_tp) { err = -EINVAL; goto out_free; From 204f817fb9ab1b42c34c27d593cad2992d575f71 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:14 +0800 Subject: [PATCH 5/9] sctp: factor out sctp_sendmsg_parse from sctp_sendmsg This patch is to move the codes for parsing msghdr and checking sk into sctp_sendmsg_parse. Note that different from before, 'sinfo' in sctp_sendmsg won't be NULL any more. It gets the value either from cmsgs->srinfo, cmsgs->sinfo or asoc. With it, the 'sinfo' and 'fill_sinfo_ttl' check can be removed from sctp_sendmsg. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 172 +++++++++++++++++++--------------------------- 1 file changed, 69 insertions(+), 103 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 68691d2b3167..bf089e59c792 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1606,6 +1606,61 @@ static int sctp_error(struct sock *sk, int flags, int err) static int sctp_msghdr_parse(const struct msghdr *msg, struct sctp_cmsgs *cmsgs); +static int sctp_sendmsg_parse(struct sock *sk, struct sctp_cmsgs *cmsgs, + struct sctp_sndrcvinfo *srinfo, + const struct msghdr *msg, size_t msg_len) +{ + __u16 sflags; + int err; + + if (sctp_sstate(sk, LISTENING) && sctp_style(sk, TCP)) + return -EPIPE; + + if (msg_len > sk->sk_sndbuf) + return -EMSGSIZE; + + memset(cmsgs, 0, sizeof(*cmsgs)); + err = sctp_msghdr_parse(msg, cmsgs); + if (err) { + pr_debug("%s: msghdr parse err:%x\n", __func__, err); + return err; + } + + memset(srinfo, 0, sizeof(*srinfo)); + if (cmsgs->srinfo) { + srinfo->sinfo_stream = cmsgs->srinfo->sinfo_stream; + srinfo->sinfo_flags = cmsgs->srinfo->sinfo_flags; + srinfo->sinfo_ppid = cmsgs->srinfo->sinfo_ppid; + srinfo->sinfo_context = cmsgs->srinfo->sinfo_context; + srinfo->sinfo_assoc_id = cmsgs->srinfo->sinfo_assoc_id; + srinfo->sinfo_timetolive = cmsgs->srinfo->sinfo_timetolive; + } + + if (cmsgs->sinfo) { + srinfo->sinfo_stream = cmsgs->sinfo->snd_sid; + srinfo->sinfo_flags = cmsgs->sinfo->snd_flags; + srinfo->sinfo_ppid = cmsgs->sinfo->snd_ppid; + srinfo->sinfo_context = cmsgs->sinfo->snd_context; + srinfo->sinfo_assoc_id = cmsgs->sinfo->snd_assoc_id; + } + + sflags = srinfo->sinfo_flags; + if (!sflags && msg_len) + return 0; + + if (sctp_style(sk, TCP) && (sflags & (SCTP_EOF | SCTP_ABORT))) + return -EINVAL; + + if (((sflags & SCTP_EOF) && msg_len > 0) || + (!(sflags & (SCTP_EOF | SCTP_ABORT)) && msg_len == 0)) + return -EINVAL; + + if ((sflags & SCTP_ADDR_OVER) && !msg->msg_name) + return -EINVAL; + + return 0; +} + static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, struct sctp_cmsgs *cmsgs, union sctp_addr *daddr, @@ -1839,39 +1894,23 @@ static union sctp_addr *sctp_sendmsg_get_daddr(struct sock *sk, static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) { - struct sctp_sock *sp; - struct sctp_endpoint *ep; + struct sctp_endpoint *ep = sctp_sk(sk)->ep; struct sctp_association *new_asoc = NULL, *asoc = NULL; struct sctp_transport *transport, *chunk_tp; - struct sctp_sndrcvinfo default_sinfo; - struct sctp_sndrcvinfo *sinfo; - struct sctp_initmsg *sinit; + struct sctp_sndrcvinfo _sinfo, *sinfo; sctp_assoc_t associd = 0; struct sctp_cmsgs cmsgs = { NULL }; - bool fill_sinfo_ttl = false; __u16 sinfo_flags = 0; union sctp_addr *daddr; int err; - err = 0; - sp = sctp_sk(sk); - ep = sp->ep; - - pr_debug("%s: sk:%p, msg:%p, msg_len:%zu ep:%p\n", __func__, sk, - msg, msg_len, ep); - - /* We cannot send a message over a TCP-style listening socket. */ - if (sctp_style(sk, TCP) && sctp_sstate(sk, LISTENING)) { - err = -EPIPE; + /* Parse and get snd_info */ + err = sctp_sendmsg_parse(sk, &cmsgs, &_sinfo, msg, msg_len); + if (err) goto out_nounlock; - } - /* Parse out the SCTP CMSGs. */ - err = sctp_msghdr_parse(msg, &cmsgs); - if (err) { - pr_debug("%s: msghdr parse err:%x\n", __func__, err); - goto out_nounlock; - } + sinfo = &_sinfo; + sinfo_flags = sinfo->sinfo_flags; /* Get daddr from msg */ daddr = sctp_sendmsg_get_daddr(sk, msg, &cmsgs); @@ -1880,58 +1919,6 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) goto out_nounlock; } - sinit = cmsgs.init; - if (cmsgs.sinfo != NULL) { - memset(&default_sinfo, 0, sizeof(default_sinfo)); - default_sinfo.sinfo_stream = cmsgs.sinfo->snd_sid; - default_sinfo.sinfo_flags = cmsgs.sinfo->snd_flags; - default_sinfo.sinfo_ppid = cmsgs.sinfo->snd_ppid; - default_sinfo.sinfo_context = cmsgs.sinfo->snd_context; - default_sinfo.sinfo_assoc_id = cmsgs.sinfo->snd_assoc_id; - - sinfo = &default_sinfo; - fill_sinfo_ttl = true; - } else { - sinfo = cmsgs.srinfo; - } - /* Did the user specify SNDINFO/SNDRCVINFO? */ - if (sinfo) { - sinfo_flags = sinfo->sinfo_flags; - associd = sinfo->sinfo_assoc_id; - } - - pr_debug("%s: msg_len:%zu, sinfo_flags:0x%x\n", __func__, - msg_len, sinfo_flags); - - /* SCTP_EOF or SCTP_ABORT cannot be set on a TCP-style socket. */ - if (sctp_style(sk, TCP) && (sinfo_flags & (SCTP_EOF | SCTP_ABORT))) { - err = -EINVAL; - goto out_nounlock; - } - - /* If SCTP_EOF is set, no data can be sent. Disallow sending zero - * length messages when SCTP_EOF|SCTP_ABORT is not set. - * If SCTP_ABORT is set, the message length could be non zero with - * the msg_iov set to the user abort reason. - */ - if (((sinfo_flags & SCTP_EOF) && (msg_len > 0)) || - (!(sinfo_flags & (SCTP_EOF|SCTP_ABORT)) && (msg_len == 0))) { - err = -EINVAL; - goto out_nounlock; - } - - /* If SCTP_ADDR_OVER is set, there must be an address - * specified in msg_name. - */ - if ((sinfo_flags & SCTP_ADDR_OVER) && (!msg->msg_name)) { - err = -EINVAL; - goto out_nounlock; - } - - transport = NULL; - - pr_debug("%s: about to look up association\n", __func__); - lock_sock(sk); /* If a msg_name has been specified, assume this is to be used. */ @@ -1964,36 +1951,15 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) new_asoc = asoc; } - /* ASSERT: we have a valid association at this point. */ - pr_debug("%s: we have a valid association\n", __func__); + if (!cmsgs.srinfo && !cmsgs.sinfo) { + sinfo->sinfo_stream = asoc->default_stream; + sinfo->sinfo_ppid = asoc->default_ppid; + sinfo->sinfo_context = asoc->default_context; + sinfo->sinfo_assoc_id = sctp_assoc2id(asoc); + } - if (!sinfo) { - /* If the user didn't specify SNDINFO/SNDRCVINFO, make up - * one with some defaults. - */ - memset(&default_sinfo, 0, sizeof(default_sinfo)); - default_sinfo.sinfo_stream = asoc->default_stream; - default_sinfo.sinfo_flags = asoc->default_flags; - default_sinfo.sinfo_ppid = asoc->default_ppid; - default_sinfo.sinfo_context = asoc->default_context; - default_sinfo.sinfo_timetolive = asoc->default_timetolive; - default_sinfo.sinfo_assoc_id = sctp_assoc2id(asoc); - - sinfo = &default_sinfo; - } else if (fill_sinfo_ttl) { - /* In case SNDINFO was specified, we still need to fill - * it with a default ttl from the assoc here. - */ + if (!cmsgs.srinfo) sinfo->sinfo_timetolive = asoc->default_timetolive; - } - - /* API 7.1.7, the sndbuf size per association bounds the - * maximum size of data that can be sent in a single send call. - */ - if (msg_len > sk->sk_sndbuf) { - err = -EMSGSIZE; - goto out_free; - } /* If an address is passed with the sendto/sendmsg call, it is used * to override the primary destination address in the TCP model, or From d42cb06e5b542cabea88c9074b2abf90d43757f7 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:15 +0800 Subject: [PATCH 6/9] sctp: factor out sctp_sendmsg_update_sinfo from sctp_sendmsg This patch is to move the codes for trying to get sinfo from asoc into sctp_sendmsg_update_sinfo. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index bf089e59c792..bd1a657117f1 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1892,6 +1892,21 @@ static union sctp_addr *sctp_sendmsg_get_daddr(struct sock *sk, return daddr; } +static void sctp_sendmsg_update_sinfo(struct sctp_association *asoc, + struct sctp_sndrcvinfo *sinfo, + struct sctp_cmsgs *cmsgs) +{ + if (!cmsgs->srinfo && !cmsgs->sinfo) { + sinfo->sinfo_stream = asoc->default_stream; + sinfo->sinfo_ppid = asoc->default_ppid; + sinfo->sinfo_context = asoc->default_context; + sinfo->sinfo_assoc_id = sctp_assoc2id(asoc); + } + + if (!cmsgs->srinfo) + sinfo->sinfo_timetolive = asoc->default_timetolive; +} + static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) { struct sctp_endpoint *ep = sctp_sk(sk)->ep; @@ -1951,15 +1966,8 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) new_asoc = asoc; } - if (!cmsgs.srinfo && !cmsgs.sinfo) { - sinfo->sinfo_stream = asoc->default_stream; - sinfo->sinfo_ppid = asoc->default_ppid; - sinfo->sinfo_context = asoc->default_context; - sinfo->sinfo_assoc_id = sctp_assoc2id(asoc); - } - - if (!cmsgs.srinfo) - sinfo->sinfo_timetolive = asoc->default_timetolive; + /* Update snd_info with the asoc */ + sctp_sendmsg_update_sinfo(asoc, sinfo, &cmsgs); /* If an address is passed with the sendto/sendmsg call, it is used * to override the primary destination address in the TCP model, or From 8e87c6eb18f9d7427054f28a284d495c174d9970 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:16 +0800 Subject: [PATCH 7/9] sctp: remove the unnecessary transport looking up from sctp_sendmsg Now sctp_assoc_lookup_paddr can only be called only if daddr has been set. But if daddr has been set, sctp_endpoint_lookup_assoc would be done, where it could already have the transport. So this unnecessary transport looking up should be removed, but only reset transport as NULL when SCTP_ADDR_OVER is not set for UDP type socket. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index bd1a657117f1..426031095afe 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1911,7 +1911,7 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) { struct sctp_endpoint *ep = sctp_sk(sk)->ep; struct sctp_association *new_asoc = NULL, *asoc = NULL; - struct sctp_transport *transport, *chunk_tp; + struct sctp_transport *transport = NULL; struct sctp_sndrcvinfo _sinfo, *sinfo; sctp_assoc_t associd = 0; struct sctp_cmsgs cmsgs = { NULL }; @@ -1966,29 +1966,17 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) new_asoc = asoc; } + if (!sctp_style(sk, TCP) && !(sinfo_flags & SCTP_ADDR_OVER)) + transport = NULL; + /* Update snd_info with the asoc */ sctp_sendmsg_update_sinfo(asoc, sinfo, &cmsgs); - /* If an address is passed with the sendto/sendmsg call, it is used - * to override the primary destination address in the TCP model, or - * when SCTP_ADDR_OVER flag is set in the UDP model. - */ - if ((sctp_style(sk, TCP) && daddr) || - (sinfo_flags & SCTP_ADDR_OVER)) { - chunk_tp = sctp_assoc_lookup_paddr(asoc, daddr); - if (!chunk_tp) { - err = -EINVAL; - goto out_free; - } - } else - chunk_tp = NULL; - /* Send msg to the asoc */ - err = sctp_sendmsg_to_asoc(asoc, msg, msg_len, chunk_tp, sinfo); - -out_free: + err = sctp_sendmsg_to_asoc(asoc, msg, msg_len, transport, sinfo); if (err < 0 && err != -ESRCH && new_asoc) sctp_association_free(asoc); + out_unlock: release_sock(sk); out_nounlock: From 007b7e18be74a49b61f89664966ac1477e1c9608 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:17 +0800 Subject: [PATCH 8/9] sctp: improve some variables in sctp_sendmsg This patch mostly is to: - rename sinfo_flags as sflags, to make the indents look better, and also keep consistent with other sctp_sendmsg_xx functions. - replace new_asoc with bool new, no need to define a pointer here, as if new_asoc is set, it must be asoc. - rename the 'out_nounlock:' as 'out', shorter and nicer. - remove associd, only one place is using it now, just use sinfo->sinfo_assoc_id directly. - remove 'cmsgs' initialization in sctp_sendmsg, as it will be done in sctp_sendmsg_parse. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 426031095afe..a1c78fc19d95 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1910,28 +1910,28 @@ static void sctp_sendmsg_update_sinfo(struct sctp_association *asoc, static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) { struct sctp_endpoint *ep = sctp_sk(sk)->ep; - struct sctp_association *new_asoc = NULL, *asoc = NULL; struct sctp_transport *transport = NULL; struct sctp_sndrcvinfo _sinfo, *sinfo; - sctp_assoc_t associd = 0; - struct sctp_cmsgs cmsgs = { NULL }; - __u16 sinfo_flags = 0; + struct sctp_association *asoc; + struct sctp_cmsgs cmsgs; union sctp_addr *daddr; + bool new = false; + __u16 sflags; int err; /* Parse and get snd_info */ err = sctp_sendmsg_parse(sk, &cmsgs, &_sinfo, msg, msg_len); if (err) - goto out_nounlock; + goto out; sinfo = &_sinfo; - sinfo_flags = sinfo->sinfo_flags; + sflags = sinfo->sinfo_flags; /* Get daddr from msg */ daddr = sctp_sendmsg_get_daddr(sk, msg, &cmsgs); if (IS_ERR(daddr)) { err = PTR_ERR(daddr); - goto out_nounlock; + goto out; } lock_sock(sk); @@ -1941,7 +1941,7 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) /* Look for a matching association on the endpoint. */ asoc = sctp_endpoint_lookup_assoc(ep, daddr, &transport); } else { - asoc = sctp_id2assoc(sk, associd); + asoc = sctp_id2assoc(sk, sinfo->sinfo_assoc_id); if (!asoc) { err = -EPIPE; goto out_unlock; @@ -1949,24 +1949,23 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) } if (asoc) { - err = sctp_sendmsg_check_sflags(asoc, sinfo_flags, msg, - msg_len); + err = sctp_sendmsg_check_sflags(asoc, sflags, msg, msg_len); if (err <= 0) goto out_unlock; } /* Do we need to create the association? */ if (!asoc) { - err = sctp_sendmsg_new_asoc(sk, sinfo_flags, &cmsgs, daddr, + err = sctp_sendmsg_new_asoc(sk, sflags, &cmsgs, daddr, &transport); if (err) goto out_unlock; asoc = transport->asoc; - new_asoc = asoc; + new = true; } - if (!sctp_style(sk, TCP) && !(sinfo_flags & SCTP_ADDR_OVER)) + if (!sctp_style(sk, TCP) && !(sflags & SCTP_ADDR_OVER)) transport = NULL; /* Update snd_info with the asoc */ @@ -1974,12 +1973,12 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) /* Send msg to the asoc */ err = sctp_sendmsg_to_asoc(asoc, msg, msg_len, transport, sinfo); - if (err < 0 && err != -ESRCH && new_asoc) + if (err < 0 && err != -ESRCH && new) sctp_association_free(asoc); out_unlock: release_sock(sk); -out_nounlock: +out: return sctp_error(sk, msg->msg_flags, err); } From 0a3920d28b5490e6c90110156135dbc12c7fc413 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 1 Mar 2018 23:05:18 +0800 Subject: [PATCH 9/9] sctp: adjust some codes in a better order in sctp_sendmsg sctp_sendmsg_new_asoc and SCTP_ADDR_OVER check is only necessary when daddr is set, so move them up to if (daddr) statement. Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/socket.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index a1c78fc19d95..7fa76031bb08 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1936,38 +1936,38 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) lock_sock(sk); - /* If a msg_name has been specified, assume this is to be used. */ + /* Get and check or create asoc */ if (daddr) { - /* Look for a matching association on the endpoint. */ asoc = sctp_endpoint_lookup_assoc(ep, daddr, &transport); + if (asoc) { + err = sctp_sendmsg_check_sflags(asoc, sflags, msg, + msg_len); + if (err <= 0) + goto out_unlock; + } else { + err = sctp_sendmsg_new_asoc(sk, sflags, &cmsgs, daddr, + &transport); + if (err) + goto out_unlock; + + asoc = transport->asoc; + new = true; + } + + if (!sctp_style(sk, TCP) && !(sflags & SCTP_ADDR_OVER)) + transport = NULL; } else { asoc = sctp_id2assoc(sk, sinfo->sinfo_assoc_id); if (!asoc) { err = -EPIPE; goto out_unlock; } - } - if (asoc) { err = sctp_sendmsg_check_sflags(asoc, sflags, msg, msg_len); if (err <= 0) goto out_unlock; } - /* Do we need to create the association? */ - if (!asoc) { - err = sctp_sendmsg_new_asoc(sk, sflags, &cmsgs, daddr, - &transport); - if (err) - goto out_unlock; - - asoc = transport->asoc; - new = true; - } - - if (!sctp_style(sk, TCP) && !(sflags & SCTP_ADDR_OVER)) - transport = NULL; - /* Update snd_info with the asoc */ sctp_sendmsg_update_sinfo(asoc, sinfo, &cmsgs);