Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in socket refc handling #1477

Merged
merged 1 commit into from
Jan 18, 2025
Merged

Fix crash in socket refc handling #1477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libAtomVM/globalcontext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ COLD_FUNC void globalcontext_destroy(GlobalContext *glb)
struct RefcBinary *refc = GET_LIST_ENTRY(item, struct RefcBinary, head);
#ifndef NDEBUG
if (refc->resource_type) {
fprintf(stderr, "Warning, dangling resource of type %s, ref_count = %d\n", refc->resource_type->name, (int) refc->ref_count);
fprintf(stderr, "Warning, dangling resource of type %s, ref_count = %d, data = %p\n", refc->resource_type->name, (int) refc->ref_count, refc->data);
} else {
fprintf(stderr, "Warning, dangling refc binary, ref_count = %d\n", (int) refc->ref_count);
}
Expand Down
68 changes: 42 additions & 26 deletions src/libAtomVM/otp_socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <otp_socket.h>
#include <port.h>
#include <posix_nifs.h>
#include <refc_binary.h>
#include <scheduler.h>
#include <smp.h>
#include <sys.h>
Expand Down Expand Up @@ -285,6 +286,8 @@ static void socket_stop(ErlNifEnv *caller_env, void *obj, ErlNifEvent event, int
if (rsrc_obj->selecting_process_id != INVALID_PROCESS_ID) {
enif_demonitor_process(caller_env, rsrc_obj, &rsrc_obj->selecting_process_monitor);
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
struct RefcBinary *rsrc_refc = refc_binary_from_data(rsrc_obj);
refc_binary_decrement_refcount(rsrc_refc, caller_env->global);
}

TRACE("socket_stop called on fd=%i\n", rsrc_obj->fd);
Expand All @@ -305,42 +308,43 @@ static void socket_down(ErlNifEnv *caller_env, void *obj, ErlNifPid *pid, ErlNif
TRACE("socket_down called on process_id=%i\n", (int) *pid);
#endif

// Increment the reference count so the resource doesn't go away
// (enif_select will decrement the ref count)
struct RefcBinary *rsrc_refc = refc_binary_from_data(rsrc_obj);
refc_binary_increment_refcount(rsrc_refc);
SMP_RWLOCK_WRLOCK(rsrc_obj->socket_lock);

#if OTP_SOCKET_BSD
if (rsrc_obj->selecting_process_id != INVALID_PROCESS_ID) {
// Monitor fired, so make sure we don't try to demonitor in select_stop
// as it could crash trying to reacquire lock on process table
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
enif_select(caller_env, rsrc_obj->fd, ERL_NIF_SELECT_STOP, rsrc_obj, NULL, term_nil());
if (rsrc_obj->selecting_process_id == INVALID_PROCESS_ID) {
SMP_RWLOCK_UNLOCK(rsrc_obj->socket_lock);
return;
}

#if OTP_SOCKET_BSD
// Monitor fired, so make sure we don't try to demonitor in select_stop
// as it could crash trying to reacquire lock on process table
// enif_select can decrement ref count but it's at least 2 in this case (1 for monitor and 1 for select)
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
enif_select(caller_env, rsrc_obj->fd, ERL_NIF_SELECT_STOP, rsrc_obj, NULL, term_nil());
#elif OTP_SOCKET_LWIP
// Monitor can be called when we're selecting, accepting or connecting.
if (rsrc_obj->selecting_process_id != INVALID_PROCESS_ID) {
LWIP_BEGIN();
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
if (rsrc_obj->socket_state & SocketStateTCP) {
if (rsrc_obj->socket_state & SocketStateTCPListening) {
(void) tcp_close(rsrc_obj->tcp_pcb);
} else {
tcp_abort(rsrc_obj->tcp_pcb);
}
rsrc_obj->tcp_pcb = NULL;
rsrc_obj->socket_state = SocketStateClosed;
} else if (rsrc_obj->socket_state & SocketStateUDP) {
udp_remove(rsrc_obj->udp_pcb);
rsrc_obj->udp_pcb = NULL;
rsrc_obj->socket_state = SocketStateClosed;
LWIP_BEGIN();
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
if (rsrc_obj->socket_state & SocketStateTCP) {
if (rsrc_obj->socket_state & SocketStateTCPListening) {
(void) tcp_close(rsrc_obj->tcp_pcb);
} else {
tcp_abort(rsrc_obj->tcp_pcb);
}
LWIP_END();
rsrc_obj->tcp_pcb = NULL;
rsrc_obj->socket_state = SocketStateClosed;
} else if (rsrc_obj->socket_state & SocketStateUDP) {
udp_remove(rsrc_obj->udp_pcb);
rsrc_obj->udp_pcb = NULL;
rsrc_obj->socket_state = SocketStateClosed;
}
LWIP_END();
#endif

SMP_RWLOCK_UNLOCK(rsrc_obj->socket_lock);

// We're no longer monitoring so we can decrement ref count
refc_binary_decrement_refcount(rsrc_refc, caller_env->global);
}

Expand Down Expand Up @@ -948,13 +952,16 @@ static term nif_socket_select_read(Context *ctx, int argc, term argv[])
RAISE_ERROR(BADARG_ATOM);
}

struct RefcBinary *rsrc_refc = refc_binary_from_data(rsrc_obj);
SMP_RWLOCK_WRLOCK(rsrc_obj->socket_lock);

ErlNifEnv *env = erl_nif_env_from_context(ctx);
if (rsrc_obj->selecting_process_id != ctx->process_id && rsrc_obj->selecting_process_id != INVALID_PROCESS_ID) {
// demonitor can fail if process is gone.
enif_demonitor_process(env, rsrc_obj, &rsrc_obj->selecting_process_monitor);
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
// decrement ref count as we are demonitoring
refc_binary_decrement_refcount(rsrc_refc, ctx->global);
}
// Monitor first as select is less likely to fail and it's less expensive to demonitor
// if select fails than to stop select if monitor fails
Expand All @@ -963,6 +970,8 @@ static term nif_socket_select_read(Context *ctx, int argc, term argv[])
SMP_RWLOCK_UNLOCK(rsrc_obj->socket_lock);
RAISE_ERROR(NOPROC_ATOM);
}
// increment ref count so the resource doesn't go away until monitor is fired
refc_binary_increment_refcount(rsrc_refc);
rsrc_obj->selecting_process_id = ctx->process_id;
}

Expand All @@ -979,6 +988,7 @@ static term nif_socket_select_read(Context *ctx, int argc, term argv[])
enif_demonitor_process(env, rsrc_obj, &rsrc_obj->selecting_process_monitor);
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
SMP_RWLOCK_UNLOCK(rsrc_obj->socket_lock);
refc_binary_decrement_refcount(rsrc_refc, ctx->global);
RAISE_ERROR(BADARG_ATOM);
}
}
Expand Down Expand Up @@ -1025,6 +1035,7 @@ static term nif_socket_select_read(Context *ctx, int argc, term argv[])
enif_demonitor_process(env, rsrc_obj, &rsrc_obj->selecting_process_monitor);
LWIP_END();
SMP_RWLOCK_UNLOCK(rsrc_obj->socket_lock);
refc_binary_decrement_refcount(rsrc_refc, ctx->global);
RAISE_ERROR(BADARG_ATOM);
}
LWIP_END();
Expand All @@ -1047,7 +1058,12 @@ static term nif_socket_select_stop(Context *ctx, int argc, term argv[])
}
// Avoid the race condition with select object here.
SMP_RWLOCK_WRLOCK(rsrc_obj->socket_lock);
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
if (rsrc_obj->selecting_process_id != INVALID_PROCESS_ID) {
enif_demonitor_process(erl_nif_env_from_context(ctx), rsrc_obj, &rsrc_obj->selecting_process_monitor);
rsrc_obj->selecting_process_id = INVALID_PROCESS_ID;
struct RefcBinary *rsrc_refc = refc_binary_from_data(rsrc_obj);
refc_binary_decrement_refcount(rsrc_refc, ctx->global);
}
#if OTP_SOCKET_BSD
if (UNLIKELY(enif_select(erl_nif_env_from_context(ctx), rsrc_obj->fd, ERL_NIF_SELECT_STOP, rsrc_obj, NULL, term_nil()) < 0)) {
RAISE_ERROR(BADARG_ATOM);
Expand Down
Loading