diff options
| author | Misaki Shioi <[email protected]> | 2025-05-03 21:39:57 +0900 |
|---|---|---|
| committer | GitHub <[email protected]> | 2025-05-03 21:39:57 +0900 |
| commit | 2be117a97dc46b5f1e0c571d2de81b57905313d9 (patch) | |
| tree | 2309bd8eed541188ca38aa540f6996c7eadbbff2 /ext/socket/ipsocket.c | |
| parent | 36c64b3be83f17992137d63ffd0b94f90e24424a (diff) | |
Fix `heap-use-after-free` in `free_fast_fallback_getaddrinfo_entry` (#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
#2 0x7fcadf3b611a in rb_ensure /home/runner/work/ruby-dev-builder/ruby-dev-builder/eval.c:1081:5
#3 0x7fcad5c9b44b in rsock_init_inetsock /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/ipsocket.c:1289:20
#4 0x7fcad5ca22b8 in tcp_init /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/tcpsocket.c:76:12
#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.
Notes
Notes:
Merged-By: shioimm <[email protected]>
Diffstat (limited to 'ext/socket/ipsocket.c')
| -rw-r--r-- | ext/socket/ipsocket.c | 16 |
1 files changed, 10 insertions, 6 deletions
diff --git a/ext/socket/ipsocket.c b/ext/socket/ipsocket.c index 4dfd9c8a56..da42fbd27b 100644 --- a/ext/socket/ipsocket.c +++ b/ext/socket/ipsocket.c @@ -1159,13 +1159,19 @@ fast_fallback_inetsock_cleanup(VALUE v) getaddrinfo_shared->notify = -1; int shared_need_free = 0; - int need_free[2] = { 0, 0 }; + struct addrinfo *ais[arg->family_size]; + for (int i = 0; i < arg->family_size; i++) ais[i] = NULL; rb_nativethread_lock_lock(&getaddrinfo_shared->lock); { for (int i = 0; i < arg->family_size; i++) { - if (arg->getaddrinfo_entries[i] && --(arg->getaddrinfo_entries[i]->refcount) == 0) { - need_free[i] = 1; + struct fast_fallback_getaddrinfo_entry *getaddrinfo_entry = arg->getaddrinfo_entries[i]; + + if (!getaddrinfo_entry) continue; + + if (--(getaddrinfo_entry->refcount) == 0) { + ais[i] = getaddrinfo_entry->ai; + getaddrinfo_entry->ai = NULL; } } if (--(getaddrinfo_shared->refcount) == 0) { @@ -1175,9 +1181,7 @@ fast_fallback_inetsock_cleanup(VALUE v) rb_nativethread_lock_unlock(&getaddrinfo_shared->lock); for (int i = 0; i < arg->family_size; i++) { - if (arg->getaddrinfo_entries[i] && need_free[i]) { - free_fast_fallback_getaddrinfo_entry(&arg->getaddrinfo_entries[i]); - } + if (ais[i]) freeaddrinfo(ais[i]); } if (getaddrinfo_shared && shared_need_free) { free_fast_fallback_getaddrinfo_shared(&getaddrinfo_shared); |