Skip to content

Fix S3 CRT async request lifetime - #3944

Open
FranciscoMaxwell wants to merge 1 commit into
aws:mainfrom
FranciscoMaxwell:fix-s3crt-request-lifetime
Open

FranciscoMaxwell wants to merge 1 commit into
aws:mainfrom
FranciscoMaxwell:fix-s3crt-request-lifetime

Conversation

@FranciscoMaxwell

Copy link
Copy Markdown

Issue #, if available:
Fixes #3881

Description of changes:
This updates the S3 CRT async callback state to keep the original service request alive until the CRT shutdown callback runs.

Previously, CrtRequestCallbackUserData stored originalRequest as a raw pointer to the request reference passed into CopyObjectAsync, GetObjectAsync, and PutObjectAsync. Those async methods can return before the CRT callbacks finish, leaving headers/body/progress/shutdown callbacks with a dangling pointer when they access GetContinueRequestHandler() or pass the typed request to the user response handler.

The callback data now owns a shared_ptr copy of the request. InitCommonCrtRequestOption receives and stores that shared_ptr, and the shutdown callbacks use the owned request when invoking the user handler.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (No new unit test was added; this lifetime issue depends on the async CRT request lifecycle. I verified the affected target builds successfully.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

Testing:

  • git diff --check
  • cmake -S . -B build-s3crt -DBUILD_ONLY=s3-crt -DENABLE_TESTING=OFF -DAUTORUN_UNIT_TESTS=OFF
  • cmake --build build-s3crt --config Release --target aws-cpp-sdk-s3-crt -- /m

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SIGSEGV in S3CrtRequestHeadersCallback — use-after-free reading ContinueRequestHandler from destroyed request

1 participant