scst_dlm: Fix use-after-free by waiting for lkb destruction#368
Open
bmeagherix wants to merge 1 commit into
Open
scst_dlm: Fix use-after-free by waiting for lkb destruction#368bmeagherix wants to merge 1 commit into
bmeagherix wants to merge 1 commit into
Conversation
When scst_dlm_unlock_wait() timed out, the caller could proceed to free the containing storage of @lksb while DLM still held a reference to it in lkb->lkb_lksb. A subsequently delivered AST -- e.g. a recovery-synthesized UNLOCK_REPLY for a departed peer -- would then write sb_status into freed memory. Make scst_dlm_remove_lock() honor its contract on every path: after the convert-to-NL clean-release step, issue dlm_unlock(DLM_LKF_FORCEUNLOCK) and loop on the completion until a destroying CAST (sb_status == -DLM_EUNLOCK) is observed. FORCEUNLOCK ensures that such a CAST arrives even if an earlier operation is still in flight or a peer is departing, since dlm_recover_waiters_pre() synthesizes the reply in the latter case. Non-destroying CASTs from previously canceled converts are logged and the wait continues. Route the per-registrant teardown (scst_dlm_pr_rm_reg_ls), the remote-UA teardown (scst_dlm_rm_rem_ua_ls), and the remaining stack-allocated lksb teardown sites through scst_dlm_remove_lock() so they all observe the destroying CAST before their storage may be reused. As a side benefit, those sites gain the convert-to-NL step that preserves LVB validity on peers per the DLM EX/PW release rule.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When scst_dlm_unlock_wait() timed out, the caller could proceed to free the containing storage of @lksb while DLM still held a reference to it in lkb->lkb_lksb. A subsequently delivered AST -- e.g. a recovery-synthesized UNLOCK_REPLY for a departed peer -- would then write sb_status into freed memory.
Make scst_dlm_remove_lock() honor its contract on every path: after the convert-to-NL clean-release step, issue
dlm_unlock(DLM_LKF_FORCEUNLOCK) and loop on the completion until a destroying CAST (sb_status == -DLM_EUNLOCK) is observed. FORCEUNLOCK ensures that such a CAST arrives even if an earlier operation is still in flight or a peer is departing, since
dlm_recover_waiters_pre() synthesizes the reply in the latter case. Non-destroying CASTs from previously canceled converts are logged and the wait continues.
Route the per-registrant teardown (scst_dlm_pr_rm_reg_ls), the remote-UA teardown (scst_dlm_rm_rem_ua_ls), and the remaining stack-allocated lksb teardown sites through scst_dlm_remove_lock() so they all observe the destroying CAST before their storage may be reused. As a side benefit, those sites gain the convert-to-NL step that preserves LVB validity on peers per the DLM EX/PW release rule.