* [PATCH net-next 0/9] sctp: clean up sctp_sendmsg
@ 2018-03-01 15:05 Xin Long
2018-03-01 15:05 ` [PATCH net-next 1/9] sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg Xin Long
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This cleanup mostly does three things:
- extract some codes into functions to make sendmsg more readable.
- tidy up some codes to avoid the unnecessary checks.
- adjust some logic so that it will be easier to add the send flags
and cmsgs features that I will post after this.
To make it easy to review and to check if the code is compatible with
before, this patchset is to do it step by step in 9 patches.
NOTE:
There will be a conflict when merging
Commit 2277c7cd75e3 ("sctp: Add LSM hooks") from selinux tree,
the solution is to:
1. remove all the lines in [B]:
<<<<<<< HEAD
[A]
=======
[B]
>>>>>>> 2277c7c... sctp: Add LSM hooks
2. and apply the following diff-output:
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 980621e..d6803c8 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1686,6 +1686,7 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
struct net *net = sock_net(sk);
struct sctp_association *asoc;
enum sctp_scope scope;
+ struct sctp_af *af;
int err = -EINVAL;
*tp = NULL;
@@ -1711,6 +1712,22 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
scope = sctp_scope(daddr);
+ /* Label connection socket for first association 1-to-many
+ * style for client sequence socket()->sendmsg(). This
+ * needs to be done before sctp_assoc_add_peer() as that will
+ * set up the initial packet that needs to account for any
+ * security ip options (CIPSO/CALIPSO) added to the packet.
+ */
+ af = sctp_get_af_specific(daddr->sa.sa_family);
+ if (!af)
+ return -EINVAL;
+
+ err = security_sctp_bind_connect(sk, SCTP_SENDMSG_CONNECT,
+ (struct sockaddr *)daddr,
+ af->sockaddr_len);
+ if (err < 0)
+ return err;
+
asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
if (!asoc)
return -ENOMEM;
Xin Long (9):
sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg
sctp: factor out sctp_sendmsg_new_asoc from sctp_sendmsg
sctp: factor out sctp_sendmsg_check_sflags from sctp_sendmsg
sctp: factor out sctp_sendmsg_get_daddr from sctp_sendmsg
sctp: factor out sctp_sendmsg_parse from sctp_sendmsg
sctp: factor out sctp_sendmsg_update_sinfo from sctp_sendmsg
sctp: remove the unnecessary transport looking up from sctp_sendmsg
sctp: improve some variables in sctp_sendmsg
sctp: adjust some codes in a better order in sctp_sendmsg
net/sctp/socket.c | 638 +++++++++++++++++++++++-------------------------------
1 file changed, 274 insertions(+), 364 deletions(-)
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 1/9] sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg 2018-03-01 15:05 [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 2/9] sctp: factor out sctp_sendmsg_new_asoc " Xin Long 2018-03-01 18:09 ` [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Neil Horman ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 bf271f8..183129e 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 -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 2/9] sctp: factor out sctp_sendmsg_new_asoc from sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 1/9] sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 3/9] sctp: factor out sctp_sendmsg_check_sflags " Xin Long 0 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 183129e..58bb55d 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; - 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; + err = sctp_sendmsg_new_asoc(sk, sinfo_flags, &cmsgs, &to, + &transport); + if (err) 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. */ -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 3/9] sctp: factor out sctp_sendmsg_check_sflags from sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 2/9] sctp: factor out sctp_sendmsg_new_asoc " Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 4/9] sctp: factor out sctp_sendmsg_get_daddr " Xin Long 0 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 58bb55d..93cff99 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1687,6 +1687,39 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, 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 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc, 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; - 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; + err = sctp_sendmsg_check_sflags(asoc, sinfo_flags, msg, + msg_len); + if (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? */ -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 4/9] sctp: factor out sctp_sendmsg_get_daddr from sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 3/9] sctp: factor out sctp_sendmsg_check_sflags " Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 5/9] sctp: factor out sctp_sendmsg_parse " Xin Long 0 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 93cff99..68691d2 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1814,14 +1814,35 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc, 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; -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 5/9] sctp: factor out sctp_sendmsg_parse from sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 4/9] sctp: factor out sctp_sendmsg_get_daddr " Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 6/9] sctp: factor out sctp_sendmsg_update_sinfo " Xin Long 0 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 68691d2..bf089e5 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 (!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. - */ - sinfo->sinfo_timetolive = asoc->default_timetolive; + 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); } - /* 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 (!cmsgs.srinfo) + sinfo->sinfo_timetolive = asoc->default_timetolive; /* If an address is passed with the sendto/sendmsg call, it is used * to override the primary destination address in the TCP model, or -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 6/9] sctp: factor out sctp_sendmsg_update_sinfo from sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 5/9] sctp: factor out sctp_sendmsg_parse " Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 7/9] sctp: remove the unnecessary transport looking up " Xin Long 0 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem This patch is to move the codes for trying to get sinfo from asoc into sctp_sendmsg_update_sinfo. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- 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 bf089e5..bd1a657 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 -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 7/9] sctp: remove the unnecessary transport looking up from sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 6/9] sctp: factor out sctp_sendmsg_update_sinfo " Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 8/9] sctp: improve some variables in sctp_sendmsg Xin Long 0 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 bd1a657..4260310 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: -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 8/9] sctp: improve some variables in sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 7/9] sctp: remove the unnecessary transport looking up " Xin Long @ 2018-03-01 15:05 ` Xin Long 2018-03-01 15:05 ` [PATCH net-next 9/9] sctp: adjust some codes in a better order " Xin Long 0 siblings, 1 reply; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 4260310..a1c78fc1 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); } -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 9/9] sctp: adjust some codes in a better order in sctp_sendmsg 2018-03-01 15:05 ` [PATCH net-next 8/9] sctp: improve some variables in sctp_sendmsg Xin Long @ 2018-03-01 15:05 ` Xin Long 0 siblings, 0 replies; 13+ messages in thread From: Xin Long @ 2018-03-01 15:05 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem 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 <lucien.xin@gmail.com> --- 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 a1c78fc1..7fa7603 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); -- 2.1.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/9] sctp: clean up sctp_sendmsg 2018-03-01 15:05 [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Xin Long 2018-03-01 15:05 ` [PATCH net-next 1/9] sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg Xin Long @ 2018-03-01 18:09 ` Neil Horman 2018-03-01 18:34 ` Marcelo Ricardo Leitner 2018-03-04 18:02 ` David Miller 3 siblings, 0 replies; 13+ messages in thread From: Neil Horman @ 2018-03-01 18:09 UTC (permalink / raw) To: Xin Long; +Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem On Thu, Mar 01, 2018 at 11:05:09PM +0800, Xin Long wrote: > This cleanup mostly does three things: > > - extract some codes into functions to make sendmsg more readable. > > - tidy up some codes to avoid the unnecessary checks. > > - adjust some logic so that it will be easier to add the send flags > and cmsgs features that I will post after this. > > To make it easy to review and to check if the code is compatible with > before, this patchset is to do it step by step in 9 patches. > > NOTE: > There will be a conflict when merging > Commit 2277c7cd75e3 ("sctp: Add LSM hooks") from selinux tree, > the solution is to: > > 1. remove all the lines in [B]: > > <<<<<<< HEAD > [A] > ======= > [B] > >>>>>>> 2277c7c... sctp: Add LSM hooks > > 2. and apply the following diff-output: > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 980621e..d6803c8 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1686,6 +1686,7 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, > struct net *net = sock_net(sk); > struct sctp_association *asoc; > enum sctp_scope scope; > + struct sctp_af *af; > int err = -EINVAL; > > *tp = NULL; > @@ -1711,6 +1712,22 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, > > scope = sctp_scope(daddr); > > + /* Label connection socket for first association 1-to-many > + * style for client sequence socket()->sendmsg(). This > + * needs to be done before sctp_assoc_add_peer() as that will > + * set up the initial packet that needs to account for any > + * security ip options (CIPSO/CALIPSO) added to the packet. > + */ > + af = sctp_get_af_specific(daddr->sa.sa_family); > + if (!af) > + return -EINVAL; > + > + err = security_sctp_bind_connect(sk, SCTP_SENDMSG_CONNECT, > + (struct sockaddr *)daddr, > + af->sockaddr_len); > + if (err < 0) > + return err; > + > asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL); > if (!asoc) > return -ENOMEM; > > Xin Long (9): > sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg > sctp: factor out sctp_sendmsg_new_asoc from sctp_sendmsg > sctp: factor out sctp_sendmsg_check_sflags from sctp_sendmsg > sctp: factor out sctp_sendmsg_get_daddr from sctp_sendmsg > sctp: factor out sctp_sendmsg_parse from sctp_sendmsg > sctp: factor out sctp_sendmsg_update_sinfo from sctp_sendmsg > sctp: remove the unnecessary transport looking up from sctp_sendmsg > sctp: improve some variables in sctp_sendmsg > sctp: adjust some codes in a better order in sctp_sendmsg > > net/sctp/socket.c | 638 +++++++++++++++++++++++------------------------------- > 1 file changed, 274 insertions(+), 364 deletions(-) > > -- > 2.1.0 > > Series Acked-by: Neil Horman <nhorman@tuxdriver.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/9] sctp: clean up sctp_sendmsg 2018-03-01 15:05 [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Xin Long 2018-03-01 15:05 ` [PATCH net-next 1/9] sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg Xin Long 2018-03-01 18:09 ` [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Neil Horman @ 2018-03-01 18:34 ` Marcelo Ricardo Leitner 2018-03-04 18:02 ` David Miller 3 siblings, 0 replies; 13+ messages in thread From: Marcelo Ricardo Leitner @ 2018-03-01 18:34 UTC (permalink / raw) To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem Hi, I won't be able to review this one. Thanks Neil for reviewing it this quick. Marcelo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/9] sctp: clean up sctp_sendmsg 2018-03-01 15:05 [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Xin Long ` (2 preceding siblings ...) 2018-03-01 18:34 ` Marcelo Ricardo Leitner @ 2018-03-04 18:02 ` David Miller 3 siblings, 0 replies; 13+ messages in thread From: David Miller @ 2018-03-04 18:02 UTC (permalink / raw) To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman From: Xin Long <lucien.xin@gmail.com> Date: Thu, 1 Mar 2018 23:05:09 +0800 > This cleanup mostly does three things: > > - extract some codes into functions to make sendmsg more readable. > > - tidy up some codes to avoid the unnecessary checks. > > - adjust some logic so that it will be easier to add the send flags > and cmsgs features that I will post after this. > > To make it easy to review and to check if the code is compatible with > before, this patchset is to do it step by step in 9 patches. > > NOTE: > There will be a conflict when merging > Commit 2277c7cd75e3 ("sctp: Add LSM hooks") from selinux tree, > the solution is to: > > 1. remove all the lines in [B]: > > <<<<<<< HEAD > [A] > ======= > [B] > >>>>>>> 2277c7c... sctp: Add LSM hooks > > 2. and apply the following diff-output: ... Series applied, thank you. In particular, thanks for the merge resolution details. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-03-04 18:02 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-01 15:05 [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Xin Long 2018-03-01 15:05 ` [PATCH net-next 1/9] sctp: factor out sctp_sendmsg_to_asoc from sctp_sendmsg Xin Long 2018-03-01 15:05 ` [PATCH net-next 2/9] sctp: factor out sctp_sendmsg_new_asoc " Xin Long 2018-03-01 15:05 ` [PATCH net-next 3/9] sctp: factor out sctp_sendmsg_check_sflags " Xin Long 2018-03-01 15:05 ` [PATCH net-next 4/9] sctp: factor out sctp_sendmsg_get_daddr " Xin Long 2018-03-01 15:05 ` [PATCH net-next 5/9] sctp: factor out sctp_sendmsg_parse " Xin Long 2018-03-01 15:05 ` [PATCH net-next 6/9] sctp: factor out sctp_sendmsg_update_sinfo " Xin Long 2018-03-01 15:05 ` [PATCH net-next 7/9] sctp: remove the unnecessary transport looking up " Xin Long 2018-03-01 15:05 ` [PATCH net-next 8/9] sctp: improve some variables in sctp_sendmsg Xin Long 2018-03-01 15:05 ` [PATCH net-next 9/9] sctp: adjust some codes in a better order " Xin Long 2018-03-01 18:09 ` [PATCH net-next 0/9] sctp: clean up sctp_sendmsg Neil Horman 2018-03-01 18:34 ` Marcelo Ricardo Leitner 2018-03-04 18:02 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).