Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migration_notifier: get schema_ptr by value #15098

Closed

Conversation

bhalevy
Copy link
Member

@bhalevy bhalevy commented Aug 20, 2023

To prevent use-after-free as seen in
#15097
where a temp schema_ptr retrieved from a global_schema_ptr get destroyed when the notification function yielded.

Capturing the schema_ptr on the coroutine frame
is inexpensive since its a shared ptr and it makes sure that the schema remains valid throughput the coroutine life time.

Fixes #15097

To prevent use-after-free as seen in
scylladb#15097
where a temp schema_ptr retrieved from a global_schema_ptr
get destroyed when the notification function yielded.

Capturing the schema_ptr on the coroutine frame
is inexpensive since its a shared ptr and it makes sure
that the schema remains valid throughput the coroutine
life time.

Fixes scylladb#15097

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
@github-actions github-actions bot deleted a comment from aws-amplify-us-east-2 bot Aug 20, 2023
@bhalevy
Copy link
Member Author

bhalevy commented Aug 20, 2023

It looks like the caller is losing the temporary arg since it uses a regular future<> returning lambda.
Using a coroutine lambda should solve this issue too.
See #15097 (comment)

@avikivity
Copy link
Member

When did the regression start?

@tgrabiec
Copy link
Contributor

When did the regression start?

Probably 825d617.

@bhalevy
Copy link
Member Author

bhalevy commented Aug 20, 2023

When did the regression start?

Probably 825d617.

Right. Previously the arg was captured by the async lambda before the function yielded

@scylladb-promoter
Copy link
Contributor

raphaelsc pushed a commit to raphaelsc/scylla that referenced this pull request Aug 22, 2023
To prevent use-after-free as seen in
scylladb#15097
where a temp schema_ptr retrieved from a global_schema_ptr
get destroyed when the notification function yielded.

Capturing the schema_ptr on the coroutine frame
is inexpensive since its a shared ptr and it makes sure
that the schema remains valid throughput the coroutine
life time.

Fixes scylladb#15097

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Closes scylladb#15098
raphaelsc pushed a commit to raphaelsc/scylla that referenced this pull request Aug 29, 2023
To prevent use-after-free as seen in
scylladb#15097
where a temp schema_ptr retrieved from a global_schema_ptr
get destroyed when the notification function yielded.

Capturing the schema_ptr on the coroutine frame
is inexpensive since its a shared ptr and it makes sure
that the schema remains valid throughput the coroutine
life time.

Fixes scylladb#15097

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Closes scylladb#15098
bhalevy added a commit to bhalevy/scylla that referenced this pull request Oct 29, 2023
To prevent use-after-free as seen in
scylladb#15097
where a temp schema_ptr retrieved from a global_schema_ptr
get destroyed when the notification function yielded.

Capturing the schema_ptr on the coroutine frame
is inexpensive since its a shared ptr and it makes sure
that the schema remains valid throughput the coroutine
life time.

\Fixes scylladb#15097

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

\Closes scylladb#15098

(cherry picked from commit 0f54e24)
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.

use-after-free in cql3::query_processor::migration_subscriber::on_drop_column_family
4 participants