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

[release-3.0] raftstore: check merge entries' length to avoid slice out of range #5291

Closed
wants to merge 3 commits into from

Conversation

@Connor1996
Copy link
Member

Connor1996 commented Aug 16, 2019

What have you changed? (mandatory)

In merge process, target region will send entries to fall-behind source region. It is possible to propose a CompactLog command after proposing PrepareMerge. Note that, the entries involved in CommitMerge would not take the CompactLog record.

So during the merge process, if the source region catches up with the logs by raft append msgs, self.raft_group.raft.raft_log.committed - log_idx would be larger than entries.len() which leads to entries[self.raft_group.raft.raft_log.committed - log_idx..] out of slice range.

So check the len before cutting up the slice.

What are the type of changes? (mandatory)

  • Bugfix (a change which fixes an issue)

How has this PR been tested? (mandatory)

unit-test

Does this PR affect documentation (docs) or release note? (mandatory)

No

Does this PR affect tidb-ansible update? (mandatory)

No

Refer to a related PR or issue link (optional)

pingcap/tidb#11744

@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Aug 16, 2019

/release

@Connor1996 Connor1996 force-pushed the Connor1996:release-3.0 branch from d30a74d to b8b3c3a Aug 16, 2019
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Aug 16, 2019

/release

@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Aug 16, 2019

/test

@zhouqiang-cl

This comment has been minimized.

Copy link
Member

zhouqiang-cl commented Aug 17, 2019

/run-all-tests

@siddontang

This comment has been minimized.

Copy link
Contributor

siddontang commented Aug 18, 2019

no test? @Connor1996

@Connor1996 Connor1996 added the S: WIP label Aug 19, 2019
@Connor1996

This comment has been minimized.

Copy link
Member Author

Connor1996 commented Aug 19, 2019

@siddontang Already added locally, but it shows the reason is not what I thought. So this PR's fix may be incomplete, still in investigation.

Connor1996 added 2 commits Aug 21, 2019
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@Connor1996 Connor1996 added C: Raft T: BugFix and removed S: WIP labels Aug 21, 2019
@Connor1996 Connor1996 force-pushed the Connor1996:release-3.0 branch from b8b3c3a to 3ec4a7c Aug 21, 2019
@Connor1996 Connor1996 changed the title raftstore: check merge entries' length to avoid slice out of range [release-3.0] raftstore: check merge entries' length to avoid slice out of range Aug 21, 2019
@Connor1996 Connor1996 closed this Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.