summary | log | graphiclog1 | graphiclog2 | commit | commitdiff | tree | refs | edit | fork
raw | patch | inline | side by side (parent: 36c64b3)
Fix `heap-use-after-free` in `free_fast_fallback_getaddrinfo_entry` (#13231)
authorMisaki Shioi <[email protected]>
Sat, 3 May 2025 12:39:57 +0000 (3 21:39 +0900)
committerGitHub <[email protected]>
Sat, 3 May 2025 12:39:57 +0000 (3 21:39 +0900)
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.

ext/socket/ipsocket.c patch | blob | blame | history
ext/socket/raddrinfo.c patch | blob | blame | history

diff --git a/ext/socket/ipsocket.c b/ext/socket/ipsocket.c
index 4dfd9c8..da42fbd 100644 (file)
--- 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;
         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++) {
 
         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) {
                 }
             }
             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++) {
         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);
         }
         if (getaddrinfo_shared && shared_need_free) {
             free_fast_fallback_getaddrinfo_shared(&getaddrinfo_shared);
diff --git a/ext/socket/raddrinfo.c b/ext/socket/raddrinfo.c
index ac47b5b..91e2be1 100644 (file)
--- a/ext/socket/raddrinfo.c
+++ b/ext/socket/raddrinfo.c
@@ -3038,22 +3038,13 @@ free_fast_fallback_getaddrinfo_shared(struct fast_fallback_getaddrinfo_shared **
     *shared = NULL;
 }
 
     *shared = NULL;
 }
 
-void
-free_fast_fallback_getaddrinfo_entry(struct fast_fallback_getaddrinfo_entry **entry)
-{
-    if ((*entry)->ai) {
-        freeaddrinfo((*entry)->ai);
-        (*entry)->ai = NULL;
-    }
-    *entry = NULL;
-}
-
 static void *
 do_fast_fallback_getaddrinfo(void *ptr)
 {
     struct fast_fallback_getaddrinfo_entry *entry = (struct fast_fallback_getaddrinfo_entry *)ptr;
     struct fast_fallback_getaddrinfo_shared *shared = entry->shared;
 static void *
 do_fast_fallback_getaddrinfo(void *ptr)
 {
     struct fast_fallback_getaddrinfo_entry *entry = (struct fast_fallback_getaddrinfo_entry *)ptr;
     struct fast_fallback_getaddrinfo_shared *shared = entry->shared;
-    int err = 0, need_free = 0, shared_need_free = 0;
+    int err = 0, shared_need_free = 0;
+    struct addrinfo *ai = NULL;
 
     sigset_t set;
     sigemptyset(&set);
 
     sigset_t set;
     sigemptyset(&set);
@@ -3102,14 +3093,15 @@ do_fast_fallback_getaddrinfo(void *ptr)
             entry->err = errno;
             entry->has_syserr = true;
         }
             entry->err = errno;
             entry->has_syserr = true;
         }
-        if (--(entry->refcount) == 0) need_free = 1;
+        if (--(entry->refcount) == 0) {
+            ai = entry->ai;
+            entry->ai = NULL;
+        }
         if (--(shared->refcount) == 0) shared_need_free = 1;
     }
     rb_nativethread_lock_unlock(&shared->lock);
 
         if (--(shared->refcount) == 0) shared_need_free = 1;
     }
     rb_nativethread_lock_unlock(&shared->lock);
 
-    if (need_free && entry) {
-        free_fast_fallback_getaddrinfo_entry(&entry);
-    }
+    if (ai) freeaddrinfo(ai);
     if (shared_need_free && shared) {
         free_fast_fallback_getaddrinfo_shared(&shared);
     }
     if (shared_need_free && shared) {
         free_fast_fallback_getaddrinfo_shared(&shared);
     }
The Ruby Programming Language