Skip to content

Commit bbc6418

Browse files
Fix potential race condition leading to nil dereference (#105)
Split ApproveOrDeny*Limit public functions into a public part which takes a msgCounter and resolves it to a Message using the pendingLimits as before and a private part which takes a *Message and can be called directly from the internal WriteApprovalCallbacks. This removes the need to call FeatureLocalInterface.ApproveOrDenyWrite with an "empty" message when a message is not found in the pendingLimits for ApprovalRequests that don't match the current use case. Fixes #104
2 parents 9e534cd + 5c7fc4d commit bbc6418

File tree

4 files changed

+34
-40
lines changed

4 files changed

+34
-40
lines changed

usecases/cs/lpc/public.go

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"github.com/enbility/eebus-go/api"
88
"github.com/enbility/eebus-go/features/server"
99
ucapi "github.com/enbility/eebus-go/usecases/api"
10-
spineapi "github.com/enbility/spine-go/api"
1110
"github.com/enbility/spine-go/model"
1211
"github.com/enbility/spine-go/util"
1312
)
@@ -134,29 +133,13 @@ func (e *LPC) ApproveOrDenyConsumptionLimit(msgCounter model.MsgCounterType, app
134133
e.pendingMux.Lock()
135134
defer e.pendingMux.Unlock()
136135

137-
f := e.LocalEntity.FeatureOfTypeAndRole(model.FeatureTypeTypeLoadControl, model.RoleTypeServer)
138-
139-
result := model.ErrorType{
140-
ErrorNumber: model.ErrorNumberType(0),
141-
}
142-
143136
msg, ok := e.pendingLimits[msgCounter]
144137
if !ok {
145-
// it is not a limit of this usecase, so approve it
146-
newMsg := &spineapi.Message{
147-
RequestHeader: &model.HeaderType{
148-
MsgCounter: &msgCounter,
149-
},
150-
}
151-
f.ApproveOrDenyWrite(newMsg, result)
138+
// no pending limit for this msgCounter, this is a caller error
152139
return
153140
}
154141

155-
if !approve {
156-
result.ErrorNumber = model.ErrorNumberType(7)
157-
result.Description = util.Ptr(model.DescriptionType(reason))
158-
}
159-
f.ApproveOrDenyWrite(msg, result)
142+
e.approveOrDenyConsumptionLimit(msg, approve, reason)
160143

161144
delete(e.pendingLimits, msgCounter)
162145
}

usecases/cs/lpc/usecase.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,20 @@ func (e *LPC) loadControlServerAndLimitId() (lc *server.LoadControl, limitid mod
103103
return lc, *description.LimitId, nil
104104
}
105105

106+
func (e *LPC) approveOrDenyConsumptionLimit(msg *spineapi.Message, approve bool, reason string) {
107+
f := e.LocalEntity.FeatureOfTypeAndRole(model.FeatureTypeTypeLoadControl, model.RoleTypeServer)
108+
109+
result := model.ErrorType{
110+
ErrorNumber: model.ErrorNumberType(0),
111+
}
112+
113+
if !approve {
114+
result.ErrorNumber = model.ErrorNumberType(7)
115+
result.Description = util.Ptr(model.DescriptionType(reason))
116+
}
117+
f.ApproveOrDenyWrite(msg, result)
118+
}
119+
106120
// callback invoked on incoming write messages to this
107121
// loadcontrol server feature.
108122
// the implementation only considers write messages for this use case and
@@ -145,7 +159,7 @@ func (e *LPC) loadControlWriteCB(msg *spineapi.Message) {
145159
e.pendingMux.Unlock()
146160

147161
// approve, because this is no request for this usecase
148-
go e.ApproveOrDenyConsumptionLimit(*msg.RequestHeader.MsgCounter, true, "")
162+
go e.approveOrDenyConsumptionLimit(msg, true, "")
149163
}
150164

151165
func (e *LPC) AddFeatures() {

usecases/cs/lpp/public.go

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"github.com/enbility/eebus-go/api"
88
"github.com/enbility/eebus-go/features/server"
99
ucapi "github.com/enbility/eebus-go/usecases/api"
10-
spineapi "github.com/enbility/spine-go/api"
1110
"github.com/enbility/spine-go/model"
1211
"github.com/enbility/spine-go/util"
1312
)
@@ -134,29 +133,13 @@ func (e *LPP) ApproveOrDenyProductionLimit(msgCounter model.MsgCounterType, appr
134133
e.pendingMux.Lock()
135134
defer e.pendingMux.Unlock()
136135

137-
f := e.LocalEntity.FeatureOfTypeAndRole(model.FeatureTypeTypeLoadControl, model.RoleTypeServer)
138-
139-
result := model.ErrorType{
140-
ErrorNumber: model.ErrorNumberType(0),
141-
}
142-
143136
msg, ok := e.pendingLimits[msgCounter]
144137
if !ok {
145-
// it is not a limit of this usecase, so approve it
146-
newMsg := &spineapi.Message{
147-
RequestHeader: &model.HeaderType{
148-
MsgCounter: &msgCounter,
149-
},
150-
}
151-
f.ApproveOrDenyWrite(newMsg, result)
138+
// no pending limit for this msgCounter, this is a caller error
152139
return
153140
}
154141

155-
if !approve {
156-
result.ErrorNumber = model.ErrorNumberType(7)
157-
result.Description = util.Ptr(model.DescriptionType(reason))
158-
}
159-
f.ApproveOrDenyWrite(msg, result)
142+
e.approveOrDenyProductionLimit(msg, approve, reason)
160143

161144
delete(e.pendingLimits, msgCounter)
162145
}

usecases/cs/lpp/usecase.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,20 @@ func (e *LPP) loadControlServerAndLimitId() (lc *server.LoadControl, limitid mod
102102
return lc, *description.LimitId, nil
103103
}
104104

105+
func (e *LPP) approveOrDenyProductionLimit(msg *spineapi.Message, approve bool, reason string) {
106+
f := e.LocalEntity.FeatureOfTypeAndRole(model.FeatureTypeTypeLoadControl, model.RoleTypeServer)
107+
108+
result := model.ErrorType{
109+
ErrorNumber: model.ErrorNumberType(0),
110+
}
111+
112+
if !approve {
113+
result.ErrorNumber = model.ErrorNumberType(7)
114+
result.Description = util.Ptr(model.DescriptionType(reason))
115+
}
116+
f.ApproveOrDenyWrite(msg, result)
117+
}
118+
105119
// callback invoked on incoming write messages to this
106120
// loadcontrol server feature.
107121
// the implementation only considers write messages for this use case and
@@ -145,7 +159,7 @@ func (e *LPP) loadControlWriteCB(msg *spineapi.Message) {
145159
e.pendingMux.Unlock()
146160

147161
// approve, because this is no request for this usecase
148-
go e.ApproveOrDenyProductionLimit(*msg.RequestHeader.MsgCounter, true, "")
162+
go e.approveOrDenyProductionLimit(msg, true, "")
149163
}
150164

151165
func (e *LPP) AddFeatures() {

0 commit comments

Comments
 (0)