Skip to content

Commit 2be117a

Browse files
authored
Fix heap-use-after-free in free_fast_fallback_getaddrinfo_entry (ruby#13231)
This change addresses the following ASAN error: ``` ==36597==ERROR: AddressSanitizer: heap-use-after-free on address 0x512000396ba8 at pc 0x7fcad5cbad9f bp 0x7fff19739af0 sp 0x7fff19739ae8 WRITE of size 8 at 0x512000396ba8 thread T0 [643/756] 36600=optparse/test_summary #0 0x7fcad5cbad9e in free_fast_fallback_getaddrinfo_entry /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/raddrinfo.c:3046:22 #1 0x7fcad5c9fb48 in fast_fallback_inetsock_cleanup /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/ipsocket.c:1179:17 ruby#2 0x7fcadf3b611a in rb_ensure /home/runner/work/ruby-dev-builder/ruby-dev-builder/eval.c:1081:5 ruby#3 0x7fcad5c9b44b in rsock_init_inetsock /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/ipsocket.c:1289:20 ruby#4 0x7fcad5ca22b8 in tcp_init /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/tcpsocket.c:76:12 ruby#5 0x7fcadf83ba70 in vm_call0_cfunc_with_frame /home/runner/work/ruby-dev-builder/ruby-dev-builder/./vm_eval.c:164:15 ... ``` A `struct fast_fallback_getaddrinfo_shared` is shared between the main thread and two child threads. This struct contains an array of `fast_fallback_getaddrinfo_entry`. `fast_fallback_getaddrinfo_entry` and `fast_fallback_getaddrinfo_shared` were freed separately, and if `fast_fallback_getaddrinfo_shared` was freed first and then an attempt was made to free a `fast_fallback_getaddrinfo_entry`, a `heap-use-after-free` could occur. This change avoids that possibility by separating the deallocation of the addrinfo memory held by `fast_fallback_getaddrinfo_entry` from the access and lifecycle of the `fast_fallback_getaddrinfo_entry` itself.
1 parent 36c64b3 commit 2be117a

File tree

2 files changed

+17
-21
lines changed

2 files changed

+17
-21
lines changed

ext/socket/ipsocket.c

+10-6
Original file line numberDiff line numberDiff line change
@@ -1159,13 +1159,19 @@ fast_fallback_inetsock_cleanup(VALUE v)
11591159
getaddrinfo_shared->notify = -1;
11601160

11611161
int shared_need_free = 0;
1162-
int need_free[2] = { 0, 0 };
1162+
struct addrinfo *ais[arg->family_size];
1163+
for (int i = 0; i < arg->family_size; i++) ais[i] = NULL;
11631164

11641165
rb_nativethread_lock_lock(&getaddrinfo_shared->lock);
11651166
{
11661167
for (int i = 0; i < arg->family_size; i++) {
1167-
if (arg->getaddrinfo_entries[i] && --(arg->getaddrinfo_entries[i]->refcount) == 0) {
1168-
need_free[i] = 1;
1168+
struct fast_fallback_getaddrinfo_entry *getaddrinfo_entry = arg->getaddrinfo_entries[i];
1169+
1170+
if (!getaddrinfo_entry) continue;
1171+
1172+
if (--(getaddrinfo_entry->refcount) == 0) {
1173+
ais[i] = getaddrinfo_entry->ai;
1174+
getaddrinfo_entry->ai = NULL;
11691175
}
11701176
}
11711177
if (--(getaddrinfo_shared->refcount) == 0) {
@@ -1175,9 +1181,7 @@ fast_fallback_inetsock_cleanup(VALUE v)
11751181
rb_nativethread_lock_unlock(&getaddrinfo_shared->lock);
11761182

11771183
for (int i = 0; i < arg->family_size; i++) {
1178-
if (arg->getaddrinfo_entries[i] && need_free[i]) {
1179-
free_fast_fallback_getaddrinfo_entry(&arg->getaddrinfo_entries[i]);
1180-
}
1184+
if (ais[i]) freeaddrinfo(ais[i]);
11811185
}
11821186
if (getaddrinfo_shared && shared_need_free) {
11831187
free_fast_fallback_getaddrinfo_shared(&getaddrinfo_shared);

ext/socket/raddrinfo.c

+7-15
Original file line numberDiff line numberDiff line change
@@ -3038,22 +3038,13 @@ free_fast_fallback_getaddrinfo_shared(struct fast_fallback_getaddrinfo_shared **
30383038
*shared = NULL;
30393039
}
30403040

3041-
void
3042-
free_fast_fallback_getaddrinfo_entry(struct fast_fallback_getaddrinfo_entry **entry)
3043-
{
3044-
if ((*entry)->ai) {
3045-
freeaddrinfo((*entry)->ai);
3046-
(*entry)->ai = NULL;
3047-
}
3048-
*entry = NULL;
3049-
}
3050-
30513041
static void *
30523042
do_fast_fallback_getaddrinfo(void *ptr)
30533043
{
30543044
struct fast_fallback_getaddrinfo_entry *entry = (struct fast_fallback_getaddrinfo_entry *)ptr;
30553045
struct fast_fallback_getaddrinfo_shared *shared = entry->shared;
3056-
int err = 0, need_free = 0, shared_need_free = 0;
3046+
int err = 0, shared_need_free = 0;
3047+
struct addrinfo *ai = NULL;
30573048

30583049
sigset_t set;
30593050
sigemptyset(&set);
@@ -3102,14 +3093,15 @@ do_fast_fallback_getaddrinfo(void *ptr)
31023093
entry->err = errno;
31033094
entry->has_syserr = true;
31043095
}
3105-
if (--(entry->refcount) == 0) need_free = 1;
3096+
if (--(entry->refcount) == 0) {
3097+
ai = entry->ai;
3098+
entry->ai = NULL;
3099+
}
31063100
if (--(shared->refcount) == 0) shared_need_free = 1;
31073101
}
31083102
rb_nativethread_lock_unlock(&shared->lock);
31093103

3110-
if (need_free && entry) {
3111-
free_fast_fallback_getaddrinfo_entry(&entry);
3112-
}
3104+
if (ai) freeaddrinfo(ai);
31133105
if (shared_need_free && shared) {
31143106
free_fast_fallback_getaddrinfo_shared(&shared);
31153107
}

0 commit comments

Comments
 (0)