Skip to content

Commit

Permalink
socket: fix user-after-free on name resolution error (#3471)
Browse files Browse the repository at this point in the history
When a connection attempt fails, a new thread is created to clean it up
due to some potential deadlocks. However this new thread was executed
without a reference on the rpc transport when name resolution failed,
making it use stale pointer in some cases and causing a crash.

The fix makes sure that the thread always has a valid reference.

Fixes: #3470
Change-Id: Iebff6cd95602a6cfc3f81a0c6781f20fd1a76638
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
  • Loading branch information
xhernandez authored Apr 28, 2022
1 parent 5f82b13 commit 0a22c47
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions rpc/rpc-transport/socket/src/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ ssl_setup_connection_params(rpc_transport_t *this);
struct socket_connect_error_state_ {
xlator_t *this;
rpc_transport_t *trans;
gf_boolean_t refd;
};
typedef struct socket_connect_error_state_ socket_connect_error_state_t;

Expand Down Expand Up @@ -3132,8 +3131,7 @@ socket_connect_error_cbk(void *opaque)

rpc_transport_notify(arg->trans, RPC_TRANSPORT_DISCONNECT, arg->trans);

if (arg->refd)
rpc_transport_unref(arg->trans);
rpc_transport_unref(arg->trans);

GF_FREE(opaque);
return NULL;
Expand Down Expand Up @@ -3458,7 +3456,11 @@ socket_connect(rpc_transport_t *this, int port)
arg = GF_CALLOC(1, sizeof(*arg), gf_sock_connect_error_state_t);
arg->this = THIS;
arg->trans = this;
arg->refd = refd;
if (!refd) {
/* A reference is required by the thread that will handle the
* error. */
rpc_transport_ref(this);
}
th_ret = gf_thread_create_detached(&th_id, socket_connect_error_cbk,
arg, "scleanup");
if (th_ret) {
Expand Down

0 comments on commit 0a22c47

Please sign in to comment.