mirror/bgp-stayrtr
1
0
Fork 0
mirror of https://github.com/bgp/stayrtr.git synced 2024-05-06 15:54:54 +00:00
Code Issues

Harden ^b2a79528c5d221f46bdd766ce9c448714f3b62d5

Browse Source 
It appears that the sorting function can be prone to data races.
This commit puts a lock on that.

Tag: https://github.com/bgp/stayrtr/issues/92
This commit is contained in:
Ben Cartwright-Cox
2023-02-27 15:52:31 +00:00
parent b2a79528c5
commit 28752753e0
2 changed files with 13 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
cmd/stayrtr/stayrtr.go
View File

@ -708,7 +708,8 @@ func run() error {
log.SetLevel(lvl)
deh := &rtr.DefaultRTREventHandler{
Log: log.StandardLogger(),
Log: log.StandardLogger(),
SortLock: &sync.RWMutex{},
}
sc := rtr.ServerConfiguration{

19
lib/server.go
View File

@ -56,6 +56,7 @@ type SendableDataManager interface {
type DefaultRTREventHandler struct {
sdManager SendableDataManager
Log Logger
SortLock *sync.RWMutex
}
func (e *DefaultRTREventHandler) SetSDManager(m SendableDataManager) {
@ -81,7 +82,7 @@ func (e *DefaultRTREventHandler) RequestCache(c *Client) {
e.Log.Debugf("%v < Internal error requesting cache (does not exists)", c)
}
} else {
c.SendSDs(sessionId, serial, vrps)
c.SendSDs(sessionId, serial, vrps, e.SortLock)
if e.Log != nil {
e.Log.Debugf("%v < Sent VRPs (current serial %d, session: %d)", c, serial, sessionId)
}
@ -116,7 +117,7 @@ func (e *DefaultRTREventHandler) RequestNewVersion(c *Client, sessionId uint16,
e.Log.Debugf("%v < Sent cache reset", c)
}
} else {
c.SendSDs(sessionId, serial, vrps)
c.SendSDs(sessionId, serial, vrps, e.SortLock)
if e.Log != nil {
e.Log.Debugf("%v < Sent VRPs (current serial %d, session from client: %d)", c, serial, sessionId)
}
@ -1031,12 +1032,12 @@ func (vap *VAP) GetFlag() uint8 {
return vap.Flags
}
func (c *Client) SendSDs(sessionId uint16, serialNumber uint32, data []SendableData) {
func (c *Client) SendSDs(sessionId uint16, serialNumber uint32, data []SendableData, sortLock *sync.RWMutex) {
sortLock.Lock()
sort.Slice(data, func(i, j int) bool {
if data[i].Type() == "VRP" && data[j].Type() != "VRP" {
return false // Always send VRPs first
}
if data[i].Type() == "VRP" && data[j].Type() == "VRP" {
VRPi, oki := data[i].(*VRP)
VRPj, okj := data[j].(*VRP)
if oki && okj {
// Sort VRPs as per draft-ietf-sidrops-8210bis-10
/*
11. ROA PDU Race Minimization
@ -1056,7 +1057,6 @@ func (c *Client) SendSDs(sessionId uint16, serialNumber uint32, data []SendableD
P1 is likely to mark a BGP prefix P1 invalid. Therefore, the cache SHOULD announce the
sub-prefix P1 before the covering prefix P0.
*/
VRPi, VRPj := data[i].(*VRP), data[j].(*VRP)
CIDRSizei, _ := VRPi.Prefix.Mask.Size()
CIDRSizej, _ := VRPj.Prefix.Mask.Size()
if CIDRSizei == CIDRSizej {
@ -1070,14 +1070,17 @@ func (c *Client) SendSDs(sessionId uint16, serialNumber uint32, data []SendableD
}
return true
})
sortLock.Unlock()
pduBegin := &PDUCacheResponse{
SessionId: sessionId,
}
c.SendPDU(pduBegin)
sortLock.RLock()
for _, data := range data {
c.SendData(data.Copy())
}
sortLock.RUnlock()
pduEnd := &PDUEndOfData{
SessionId: sessionId,
SerialNumber: serialNumber,