Paths

Table of Contentst

Differential D2890

Merge #11578: net: Add missing lock in ProcessHeadersMessage(...)
ClosedPublic
Actions

Authored by jasonbcox on Apr 29 2019, 23:51.

Tags

None

Subscribers

None

Details

Reviewers

deadalnix
Fabien

Group Reviewers

Restricted Project

Commits

rSTAGING4f4f0f720352: Merge #11578: net: Add missing lock in ProcessHeadersMessage(...)
rABC4f4f0f720352: Merge #11578: net: Add missing lock in ProcessHeadersMessage(...)

Summary

2530bf2 net: Add missing lock in ProcessHeadersMessage(...) (practicalswift)

Pull request description:

Add missing lock in `ProcessHeadersMessage(...)`.

Reading the variable `mapBlockIndex` requires holding the mutex `cs_main`.

The new "Disconnect outbound peers relaying invalid headers" code added in commit 37886d5e2f9992678dea4b1bd893f4f10d61d3ad and merged as part of #11568 two days ago did not lock `cs_main` prior to accessing `mapBlockIndex`.

Tree-SHA512: b799c234be8043d036183a00bc7867bbf3bd7ffe3baa94c88529da3b3cd0571c31ed11dadfaf29c5b8498341d6d0a3c928029a43b69f3267ef263682c91563a3

Backport of Core PR11578
https://github.com/bitcoin/bitcoin/pull/11578/files

Test Plan

ninja check
make check
test_runner.py
bitcoind # sanity check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jasonbcox created this revision.Apr 29 2019, 23:51

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 29 2019, 23:51

jasonbcox edited the summary of this revision. (Show Details)Apr 29 2019, 23:52

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B5623: Diff 8302.Apr 30 2019, 00:07

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

LookupBlockIndex()'s first line is an AssertLockHeld(cs_main), so whether the lock is already held or the path is not covered by any test.

This revision now requires changes to proceed.Apr 30 2019, 12:15

jasonbcox edited the test plan for this revision. (Show Details)May 8 2019, 22:29

Rebase + added test_runner.py to test plan

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5767: Diff 8531.May 8 2019, 22:53

jasonbcox edited the summary of this revision. (Show Details)May 9 2019, 00:14

Fabien accepted this revision.May 9 2019, 06:23

This revision is now accepted and ready to land.May 9 2019, 06:23

Closed by commit rABC4f4f0f720352: Merge #11578: net: Add missing lock in ProcessHeadersMessage(...) (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by jasonbcox). · Explain WhyMay 9 2019, 16:20

This revision was automatically updated to reflect the committed changes.

jasonbcox added a commit: rSTAGING4f4f0f720352: Merge #11578: net: Add missing lock in ProcessHeadersMessage(...).Sep 18 2019, 23:25

Revision Contents
Changeset List

Path

Size

src/

net_processing.cpp

2 lines

Diff 8556

src/net_processing.cpp

Loading...