chenBright opened a new pull request, #3311:
URL: https://github.com/apache/brpc/pull/3311

   ### What problem does this PR solve?
   
   Issue Number: resolve 
   
   Problem Summary:
   
   `SSLTest.ssl_sni` (and any case that runs after `SSLTest.connect_on_create`)
   can crash with `SIGSEGV`:
   
   https://github.com/chenBright/brpc/actions/runs/26140916096/job/76886067647
   
   ```shell
   Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
   Core was generated by `./brpc_ssl_unittest'.
   Program terminated with signal SIGSEGV, Segmentation fault.
   #0  0x00007f78fc6acc9d in __dynamic_cast () from 
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
   [Current thread is 1 (Thread 0x7f78f7dfb640 (LWP 43950))]
   #0  0x00007f78fc6acc9d in __dynamic_cast () from 
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
   #1  0x00007f78fd7f9be3 in brpc::Socket::OnOutputEvent (user_data=<optimized 
out>) at src/brpc/socket.cpp:1437
   #2  0x00007f78fd789cfd in brpc::IOEventData::CallOutputEventCallback 
(this=0x0, events=25, thread_attr=...) at ./src/brpc/event_dispatcher.h:77
   #3  brpc::EventDispatcher::OnEvent<false> (event_data_id=<optimized out>, 
events=25, thread_attr=...) at ./src/brpc/event_dispatcher.h:162
   #4  0x00007f78fd7893c9 in brpc::EventDispatcher::CallOutputEventCallback 
(event_data_id=140735052498784, events=4256859216, thread_attr=...) at 
./src/brpc/event_dispatcher.h:174
   #5  brpc::EventDispatcher::Run (this=<optimized out>) at 
./src/brpc/event_dispatcher_epoll.cpp:239
   #6  0x00007f78fd789049 in brpc::EventDispatcher::RunThis 
(arg=0x7fff6ecfc760) at ./src/brpc/event_dispatcher_epoll.cpp:193
   #7  0x00007f78fd6bf8a0 in bthread::TaskGroup::task_runner 
(skip_remained=<optimized out>) at src/bthread/task_group.cpp:398
   #8  0x00007f78fd693af1 in bthread_make_fcontext () from ./libbrpc.dbg.so
   #9  0x03010102464c457f in ?? ()
   #10 0x0000000000000000 in ?? ()
   
   
   Thread 3 (Thread 0x7f78fe8b3a80 (LWP 43945)):
   #0  __GI___libc_read (nbytes=4096, buf=0x7fff6ecf8590, fd=5) at 
../sysdeps/unix/sysv/linux/read.c:26
   #1  __GI___libc_read (fd=5, buf=0x7fff6ecf8590, nbytes=4096) at 
../sysdeps/unix/sysv/linux/read.c:24
   #2  0x00000000004cb7a9 in GetRawPemString[abi:cxx11](char const*) 
(fname=0x5149f3 "cert2.crt") at brpc_ssl_unittest.cpp:341
   #3  0x00000000004cb9ec in SSLTest_ssl_sni_Test::TestBody (this=0xcbae4a0) at 
brpc_ssl_unittest.cpp:362
   #4  0x000000000050c2cf in void 
testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, 
void>(testing::Test*, void (testing::Test::*)(), char const*) ()
   #5  0x0000000000500866 in testing::Test::Run() ()
   #6  0x00000000005009e5 in testing::TestInfo::Run() ()
   #7  0x0000000000500f99 in testing::TestSuite::Run() ()
   #8  0x000000000050169f in testing::internal::UnitTestImpl::RunAllTests() ()
   #9  0x000000000050c897 in bool 
testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
 bool>(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   #10 0x0000000000500aac in testing::UnitTest::Run() ()
   #11 0x00000000004ce6e1 in RUN_ALL_TESTS () at /usr/include/gtest/gtest.h:2490
   #12 0x00000000004c533f in main (argc=1, argv=0x7fff6ecfcc88) at 
brpc_ssl_unittest.cpp:50
   ```
   
   The main test thread is still preparing `ssl_sni` (executing
   `GetRawPemString("cert2.crt")`), while the background `EventDispatcher`
   thread is dispatching an EPOLL event for a **leftover socket from the
   previous case** `connect_on_create`.
   
   Root cause: `connect_on_create` stores the address of a stack-allocated
   `brpc::InputMessenger messenger;` in `socket_options.user` and then calls
   `Socket::Create(...)` but never closes the resulting client socket. When
   the test function returns:
   
   * `messenger` is destroyed, but the socket's `_user` pointer still
     references its now-invalid storage.
   * `server` is destroyed, causing the server side of the SSL connection to
     be torn down, which delivers `EPOLLHUP`/`EPOLLERR` to the client fd.
   * The fd is still registered with the global `EventDispatcher`, so
     `Socket::OnOutputEvent` is invoked. There,
     `dynamic_cast<EpollOutRequest*>(s->user())` reads the vtable of the
     destroyed `messenger` and segfaults.
   
   The crash is therefore a use-after-free on `socket_options.user`, exposed
   as a flake in subsequent SSL test cases.
   
   ### What is changed and the side effects?
   
   Changed:
   
   * `test/brpc_ssl_unittest.cpp`: at the end of
     `SSLTest.connect_on_create`, explicitly close the client socket via
     `ptr->SetFailed()` (which unregisters the fd from the
     `EventDispatcher` and schedules the socket for recycling) and then
     `Stop`/`Join` the server, matching the cleanup pattern used by every
     other `SSLTest` case. 
   
   Side effects:
   - Performance effects:
   
   - Breaking backward compatibility: 
   
   ---
   ### Check List:
   - Please make sure your changes are compilable.
   - When providing us with a new feature, it is best to add related tests.
   - Please follow [Contributor Covenant Code of 
Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to