Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eb192840 authored by Sowmini Varadhan's avatar Sowmini Varadhan Committed by David S. Miller
Browse files

RDS:TCP: Synchronize rds_tcp_accept_one with rds_send_xmit when resetting t_sock 



There is a race condition between rds_send_xmit -> rds_tcp_xmit
and the code that deals with resolution of duelling syns added
by commit 241b2719 ("RDS-TCP: Reset tcp callbacks if re-using an
outgoing socket in rds_tcp_accept_one()").

Specifically, we may end up derefencing a null pointer in rds_send_xmit
if we have the interleaving sequence:
           rds_tcp_accept_one                  rds_send_xmit

                                             conn is RDS_CONN_UP, so
    					 invoke rds_tcp_xmit

                                             tc = conn->c_transport_data
        rds_tcp_restore_callbacks
            /* reset t_sock */
    					 null ptr deref from tc->t_sock

The race condition can be avoided without adding the overhead of
additional locking in the xmit path: have rds_tcp_accept_one wait
for rds_tcp_xmit threads to complete before resetting callbacks.
The synchronization can be done in the same manner as rds_conn_shutdown().
First set the rds_conn_state to something other than RDS_CONN_UP
(so that new threads cannot get into rds_tcp_xmit()), then wait for
RDS_IN_XMIT to be cleared in the conn->c_flags indicating that any
threads in rds_tcp_xmit are done.

Fixes: 241b2719 ("RDS-TCP: Reset tcp callbacks if re-using an
outgoing socket in rds_tcp_accept_one()")
Signed-off-by: default avatarSowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: default avatarSantosh Shilimkar <santosh.shilimkar@oracle.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 42c8819b

net/rds/tcp.c

+1 −1
Original line number Diff line number Diff line
@@ -127,7 +127,7 @@ void rds_tcp_restore_callbacks(struct socket *sock, 


/*

 * This is the only path that sets tc->t_sock.  Send and receive trust that

 * it is set.  The RDS_CONN_CONNECTED bit protects those paths from being

 * it is set.  The RDS_CONN_UP bit protects those paths from being

 * called while it isn't set.

 */

void rds_tcp_set_callbacks(struct socket *sock, struct rds_connection *conn)

net/rds/tcp_listen.c

+24 −16
Original line number Diff line number Diff line
@@ -115,8 +115,17 @@ int rds_tcp_accept_one(struct socket *sock) 
	 * rds_tcp_state_change() will do that cleanup

	 */

	rs_tcp = (struct rds_tcp_connection *)conn->c_transport_data;

	if (rs_tcp->t_sock &&

	    ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {

	rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);

	if (rs_tcp->t_sock) {

		/* Need to resolve a duelling SYN between peers.

		 * We have an outstanding SYN to this peer, which may

		 * potentially have transitioned to the RDS_CONN_UP state,

		 * 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)) {

			struct sock *nsk = new_sock->sk;



			nsk->sk_user_data = NULL;
@@ -129,10 +138,9 @@ int rds_tcp_accept_one(struct socket *sock) 
			rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);

			conn->c_outgoing = 0;

		}



	rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);

	}

	rds_tcp_set_callbacks(new_sock, conn);

	rds_connect_complete(conn);

	rds_connect_complete(conn); /* marks RDS_CONN_UP */

	new_sock = NULL;

	ret = 0;