Skip to content

Commit

Permalink
Don't re-initialize or shadow package level var native to fix data race
Browse files Browse the repository at this point in the history 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue #633.

Also don't re-declare local variables shadowing the global package-level
var.

Fixes #633

Signed-off-by: Tobias Klauser <tobias@cilium.io>
  • Loading branch information
@tklauser @aboch
tklauser authored and aboch committed May 10, 2021
1 parent af1e63e commit 4ef7bcb
Show file tree
Hide file tree
Showing 9 changed files with 2 additions and 38 deletions.
1 change: 0 additions & 1 deletion addr_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ func addrSubscribeAt(newNs, curNs netns.NsHandle, ch chan<- AddrUpdate, done <-c
continue
}
if m.Header.Type == unix.NLMSG_ERROR {
native := nl.NativeEndian()
error := int32(native.Uint32(m.Data[0:4]))
if error == 0 {
continue
Expand Down
2 changes: 0 additions & 2 deletions class_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ func parseHfscClassData(class Class, data []syscall.NetlinkRouteAttr) (bool, err
func parseTcStats(data []byte) (*ClassStatistics, error) {
buf := &bytes.Buffer{}
buf.Write(data)
native := nl.NativeEndian()
tcStats := &tcStats{}
if err := binary.Read(buf, native, tcStats); err != nil {
return nil, err
Expand All @@ -363,7 +362,6 @@ func parseTcStats(data []byte) (*ClassStatistics, error) {
func parseGnetStats(data []byte, gnetStats interface{}) error {
buf := &bytes.Buffer{}
buf.Write(data)
native := nl.NativeEndian()
return binary.Read(buf, native, gnetStats)
}

Expand Down
7 changes: 0 additions & 7 deletions filter_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ func (h *Handle) FilterReplace(filter Filter) error {
}

func (h *Handle) filterModify(filter Filter, flags int) error {
native = nl.NativeEndian()
req := h.newNetlinkRequest(unix.RTM_NEWTFILTER, flags|unix.NLM_F_ACK)
base := filter.Attrs()
msg := &nl.TcMsg{
Expand Down Expand Up @@ -632,7 +631,6 @@ func parseActions(tables []syscall.NetlinkRouteAttr) ([]Action, error) {
}

func parseU32Data(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error) {
native = nl.NativeEndian()
u32 := filter.(*U32)
detailed := false
for _, datum := range data {
Expand Down Expand Up @@ -678,7 +676,6 @@ func parseU32Data(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error)
}

func parseFwData(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error) {
native = nl.NativeEndian()
fw := filter.(*Fw)
detailed := true
for _, datum := range data {
Expand Down Expand Up @@ -707,7 +704,6 @@ func parseFwData(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error) {
}

func parseBpfData(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error) {
native = nl.NativeEndian()
bpf := filter.(*BpfFilter)
detailed := true
for _, datum := range data {
Expand All @@ -733,7 +729,6 @@ func parseBpfData(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error)
}

func parseMatchAllData(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error) {
native = nl.NativeEndian()
matchall := filter.(*MatchAll)
detailed := true
for _, datum := range data {
Expand Down Expand Up @@ -801,14 +796,12 @@ func CalcRtable(rate *nl.TcRateSpec, rtab []uint32, cellLog int, mtu uint32, lin

func DeserializeRtab(b []byte) [256]uint32 {
var rtab [256]uint32
native := nl.NativeEndian()
r := bytes.NewReader(b)
_ = binary.Read(r, native, &rtab)
return rtab
}

func SerializeRtab(rtab [256]uint32) []byte {
native := nl.NativeEndian()
var w bytes.Buffer
_ = binary.Write(&w, native, rtab)
return w.Bytes()
Expand Down
1 change: 0 additions & 1 deletion link_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2057,7 +2057,6 @@ func linkSubscribeAt(newNs, curNs netns.NsHandle, ch chan<- LinkUpdate, done <-c
continue
}
if m.Header.Type == unix.NLMSG_ERROR {
native := nl.NativeEndian()
error := int32(native.Uint32(m.Data[0:4]))
if error == 0 {
continue
Expand Down
1 change: 0 additions & 1 deletion neigh_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,6 @@ func neighSubscribeAt(newNs, curNs netns.NsHandle, ch chan<- NeighUpdate, done <
continue
}
if m.Header.Type == unix.NLMSG_ERROR {
native := nl.NativeEndian()
error := int32(native.Uint32(m.Data[0:4]))
if error == 0 {
continue
Expand Down
5 changes: 0 additions & 5 deletions qdisc_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,6 @@ func parsePrioData(qdisc Qdisc, value []byte) error {
}

func parseHtbData(qdisc Qdisc, data []syscall.NetlinkRouteAttr) error {
native = nl.NativeEndian()
htb := qdisc.(*Htb)
for _, datum := range data {
switch datum.Attr.Type {
Expand All @@ -488,7 +487,6 @@ func parseHtbData(qdisc Qdisc, data []syscall.NetlinkRouteAttr) error {
}

func parseFqCodelData(qdisc Qdisc, data []syscall.NetlinkRouteAttr) error {
native = nl.NativeEndian()
fqCodel := qdisc.(*FqCodel)
for _, datum := range data {

Expand Down Expand Up @@ -518,13 +516,11 @@ func parseFqCodelData(qdisc Qdisc, data []syscall.NetlinkRouteAttr) error {

func parseHfscData(qdisc Qdisc, data []byte) error {
Hfsc := qdisc.(*Hfsc)
native = nl.NativeEndian()
Hfsc.Defcls = native.Uint16(data)
return nil
}

func parseFqData(qdisc Qdisc, data []syscall.NetlinkRouteAttr) error {
native = nl.NativeEndian()
fq := qdisc.(*Fq)
for _, datum := range data {
switch datum.Attr.Type {
Expand Down Expand Up @@ -589,7 +585,6 @@ func parseNetemData(qdisc Qdisc, value []byte) error {
}

func parseTbfData(qdisc Qdisc, data []syscall.NetlinkRouteAttr) error {
native = nl.NativeEndian()
tbf := qdisc.(*Tbf)
for _, datum := range data {
switch datum.Attr.Type {
Expand Down
19 changes: 2 additions & 17 deletions route_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ func (e *MPLSEncap) Decode(buf []byte) error {
if len(buf) < 4 {
return fmt.Errorf("lack of bytes")
}
native := nl.NativeEndian()
l := native.Uint16(buf)
if len(buf) < int(l) {
return fmt.Errorf("lack of bytes")
Expand All @@ -167,7 +166,6 @@ func (e *MPLSEncap) Decode(buf []byte) error {

func (e *MPLSEncap) Encode() ([]byte, error) {
s := nl.EncodeMPLSStack(e.Labels...)
native := nl.NativeEndian()
hdr := make([]byte, 4)
native.PutUint16(hdr, uint16(len(s)+4))
native.PutUint16(hdr[2:], nl.MPLS_IPTUNNEL_DST)
Expand Down Expand Up @@ -223,7 +221,6 @@ func (e *SEG6Encap) Decode(buf []byte) error {
if len(buf) < 4 {
return fmt.Errorf("lack of bytes")
}
native := nl.NativeEndian()
// Get Length(l) & Type(typ) : 2 + 2 bytes
l := native.Uint16(buf)
if len(buf) < int(l) {
Expand All @@ -243,7 +240,6 @@ func (e *SEG6Encap) Decode(buf []byte) error {
}
func (e *SEG6Encap) Encode() ([]byte, error) {
s, err := nl.EncodeSEG6Encap(e.Mode, e.Segments)
native := nl.NativeEndian()
hdr := make([]byte, 4)
native.PutUint16(hdr, uint16(len(s)+4))
native.PutUint16(hdr[2:], nl.SEG6_IPTUNNEL_SRH)
Expand Down Expand Up @@ -304,7 +300,6 @@ func (e *SEG6LocalEncap) Decode(buf []byte) error {
if err != nil {
return err
}
native := nl.NativeEndian()
for _, attr := range attrs {
switch attr.Attr.Type {
case nl.SEG6_LOCAL_ACTION:
Expand Down Expand Up @@ -334,7 +329,6 @@ func (e *SEG6LocalEncap) Decode(buf []byte) error {
}
func (e *SEG6LocalEncap) Encode() ([]byte, error) {
var err error
native := nl.NativeEndian()
res := make([]byte, 8)
native.PutUint16(res, 8) // length
native.PutUint16(res[2:], nl.SEG6_LOCAL_ACTION)
Expand Down Expand Up @@ -504,7 +498,6 @@ func (v *Via) Encode() ([]byte, error) {
}

func (v *Via) Decode(b []byte) error {
native := nl.NativeEndian()
if len(b) < 6 {
return fmt.Errorf("decoding failed: buffer too small (%d bytes)", len(b))
}
Expand Down Expand Up @@ -840,10 +833,7 @@ func (h *Handle) routeHandle(route *Route, req *nl.NetlinkRequest, msg *nl.RtMsg
req.AddData(attr)
}

var (
b = make([]byte, 4)
native = nl.NativeEndian()
)
b := make([]byte, 4)
native.PutUint32(b, uint32(route.LinkIndex))

req.AddData(nl.NewRtAttr(unix.RTA_OIF, b))
Expand Down Expand Up @@ -958,7 +948,6 @@ func deserializeRoute(m []byte) (Route, error) {
Flags: int(msg.Flags),
}

native := nl.NativeEndian()
var encap, encapType syscall.NetlinkRouteAttr
for _, attr := range attrs {
switch attr.Attr.Type {
Expand Down Expand Up @@ -1199,10 +1188,7 @@ func (h *Handle) RouteGetWithOptions(destination net.IP, options *RouteGetOption
if err != nil {
return nil, err
}
var (
b = make([]byte, 4)
native = nl.NativeEndian()
)
b := make([]byte, 4)
native.PutUint32(b, uint32(link.Attrs().Index))

req.AddData(nl.NewRtAttr(unix.RTA_OIF, b))
Expand Down Expand Up @@ -1329,7 +1315,6 @@ func routeSubscribeAt(newNs, curNs netns.NsHandle, ch chan<- RouteUpdate, done <
continue
}
if m.Header.Type == unix.NLMSG_ERROR {
native := nl.NativeEndian()
error := int32(native.Uint32(m.Data[0:4]))
if error == 0 {
continue
Expand Down
3 changes: 0 additions & 3 deletions rule_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ func ruleHandle(rule *Rule, req *nl.NetlinkRequest) error {
req.AddData(rtAttrs[i])
}

native := nl.NativeEndian()

if rule.Priority >= 0 {
b := make([]byte, 4)
native.PutUint32(b, uint32(rule.Priority))
Expand Down Expand Up @@ -199,7 +197,6 @@ func (h *Handle) RuleListFiltered(family int, filter *Rule, filterMask uint64) (
return nil, err
}

native := nl.NativeEndian()
var res = make([]Rule, 0)
for i := range msgs {
msg := nl.DeserializeRtMsg(msgs[i])
Expand Down
1 change: 0 additions & 1 deletion socket_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ loop:
case unix.NLMSG_DONE:
break loop
case unix.NLMSG_ERROR:
native := nl.NativeEndian()
error := int32(native.Uint32(m.Data[0:4]))
return nil, syscall.Errno(-error)
}
Expand Down

0 comments on commit 4ef7bcb

Please sign in to comment.