Commit 642dd4d3 authored by Xin Long's avatar Xin Long Committed by Kleber Sacilotto de Souza

sctp: use sk_wmem_queued to check for writable space

sk->sk_wmem_queued is used to count the size of chunks in out queue
while sk->sk_wmem_alloc is for counting the size of chunks has been
sent. sctp is increasing both of them before enqueuing the chunks,
and using sk->sk_wmem_alloc to check for writable space.

However, sk_wmem_alloc is also increased by 1 for the skb allocked
for sending in sctp_packet_transmit() but it will not wake up the
waiters when sk_wmem_alloc is decreased in this skb's destructor.

If msg size is equal to sk_sndbuf and sendmsg is waiting for sndbuf,
the check 'msg_len <= sctp_wspace(asoc)' in sctp_wait_for_sndbuf()
will keep waiting if there's a skb allocked in sctp_packet_transmit,
and later even if this skb got freed, the waiting thread will never
get waked up.

This issue has been there since very beginning, so we change to use
sk->sk_wmem_queued to check for writable space as sk_wmem_queued is
not increased for the skb allocked for sending, also as TCP does.

SOCK_SNDBUF_LOCK check is also removed here as it's for tx buf auto
tuning which I will add in another patch.
Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>

CVE-2019-3874

(backported from commit cd305c74)
[tyhicks: Backport to 4.4:
 - sctp_sendmsg_to_asoc() does not yet exist and its code is still in
   sctp_sendmsg()
 - sctp_sendmsg() has slight context differences due to timeo being
   unconditionally assigned
 - Minor context differences due to a different #include line
 - sctp_sendmsg() doesn't call sctp_prsctp_prune() due to missing commit
   8dbdf1f5 ("sctp: implement prsctp PRIO policy")]
Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
Acked-by: default avatarStefan Bader <stefan.bader@canonical.com>
Acked-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent 0857f0fc
......@@ -80,7 +80,7 @@
#include <net/sctp/sm.h>
/* Forward declarations for internal helper functions. */
static int sctp_writeable(struct sock *sk);
static bool sctp_writeable(struct sock *sk);
static void sctp_wfree(struct sk_buff *skb);
static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
size_t msg_len);
......@@ -115,25 +115,10 @@ static void sctp_enter_memory_pressure(struct sock *sk)
/* Get the sndbuf space available at the time on the association. */
static inline int sctp_wspace(struct sctp_association *asoc)
{
int amt;
struct sock *sk = asoc->base.sk;
if (asoc->ep->sndbuf_policy)
amt = asoc->sndbuf_used;
else
amt = sk_wmem_alloc_get(asoc->base.sk);
if (amt >= asoc->base.sk->sk_sndbuf) {
if (asoc->base.sk->sk_userlocks & SOCK_SNDBUF_LOCK)
amt = 0;
else {
amt = sk_stream_wspace(asoc->base.sk);
if (amt < 0)
amt = 0;
}
} else {
amt = asoc->base.sk->sk_sndbuf - amt;
}
return amt;
return asoc->ep->sndbuf_policy ? sk->sk_sndbuf - asoc->sndbuf_used
: sk_stream_wspace(sk);
}
/* Increment the used sndbuf space count of the corresponding association by
......@@ -1952,7 +1937,7 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
}
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
if (!sctp_wspace(asoc)) {
if (sctp_wspace(asoc) <= 0) {
/* sk can be changed by peel off when waiting for buf. */
err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);
if (err) {
......@@ -7005,7 +6990,7 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
goto do_error;
if (signal_pending(current))
goto do_interrupted;
if (msg_len <= sctp_wspace(asoc))
if ((int)msg_len <= sctp_wspace(asoc))
break;
/* Let another process have a go. Since we are going
......@@ -7080,14 +7065,9 @@ void sctp_write_space(struct sock *sk)
* UDP-style sockets or TCP-style sockets, this code should work.
* - Daisy
*/
static int sctp_writeable(struct sock *sk)
static bool sctp_writeable(struct sock *sk)
{
int amt = 0;
amt = sk->sk_sndbuf - sk_wmem_alloc_get(sk);
if (amt < 0)
amt = 0;
return amt;
return sk->sk_sndbuf > sk->sk_wmem_queued;
}
/* Wait for an association to go into ESTABLISHED state. If timeout is 0,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment