Improve safety of ID allocation (#307)
There was an existing mechanism to allocate IDs for sets, but this was using a global counter without any synchronization to prevent data races. I replaced this by a new mechanism which uses a connection-scoped counter, protected by the Conn.mu Mutex. This can then also be used in other places where IDs need to be allocated. As an additional safeguard, it will panic instead of allocating the same ID twice in a transaction. Most likely, your program will run out of memory before reaching this point.
This commit is contained in:
parent
a24f918d08
commit
e2fedeb355
21
conn.go
21
conn.go
|
@ -17,6 +17,7 @@ package nftables
|
||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"math"
|
||||||
"os"
|
"os"
|
||||||
"sync"
|
"sync"
|
||||||
"syscall"
|
"syscall"
|
||||||
|
@ -44,6 +45,8 @@ type Conn struct {
|
||||||
err error
|
err error
|
||||||
nlconn *netlink.Conn // netlink socket using NETLINK_NETFILTER protocol.
|
nlconn *netlink.Conn // netlink socket using NETLINK_NETFILTER protocol.
|
||||||
sockOptions []SockOption
|
sockOptions []SockOption
|
||||||
|
lastID uint32
|
||||||
|
allocatedIDs uint32
|
||||||
}
|
}
|
||||||
|
|
||||||
// ConnOption is an option to change the behavior of the nftables Conn returned by Open.
|
// ConnOption is an option to change the behavior of the nftables Conn returned by Open.
|
||||||
|
@ -244,6 +247,7 @@ func (cc *Conn) Flush() error {
|
||||||
cc.mu.Lock()
|
cc.mu.Lock()
|
||||||
defer func() {
|
defer func() {
|
||||||
cc.messages = nil
|
cc.messages = nil
|
||||||
|
cc.allocatedIDs = 0
|
||||||
cc.mu.Unlock()
|
cc.mu.Unlock()
|
||||||
}()
|
}()
|
||||||
if len(cc.messages) == 0 {
|
if len(cc.messages) == 0 {
|
||||||
|
@ -369,3 +373,20 @@ func batch(messages []netlink.Message) []netlink.Message {
|
||||||
|
|
||||||
return batch
|
return batch
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// allocateTransactionID allocates an identifier which is only valid in the
|
||||||
|
// current transaction.
|
||||||
|
func (cc *Conn) allocateTransactionID() uint32 {
|
||||||
|
if cc.allocatedIDs == math.MaxUint32 {
|
||||||
|
panic(fmt.Sprintf("trying to allocate more than %d IDs in a single nftables transaction", math.MaxUint32))
|
||||||
|
}
|
||||||
|
// To make it more likely to catch when a transaction ID is erroneously used
|
||||||
|
// in a later transaction, cc.lastID is not reset after each transaction;
|
||||||
|
// instead it is only reset once it rolls over from math.MaxUint32 to 0.
|
||||||
|
cc.allocatedIDs++
|
||||||
|
cc.lastID++
|
||||||
|
if cc.lastID == 0 {
|
||||||
|
cc.lastID = 1
|
||||||
|
}
|
||||||
|
return cc.lastID
|
||||||
|
}
|
||||||
|
|
5
set.go
5
set.go
|
@ -46,8 +46,6 @@ const (
|
||||||
NFTA_SET_ELEM_EXPRESSIONS = 0x11
|
NFTA_SET_ELEM_EXPRESSIONS = 0x11
|
||||||
)
|
)
|
||||||
|
|
||||||
var allocSetID uint32
|
|
||||||
|
|
||||||
// SetDatatype represents a datatype declared by nft.
|
// SetDatatype represents a datatype declared by nft.
|
||||||
type SetDatatype struct {
|
type SetDatatype struct {
|
||||||
Name string
|
Name string
|
||||||
|
@ -532,8 +530,7 @@ func (cc *Conn) AddSet(s *Set, vals []SetElement) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
if s.ID == 0 {
|
if s.ID == 0 {
|
||||||
allocSetID++
|
s.ID = cc.allocateTransactionID()
|
||||||
s.ID = allocSetID
|
|
||||||
if s.Anonymous {
|
if s.Anonymous {
|
||||||
s.Name = "__set%d"
|
s.Name = "__set%d"
|
||||||
if s.IsMap {
|
if s.IsMap {
|
||||||
|
|
Loading…
Reference in New Issue