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

m) Fix: read_ctx maybe accessed after free #43

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

wangfuyu
Copy link
Contributor

@wangfuyu wangfuyu commented Aug 15, 2022

read_ctx maybe accessed after free in the following situation:

  1. First: qdec_header_process got LQRHS_BLOCKED(like: Required Insert Count not enough, here we assume RIC is 17 and now Insert Count only 16) from qdec_read_header, and then stash_blocked_header, which means that read_ctx will be inserted to dec->qpd_blocked_headers[id]
  2. Second: qdec_header_process got LSRHS_ERROR(like: QPACK decompression error), then it call qdec_remove_header_block and free read_ctx, but read_ctx still be retained in qpd_blocked_headers list
  3. Third: RIC 17 is ready, then it will call qdec_process_blocked_headers, and here above read_ctx will be accessed again, but its memory is invalid, which will be coredump.

@wangfuyu
Copy link
Contributor Author

wangfuyu commented Aug 15, 2022

@litespeedtech @gwanglst Please check this PR, thanks!

read_ctx maybe accessed after free in the following situation:
1) First: qdec_header_process got LQRHS_BLOCKED(like: Required Insert Count not enough, here we assume RIC is 17) from qdec_read_header, and then stash_blocked_header, which means that read_ctx will be inserted to dec->qpd_blocked_headers[id]
2) Second: qdec_header_process got LSRHS_ERROR(like: QPACK decompression error), then it call qdec_remove_header_block and free read_ctx, but read_ctx still be retained in qpd_blocked_headers list
3) Third: RIC 17 is ready, then it will call qdec_process_blocked_headers, and here above read_ctx will be accessed again, but its memory is invalid, which will be coredump.
@litespeedtech litespeedtech merged commit b97e714 into litespeedtech:master Aug 15, 2022
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.

2 participants