[PATCH net 0/2] RDS: TCP: connection spamming fixes

[PATCH net 0/2] RDS: TCP: connection spamming fixes

[Sowmini Varadhan]

We have been testing the RDS-TCP code with a connection spammer
that sends incoming SYNs to the RDS listen port well after 
an rds-tcp connection has been established, and found a few 
race-windows that are fixed by this patch series.

Patch 1 avoids a null pointer deref when an incoming SYN 
shows up when a netns is being dismantled, or when the 
rds-tcp module is being unloaded. 

Patch 2 addresses the case when a SYN is received after the
connection arbitration algorithm has converged: the incoming
SYN should not needlessly quiesce the transmit path, and it
should not result in needless TCP connection resets due to
re-execution of the connection arbitration logic.

Sowmini Varadhan (2):
  RDS: TCP: rds_tcp_accept_worker() must exit gracefully when
    terminating rds-tcp
  RDS: TCP: Avoid rds connection churn from rogue SYNs

 net/rds/tcp_listen.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

[PATCH net 1/2] RDS: TCP: rds_tcp_accept_worker() must exit gracefully when terminating rds-tcp

[Sowmini Varadhan]

There are two instances where we want to terminate RDS-TCP: when
exiting the netns or during module unload. In either case, the
termination sequence is to stop the listen socket, mark the
rtn->rds_tcp_listen_sock as null, and flush any accept workqs.
Thus any workqs that get flushed at this point will encounter a
null rds_tcp_listen_sock, and must exit gracefully to allow
the RDS-TCP termination to complete successfully.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 net/rds/tcp_listen.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
index be263cd..e10b422 100644
--- a/net/rds/tcp_listen.c
+++ b/net/rds/tcp_listen.c
@@ -80,6 +80,9 @@ int rds_tcp_accept_one(struct socket *sock)
 	int conn_state;
 	struct sock *nsk;
 
+	if (!sock) /* module unload or netns delete in progress */
+		return -ENETUNREACH;
+
 	ret = sock_create_kern(sock_net(sock->sk), sock->sk->sk_family,
 			       sock->sk->sk_type, sock->sk->sk_protocol,
 			       &new_sock);
-- 
1.7.1

Re: [PATCH net 1/2] RDS: TCP: rds_tcp_accept_worker() must exit gracefully when terminating rds-tcp

[Santosh Shilimkar]

On 5/18/2016 10:06 AM, Sowmini Varadhan wrote:
> There are two instances where we want to terminate RDS-TCP: when
> exiting the netns or during module unload. In either case, the
> termination sequence is to stop the listen socket, mark the
> rtn->rds_tcp_listen_sock as null, and flush any accept workqs.
> Thus any workqs that get flushed at this point will encounter a
> null rds_tcp_listen_sock, and must exit gracefully to allow
> the RDS-TCP termination to complete successfully.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
>  net/rds/tcp_listen.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
> index be263cd..e10b422 100644
> --- a/net/rds/tcp_listen.c
> +++ b/net/rds/tcp_listen.c
> @@ -80,6 +80,9 @@ int rds_tcp_accept_one(struct socket *sock)
>  	int conn_state;
>  	struct sock *nsk;
>
> +	if (!sock) /* module unload or netns delete in progress */
> +		return -ENETUNREACH;
> +
New error code type in RDS but seems appropriate :-)
Patch looks good to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

[PATCH net 2/2] RDS: TCP: Avoid rds connection churn from rogue SYNs

[Sowmini Varadhan]

When a rogue SYN is received after the connection arbitration
algorithm has converged, the incoming SYN should not needlessly
quiesce the transmit path, and it should not result in needless
TCP connection resets due to re-execution of the connection
arbitration logic.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 net/rds/tcp_listen.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
index e10b422..bc387c2 100644
--- a/net/rds/tcp_listen.c
+++ b/net/rds/tcp_listen.c
@@ -132,11 +132,13 @@ int rds_tcp_accept_one(struct socket *sock)
 		 * so we must quiesce any send threads before resetting
 		 * c_transport_data.
 		 */
-		wait_event(conn->c_waitq,
-			   !test_bit(RDS_IN_XMIT, &conn->c_flags));
-		if (ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
+		if (ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr) ||
+		    !conn->c_outgoing) {
 			goto rst_nsk;
-		} else if (rs_tcp->t_sock) {
+		} else {
+			atomic_set(&conn->c_state, RDS_CONN_CONNECTING);
+			wait_event(conn->c_waitq,
+				   !test_bit(RDS_IN_XMIT, &conn->c_flags));
 			rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
 			conn->c_outgoing = 0;
 		}
-- 
1.7.1

Re: [PATCH net 2/2] RDS: TCP: Avoid rds connection churn from rogue SYNs

[Santosh Shilimkar]

On 5/18/2016 10:06 AM, Sowmini Varadhan wrote:
> When a rogue SYN is received after the connection arbitration
> algorithm has converged, the incoming SYN should not needlessly
> quiesce the transmit path, and it should not result in needless
> TCP connection resets due to re-execution of the connection
> arbitration logic.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

Re: [PATCH net 0/2] RDS: TCP: connection spamming fixes

[David Miller]

From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Wed, 18 May 2016 10:06:22 -0700

> We have been testing the RDS-TCP code with a connection spammer
> that sends incoming SYNs to the RDS listen port well after 
> an rds-tcp connection has been established, and found a few 
> race-windows that are fixed by this patch series.
> 
> Patch 1 avoids a null pointer deref when an incoming SYN 
> shows up when a netns is being dismantled, or when the 
> rds-tcp module is being unloaded. 
> 
> Patch 2 addresses the case when a SYN is received after the
> connection arbitration algorithm has converged: the incoming
> SYN should not needlessly quiesce the transmit path, and it
> should not result in needless TCP connection resets due to
> re-execution of the connection arbitration logic.

Series applied, thanks.