ljcjclljc opened a new issue, #3356:
URL: https://github.com/apache/brpc/issues/3356

   **Describe the bug**
   
   The single-stream overload of `StreamCreate(StreamId*, Controller&, const 
StreamOptions*)`
   does not check the return value of the batched overload
   `StreamCreate(StreamIds&, int, Controller&, const StreamOptions*)`.
   
   When the same `brpc::Controller` is reused to create a request stream twice, 
the batched
   overload correctly returns `-1` because `cntl._request_streams` is already 
non-empty.
   However, the single-stream wrapper still unconditionally accesses 
`request_streams[0]`,
   which is empty on failure, causing out-of-bounds access and process crash.
   
   Affected code path:
   
   - `src/brpc/stream.cpp`
   - Single-stream wrapper ignores failure from batched `StreamCreate`
   - Crash is caused by reading `request_streams[0]` after the inner call 
already failed
   
   ---
   **To Reproduce**
   
   Use the same `brpc::Controller` and call the single-stream `StreamCreate` 
twice:
   
   ```cpp
   #include "brpc/controller.h"
   #include "brpc/stream.h"
   
   int main() {
       brpc::Controller cntl;
   
       brpc::StreamId first_stream = brpc::INVALID_STREAM_ID;
       if (brpc::StreamCreate(&first_stream, cntl, NULL) != 0) {
           return 1;
       }
   
       brpc::StreamId second_stream = brpc::INVALID_STREAM_ID;
       brpc::StreamCreate(&second_stream, cntl, NULL);  // crashes before fix
       return 0;
   }
   ```
   
   A gtest reproducer also demonstrates the issue:
   
   ```cpp
   TEST_F(StreamCreateBugTest, create_twice_on_same_controller_crashes) {
       ASSERT_DEATH({
           brpc::Controller cntl;
           brpc::StreamId first_stream = brpc::INVALID_STREAM_ID;
           if (brpc::StreamCreate(&first_stream, cntl, NULL) != 0) {
               std::abort();
           }
   
           brpc::StreamId second_stream = brpc::INVALID_STREAM_ID;
           brpc::StreamCreate(&second_stream, cntl, NULL);
       }, "");
   }
   ```
   **Expected behavior**
   
   The second call should fail gracefully and return `-1` without crashing.
   
   Expected behavior:
   
   - First `StreamCreate` call succeeds
   - Second `StreamCreate` call on the same `Controller` returns `-1`
   - No out-of-bounds access
   - No process crash
   
   ---
   **Versions**
   - OS: Linux
   - Compiler: GCC 13.3.0
   - brpc: current `master` at the time of testing
   - protobuf: 3.21.12
   
   ---
   **Additional context/screenshots**
   Root cause in `src/brpc/stream.cpp`:
   
   ```cpp
   int StreamCreate(StreamId *request_stream, Controller &cntl,
                    const StreamOptions* options) {
       if (request_stream == NULL) {
           LOG(ERROR) << "request_stream is NULL";
           return -1;
       }
       StreamIds request_streams;
       StreamCreate(request_streams, 1, cntl, options);
       *request_stream = request_streams[0];
       return 0;
   }
   ```
   A minimal fix is:
   
   ```cpp
   StreamIds request_streams;
   int rc = StreamCreate(request_streams, 1, cntl, options);
   if (rc != 0) {
       return rc;
   }
   *request_stream = request_streams[0];
   return 0;
   ```
   This issue is easy to reproduce and straightforward to fix, and it can be 
covered with.


-- 
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