From 4ef7bcbf157aa300a8daa1b0212d482c238824d1 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Mon, 10 May 2021 11:48:42 +0200 Subject: [PATCH] Don't re-initialize or shadow package level var native to fix data race 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 --- addr_linux.go | 1 - class_linux.go | 2 -- filter_linux.go | 7 ------- link_linux.go | 1 - neigh_linux.go | 1 - qdisc_linux.go | 5 ----- route_linux.go | 19 ++----------------- rule_linux.go | 3 --- socket_linux.go | 1 - 9 files changed, 2 insertions(+), 38 deletions(-) diff --git a/addr_linux.go b/addr_linux.go index 71da251c..6504dbcb 100644 --- a/addr_linux.go +++ b/addr_linux.go @@ -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 diff --git a/class_linux.go b/class_linux.go index 029568a3..fe8963f4 100644 --- a/class_linux.go +++ b/class_linux.go @@ -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 @@ -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) } diff --git a/filter_linux.go b/filter_linux.go index 2cd46266..5514531d 100644 --- a/filter_linux.go +++ b/filter_linux.go @@ -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{ @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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() diff --git a/link_linux.go b/link_linux.go index f06e9478..caf30c05 100644 --- a/link_linux.go +++ b/link_linux.go @@ -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 diff --git a/neigh_linux.go b/neigh_linux.go index fb220d14..f2c70ba9 100644 --- a/neigh_linux.go +++ b/neigh_linux.go @@ -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 diff --git a/qdisc_linux.go b/qdisc_linux.go index edc4b726..9fc4b3d2 100644 --- a/qdisc_linux.go +++ b/qdisc_linux.go @@ -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 { @@ -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 { @@ -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 { @@ -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 { diff --git a/route_linux.go b/route_linux.go index a0d70d18..d9f715ce 100644 --- a/route_linux.go +++ b/route_linux.go @@ -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") @@ -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) @@ -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) { @@ -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) @@ -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: @@ -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) @@ -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)) } @@ -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)) @@ -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 { @@ -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)) @@ -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 diff --git a/rule_linux.go b/rule_linux.go index 40474f30..fe68f28f 100644 --- a/rule_linux.go +++ b/rule_linux.go @@ -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)) @@ -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]) diff --git a/socket_linux.go b/socket_linux.go index 9b0f4a08..8738d417 100644 --- a/socket_linux.go +++ b/socket_linux.go @@ -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) }