Skip to content

Commit 19260d5

Browse files
Anil KumarAnil Kumar
authored andcommitted
The Raft consensus loop took the bytes of a committed log entry (or snapshot) and called protoutil.UnmarshalBlockOrPanic(...) on them. That function does what its name says — if the bytes don't decode as a common.Block protobuf, it raises a Go panic, which terminates
the entire orderer process.
1 parent 2190a1c commit 19260d5

1 file changed

Lines changed: 24 additions & 3 deletions

File tree

orderer/consensus/etcdraft/chain.go

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,10 @@ func NewChain(
252252
var snapBlkNum uint64
253253
var cc raftpb.ConfState
254254
if s := storage.Snapshot(); !raft.IsEmptySnap(s) {
255-
b := protoutil.UnmarshalBlockOrPanic(s.Data)
255+
b, err := protoutil.UnmarshalBlock(s.Data)
256+
if err != nil {
257+
return nil, errors.Wrap(err, "failed to unmarshal block from raft snapshot")
258+
}
256259
snapBlkNum = b.Header.Number
257260
cc = s.Metadata.ConfState
258261
}
@@ -1189,7 +1192,17 @@ func (c *Chain) apply(ents []raftpb.Entry) {
11891192
break
11901193
}
11911194

1192-
block := protoutil.UnmarshalBlockOrPanic(ents[i].Data)
1195+
block, err := protoutil.UnmarshalBlock(ents[i].Data)
1196+
if err != nil {
1197+
// A committed Raft entry that does not unmarshal as a Block indicates either
1198+
// storage corruption or a byzantine proposer. Skipping the entry would advance
1199+
// appliedIndex past data that other followers may apply successfully, silently
1200+
// forking this node's state. Halt this chain so an operator can investigate;
1201+
// other channels' chains in this orderer process continue running.
1202+
c.logger.Errorf("Failed to unmarshal committed block at raft index %d: %s; halting chain", ents[i].Index, err)
1203+
go c.halt()
1204+
return
1205+
}
11931206
c.writeBlock(block, ents[i].Index)
11941207
c.Metrics.CommittedBlockNumber.Set(float64(block.Header.Number))
11951208

@@ -1250,7 +1263,15 @@ func (c *Chain) apply(ents []raftpb.Entry) {
12501263
// at position==0, ents[position].Type is ambiguous, it can be either of {raftpb.EntryNormal, raftpb.EntryConfChange}
12511264
// take a snapshot only for ents[position].Type == raftpb.EntryNormal
12521265
if c.accDataSize >= c.sizeLimit && ents[position].Type == raftpb.EntryNormal && len(ents[position].Data) > 0 {
1253-
b := protoutil.UnmarshalBlockOrPanic(ents[position].Data)
1266+
b, err := protoutil.UnmarshalBlock(ents[position].Data)
1267+
if err != nil {
1268+
// We just successfully unmarshalled and applied this same entry above; if we
1269+
// fail here, memory has been corrupted between then and now. Halt the chain
1270+
// rather than panic the orderer process.
1271+
c.logger.Errorf("Failed to unmarshal block at raft index %d while preparing snapshot: %s; halting chain", ents[position].Index, err)
1272+
go c.halt()
1273+
return
1274+
}
12541275
select {
12551276
case c.gcC <- &gc{index: c.appliedIndex, state: c.confState, data: ents[position].Data}:
12561277
c.logger.Infof("Accumulated %d bytes since last snapshot, exceeding size limit (%d bytes), "+

0 commit comments

Comments
 (0)