issue: 4347777 Replace thread-local dummy lock

The thread-local dummy locker in ring_slave could cause use-after-free
issues during XLIO shutdown when one thread attempts to access
a socket's locker that was created by a terminated thread.
This occurs because the thread-local object is freed
when its creator thread terminates.

Replace the thread-local dummy locker with a global one to prevent this
issue. To maintain data path performance, optimize the dummy lock for
a different cache-line to prevent false sharing by aligning the lock on
a 64-byte boundary.

Signed-off-by: Tomer Cabouly <[email protected]>

Author: tomerdbz

src/core/dev/ring_slave.cpp

@@ -49,13 +49,13 @@
 // AF_INET address 0.0.0.0:0, used for 3T flow spec keys.
 static const sock_addr s_sock_addrany;
 
-static thread_local lock_dummy t_lock_dummy_ring;
+static padded_lock_dummy g_lock_dummy_ring;
 
 static lock_base *get_new_lock(const char *name, bool real_lock)
 {
     return (real_lock
                 ? static_cast<lock_base *>(multilock::create_new_lock(MULTILOCK_RECURSIVE, name))
-                : static_cast<lock_base *>(&t_lock_dummy_ring));
+                : static_cast<lock_base *>(&g_lock_dummy_ring.lock));
 }
 
 ring_slave::ring_slave(int if_index, ring *parent, ring_type_t type, bool use_locks)

src/core/sock/sockinfo_tcp.cpp

@@ -85,7 +85,8 @@ extern global_stats_t g_global_stat_static;
 tcp_timers_collection *g_tcp_timers_collection = nullptr;
 thread_local thread_local_tcp_timers g_thread_local_tcp_timers;
 bind_no_port *g_bind_no_port = nullptr;
-static thread_local lock_dummy t_lock_dummy_socket;
+
+static padded_lock_dummy g_lock_dummy_socket;
 
 /*
  * The following socket options are inherited by a connected TCP socket from the listening socket:
@@ -165,7 +166,7 @@ static lock_base *get_new_tcp_lock()
     return (
         safe_mce_sys().tcp_ctl_thread != option_tcp_ctl_thread::CTL_THREAD_DELEGATE_TCP_TIMERS
             ? static_cast<lock_base *>(multilock::create_new_lock(MULTILOCK_RECURSIVE, "tcp_con"))
-            : static_cast<lock_base *>(&t_lock_dummy_socket));
+            : static_cast<lock_base *>(&g_lock_dummy_socket.lock));
 }
 
 inline void sockinfo_tcp::lwip_pbuf_init_custom(mem_buf_desc_t *p_desc)

src/utils/lock_wrapper.h

@@ -1,35 +1,7 @@
 /*
  * SPDX-FileCopyrightText: NVIDIA CORPORATION & AFFILIATES
- * Copyright (c) 2001-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
- * SPDX-License-Identifier: GPL-2.0-only or BSD-3-Clause
- *
- * This software is available to you under a choice of one of two
- * licenses.  You may choose to be licensed under the terms of the GNU
- * General Public License (GPL) Version 2, available from the file
- * COPYING in the main directory of this source tree, or the
- * BSD license below:
- *
- *     Redistribution and use in source and binary forms, with or
- *     without modification, are permitted provided that the following
- *     conditions are met:
- *
- *      - Redistributions of source code must retain the above
- *        copyright notice, this list of conditions and the following
- *        disclaimer.
- *
- *      - Redistributions in binary form must reproduce the above
- *        copyright notice, this list of conditions and the following
- *        disclaimer in the documentation and/or other materials
- *        provided with the distribution.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
+ * Copyright (c) 2021-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ * SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
  */
 
 #ifndef LOCK_WRAPPER_H
@@ -501,6 +473,14 @@ class lock_dummy : public lock_base {
     int is_locked_by_me() override { return 1; }
 };
 
+// Users of lock_dummy may wish to alignas(64) to place this lock in in a different cache-line and
+// prevent false sharing
+struct alignas(64) padded_lock_dummy {
+    lock_dummy lock;
+    // Padding to fill a full cache line
+    char padding[64 - sizeof(lock_dummy)];
+};
+
 static inline void lock_deleter_func(lock_base *lock)
 {
     lock->delete_obj();
@@ -550,4 +530,4 @@ class multilock {
     std::unique_ptr<lock_base, lock_deleter> m_lock;
 };
 
-#endif // LOCK_WRAPPER_H
+#endif // LOCK_WRAPPER_H

tests/gtest/tcp/tcp_socket.cc

@@ -37,7 +37,7 @@
 #include "common/sys.h"
 #include "common/base.h"
 #include "tcp_base.h"
-
+#include <thread>
 class tcp_socket : public tcp_base {};
 
 /**
@@ -153,3 +153,28 @@ TEST_F(tcp_socket, ti_2_ipv6only_listen_all)
     test_lambda(true);
     test_lambda(false);
 }
+
+/**
+ * @test tcp_socket.ti_3_socket_closed_different_thread_works
+ * @brief
+ *    Test that a socket can be closed after its creator thread terminates
+ * @details
+ *    Creates a socket in a separate thread, then closes it from the main thread
+ *    after the creator thread has terminated. This verifies that socket cleanup
+ *    works correctly across thread boundaries.
+ */
+TEST_F(tcp_socket, ti_3_socket_closed_different_thread_works)
+{
+    int fd = -1;
+
+    std::thread t([&fd]() {
+        fd = socket(m_family, SOCK_STREAM, IPPROTO_IP);
+        EXPECT_LE(0, fd);
+        EXPECT_EQ(errno, EOK);
+    });
+
+    t.join();
+
+    EXPECT_LE(0, fd);
+    close(fd);
+}