From de4adefafb22889ef3ebfaa55b08b537334c5571 Mon Sep 17 00:00:00 2001 From: vyzo Date: Tue, 28 Aug 2018 21:07:09 +0300 Subject: [PATCH 1/3] fix data race in ownership of RPC when piggybacking control messages. --- comm.go | 16 ++++++++++++++++ gossipsub.go | 9 +++++++++ 2 files changed, 25 insertions(+) diff --git a/comm.go b/comm.go index 182e992..9c7487a 100644 --- a/comm.go +++ b/comm.go @@ -122,3 +122,19 @@ func rpcWithControl(msgs []*pb.Message, }, } } + +func copyRPC(rpc *RPC) *RPC { + return &RPC{ + RPC: pb.RPC{ + Subscriptions: rpc.Subscriptions, + Publish: rpc.Publish, + Control: &pb.ControlMessage{ + Ihave: rpc.Control.Ihave, + Iwant: rpc.Control.Iwant, + Graft: rpc.Control.Graft, + Prune: rpc.Control.Prune, + }, + }, + from: rpc.from, + } +} diff --git a/gossipsub.go b/gossipsub.go index 700120b..563b94e 100644 --- a/gossipsub.go +++ b/gossipsub.go @@ -308,9 +308,14 @@ func (gs *GossipSubRouter) sendPrune(p peer.ID, topic string) { } func (gs *GossipSubRouter) sendRPC(p peer.ID, out *RPC) { + // do we own the RPC? + own := false + // piggyback cotrol message retries ctl, ok := gs.control[p] if ok { + out = copyRPC(out) + own = true gs.piggybackControl(p, out, ctl) delete(gs.control, p) } @@ -318,6 +323,10 @@ func (gs *GossipSubRouter) sendRPC(p peer.ID, out *RPC) { // piggyback gossip ihave, ok := gs.gossip[p] if ok { + if !own { + out = copyRPC(out) + own = true + } gs.piggybackGossip(p, out, ihave) delete(gs.gossip, p) } From 73b338aa6dbca0dfab73fe9d185774707b2e06e9 Mon Sep 17 00:00:00 2001 From: vyzo Date: Tue, 28 Aug 2018 21:17:00 +0300 Subject: [PATCH 2/3] fix nil pointer dereference in copyRPC --- comm.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/comm.go b/comm.go index 9c7487a..f679fc4 100644 --- a/comm.go +++ b/comm.go @@ -124,17 +124,20 @@ func rpcWithControl(msgs []*pb.Message, } func copyRPC(rpc *RPC) *RPC { - return &RPC{ + res := &RPC{ RPC: pb.RPC{ Subscriptions: rpc.Subscriptions, Publish: rpc.Publish, - Control: &pb.ControlMessage{ - Ihave: rpc.Control.Ihave, - Iwant: rpc.Control.Iwant, - Graft: rpc.Control.Graft, - Prune: rpc.Control.Prune, - }, }, from: rpc.from, } + if rpc.Control != nil { + res.Control = &pb.ControlMessage{ + Ihave: rpc.Control.Ihave, + Iwant: rpc.Control.Iwant, + Graft: rpc.Control.Graft, + Prune: rpc.Control.Prune, + } + } + return res } From a3003697f666db06486b801fc9b3ed0bdf3305a6 Mon Sep 17 00:00:00 2001 From: vyzo Date: Tue, 28 Aug 2018 21:35:00 +0300 Subject: [PATCH 3/3] more idiomatic copyRPC --- comm.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/comm.go b/comm.go index f679fc4..fded9a8 100644 --- a/comm.go +++ b/comm.go @@ -124,20 +124,11 @@ func rpcWithControl(msgs []*pb.Message, } func copyRPC(rpc *RPC) *RPC { - res := &RPC{ - RPC: pb.RPC{ - Subscriptions: rpc.Subscriptions, - Publish: rpc.Publish, - }, - from: rpc.from, - } + res := new(RPC) + *res = *rpc if rpc.Control != nil { - res.Control = &pb.ControlMessage{ - Ihave: rpc.Control.Ihave, - Iwant: rpc.Control.Iwant, - Graft: rpc.Control.Graft, - Prune: rpc.Control.Prune, - } + res.Control = new(pb.ControlMessage) + *res.Control = *rpc.Control } return res }