From 9b6c08878e23adb7cc84bdca94d8a944b03f099e Mon Sep 17 00:00:00 2001 From: Xin Long Date: Wed, 26 Jun 2019 16:31:39 +0800 Subject: sctp: not bind the socket in sctp_connect Now when sctp_connect() is called with a wrong sa_family, it binds to a port but doesn't set bp->port, then sctp_get_af_specific will return NULL and sctp_connect() returns -EINVAL. Then if sctp_bind() is called to bind to another port, the last port it has bound will leak due to bp->port is NULL by then. sctp_connect() doesn't need to bind ports, as later __sctp_connect will do it if bp->port is NULL. So remove it from sctp_connect(). While at it, remove the unnecessary sockaddr.sa_family len check as it's already done in sctp_inet_connect. Fixes: 644fbdeacf1d ("sctp: fix the issue that flags are ignored when using kernel_connect") Reported-by: syzbot+079bf326b38072f849d9@syzkaller.appspotmail.com Signed-off-by: Xin Long Acked-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sctp/socket.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) (limited to 'net/sctp') diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 39ea0a37af09..f33aa9ee9e27 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -4816,35 +4816,17 @@ out_nounlock: static int sctp_connect(struct sock *sk, struct sockaddr *addr, int addr_len, int flags) { - struct inet_sock *inet = inet_sk(sk); struct sctp_af *af; - int err = 0; + int err = -EINVAL; lock_sock(sk); - pr_debug("%s: sk:%p, sockaddr:%p, addr_len:%d\n", __func__, sk, addr, addr_len); - /* We may need to bind the socket. */ - if (!inet->inet_num) { - if (sk->sk_prot->get_port(sk, 0)) { - release_sock(sk); - return -EAGAIN; - } - inet->inet_sport = htons(inet->inet_num); - } - /* Validate addr_len before calling common connect/connectx routine. */ - af = addr_len < offsetofend(struct sockaddr, sa_family) ? NULL : - sctp_get_af_specific(addr->sa_family); - if (!af || addr_len < af->sockaddr_len) { - err = -EINVAL; - } else { - /* Pass correct addr len to common routine (so it knows there - * is only one address being passed. - */ + af = sctp_get_af_specific(addr->sa_family); + if (af && addr_len >= af->sockaddr_len) err = __sctp_connect(sk, addr, af->sockaddr_len, flags, NULL); - } release_sock(sk); return err; -- cgit v1.2.3-55-g7522 From 4d1415811e492d9a8238f8a92dd0d51612c788e9 Mon Sep 17 00:00:00 2001 From: Marcelo Ricardo Leitner Date: Thu, 27 Jun 2019 19:48:10 -0300 Subject: sctp: fix error handling on stream scheduler initialization It allocates the extended area for outbound streams only on sendmsg calls, if they are not yet allocated. When using the priority stream scheduler, this initialization may imply into a subsequent allocation, which may fail. In this case, it was aborting the stream scheduler initialization but leaving the ->ext pointer (allocated) in there, thus in a partially initialized state. On a subsequent call to sendmsg, it would notice the ->ext pointer in there, and trip on uninitialized stuff when trying to schedule the data chunk. The fix is undo the ->ext initialization if the stream scheduler initialization fails and avoid the partially initialized state. Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set chunk transport correctly when it's a new asoc"), this bug was actually introduced on the commit I marked below. Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations") Tested-by: Xin Long Signed-off-by: Marcelo Ricardo Leitner Acked-by: Neil Horman Signed-off-by: David S. Miller --- net/sctp/stream.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'net/sctp') diff --git a/net/sctp/stream.c b/net/sctp/stream.c index 93ed07877337..25946604af85 100644 --- a/net/sctp/stream.c +++ b/net/sctp/stream.c @@ -153,13 +153,20 @@ out: int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid) { struct sctp_stream_out_ext *soute; + int ret; soute = kzalloc(sizeof(*soute), GFP_KERNEL); if (!soute) return -ENOMEM; SCTP_SO(stream, sid)->ext = soute; - return sctp_sched_init_sid(stream, sid, GFP_KERNEL); + ret = sctp_sched_init_sid(stream, sid, GFP_KERNEL); + if (ret) { + kfree(SCTP_SO(stream, sid)->ext); + SCTP_SO(stream, sid)->ext = NULL; + } + + return ret; } void sctp_stream_free(struct sctp_stream *stream) -- cgit v1.2.3-55-g7522 From 7af033010214f2c7cff31147d8970484d46cc14c Mon Sep 17 00:00:00 2001 From: Xin Long Date: Wed, 3 Jul 2019 18:20:20 +0800 Subject: sctp: count data bundling sack chunk for outctrlchunks Now all ctrl chunks are counted for asoc stats.octrlchunks and net SCTP_MIB_OUTCTRLCHUNKS either after queuing up or bundling, other than the chunk maked and bundled in sctp_packet_bundle_sack, which caused 'outctrlchunks' not consistent with 'inctrlchunks' in peer. This issue exists since very beginning, here to fix it by increasing both net SCTP_MIB_OUTCTRLCHUNKS and asoc stats.octrlchunks when sack chunk is maked and bundled in sctp_packet_bundle_sack. Reported-by: Ja Ram Jeon Signed-off-by: Xin Long Acked-by: Marcelo Ricardo Leitner Acked-by: Neil Horman Signed-off-by: David S. Miller --- net/sctp/output.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'net/sctp') diff --git a/net/sctp/output.c b/net/sctp/output.c index e0c27477788d..dbda7e7927fd 100644 --- a/net/sctp/output.c +++ b/net/sctp/output.c @@ -282,6 +282,9 @@ static enum sctp_xmit sctp_packet_bundle_sack(struct sctp_packet *pkt, sctp_chunk_free(sack); goto out; } + SCTP_INC_STATS(sock_net(asoc->base.sk), + SCTP_MIB_OUTCTRLCHUNKS); + asoc->stats.octrlchunks++; asoc->peer.sack_needed = 0; if (del_timer(timer)) sctp_association_put(asoc); -- cgit v1.2.3-55-g7522