diff --git a/ovsnl/client.go b/ovsnl/client.go index 4e522bc..a4fe936 100644 --- a/ovsnl/client.go +++ b/ovsnl/client.go @@ -18,20 +18,11 @@ import ( "fmt" "os" "strings" - "unsafe" "github.com/digitalocean/go-openvswitch/ovsnl/internal/ovsh" "github.com/mdlayher/genetlink" ) -// Sizes of various structures, used in unsafe casts. -const ( - sizeofHeader = int(unsafe.Sizeof(ovsh.Header{})) - - sizeofDPStats = int(unsafe.Sizeof(ovsh.DPStats{})) - sizeofDPMegaflowStats = int(unsafe.Sizeof(ovsh.DPMegaflowStats{})) -) - // A Client is a Linux Open vSwitch generic netlink client. type Client struct { // Datapath provides access to DatapathService methods. @@ -116,20 +107,3 @@ func (c *Client) initFamily(f genetlink.Family) error { return fmt.Errorf("unknown OVS generic netlink family: %q", f.Name) } } - -// headerBytes converts an ovsh.Header into a byte slice. -func headerBytes(h ovsh.Header) []byte { - b := *(*[sizeofHeader]byte)(unsafe.Pointer(&h)) // #nosec G103 - return b[:] -} - -// parseHeader converts a byte slice into ovsh.Header. -func parseHeader(b []byte) (ovsh.Header, error) { - // Verify that the byte slice is long enough before doing unsafe casts. - if l := len(b); l < sizeofHeader { - return ovsh.Header{}, fmt.Errorf("not enough data for OVS message header: %d bytes", l) - } - - h := *(*ovsh.Header)(unsafe.Pointer(&b[:sizeofHeader][0])) // #nosec G103 - return h, nil -} diff --git a/ovsnl/datapath.go b/ovsnl/datapath.go index 2f4921a..05c4eeb 100644 --- a/ovsnl/datapath.go +++ b/ovsnl/datapath.go @@ -15,8 +15,7 @@ package ovsnl import ( - "fmt" - "unsafe" + "encoding/binary" "github.com/digitalocean/go-openvswitch/ovsnl/internal/ovsh" "github.com/mdlayher/genetlink" @@ -98,15 +97,20 @@ type DatapathMegaflowStats struct { // List lists all Datapaths in the kernel. func (s *DatapathService) List() ([]Datapath, error) { + data, err := ovsh.MarshalBinary(&ovsh.Header{ + // Query all datapaths. + Ifindex: 0, + }) + if err != nil { + return nil, err + } + req := genetlink.Message{ Header: genetlink.Header{ Command: ovsh.DpCmdGet, Version: uint8(s.f.Version), }, - // Query all datapaths. - Data: headerBytes(ovsh.Header{ - Ifindex: 0, - }), + Data: data, } flags := netlink.Request | netlink.Dump @@ -134,8 +138,9 @@ func parseDatapaths(msgs []genetlink.Message) ([]Datapath, error) { Index: int(h.Ifindex), } - // Skip the header to parse attributes. - attrs, err := netlink.UnmarshalAttributes(m.Data[sizeofHeader:]) + // Skip past the header to parse attributes. + hdrSize := binary.Size(h) + attrs, err := netlink.UnmarshalAttributes(m.Data[hdrSize:]) if err != nil { return nil, err } @@ -167,13 +172,12 @@ func parseDatapaths(msgs []genetlink.Message) ([]Datapath, error) { // parseDPStats converts a byte slice into DatapathStats. func parseDPStats(b []byte) (DatapathStats, error) { - // Verify that the byte slice is the correct length before doing - // unsafe casts. - if want, got := sizeofDPStats, len(b); want != got { - return DatapathStats{}, fmt.Errorf("unexpected datapath stats structure size, want %d, got %d", want, got) + s := new(ovsh.DPStats) + err := ovsh.UnmarshalBinary(b, s) + if err != nil { + return DatapathStats{}, err } - s := *(*ovsh.DPStats)(unsafe.Pointer(&b[0])) // #nosec G103 return DatapathStats{ Hit: s.Hit, Missed: s.Missed, @@ -184,16 +188,24 @@ func parseDPStats(b []byte) (DatapathStats, error) { // parseDPMegaflowStats converts a byte slice into DatapathMegaflowStats. func parseDPMegaflowStats(b []byte) (DatapathMegaflowStats, error) { - // Verify that the byte slice is the correct length before doing - // unsafe casts. - if want, got := sizeofDPMegaflowStats, len(b); want != got { - return DatapathMegaflowStats{}, fmt.Errorf("unexpected datapath megaflow stats structure size, want %d, got %d", want, got) + s := new(ovsh.DPMegaflowStats) + err := ovsh.UnmarshalBinary(b, s) + if err != nil { + return DatapathMegaflowStats{}, err } - s := *(*ovsh.DPMegaflowStats)(unsafe.Pointer(&b[0])) // #nosec G103 - return DatapathMegaflowStats{ MaskHits: s.Mask_hit, Masks: s.Masks, }, nil } + +// parseHeader converts a byte slice into ovsh.Header. +func parseHeader(b []byte) (ovsh.Header, error) { + h := new(ovsh.Header) + err := ovsh.UnmarshalBinary(b, h) + if err != nil { + return ovsh.Header{}, err + } + return *h, nil +} diff --git a/ovsnl/datapath_linux_test.go b/ovsnl/datapath_linux_test.go index c40b145..c7e14a2 100644 --- a/ovsnl/datapath_linux_test.go +++ b/ovsnl/datapath_linux_test.go @@ -13,12 +13,12 @@ // limitations under the License. //go:build linux +// +build linux package ovsnl import ( "testing" - "unsafe" "github.com/digitalocean/go-openvswitch/ovsnl/internal/ovsh" "github.com/google/go-cmp/cmp" @@ -136,7 +136,8 @@ func TestClientDatapathListOK(t *testing.T) { t.Fatalf("unexpected generic netlink command (-want +got):\n%s", diff) } - h, err := parseHeader(greq.Data) + h := new(ovsh.Header) + err := ovsh.UnmarshalBinary(greq.Data, h) if err != nil { t.Fatalf("failed to parse OvS generic netlink header: %v", err) } @@ -147,7 +148,7 @@ func TestClientDatapathListOK(t *testing.T) { return []genetlink.Message{ { - Data: mustMarshalDatapath(system), + Data: mustMarshalDatapath(t, system), }, }, nil })) @@ -172,29 +173,34 @@ func TestClientDatapathListOK(t *testing.T) { } } -func mustMarshalDatapath(dp Datapath) []byte { - h := ovsh.Header{ +func mustMarshalDatapath(t *testing.T, dp Datapath) []byte { + hb, err := ovsh.MarshalBinary(&ovsh.Header{ Ifindex: int32(dp.Index), + }) + if err != nil { + t.Fatal(t) } - hb := headerBytes(h) - - s := ovsh.DPStats{ + sb, err := ovsh.MarshalBinary(&ovsh.DPStats{ Hit: dp.Stats.Hit, Missed: dp.Stats.Missed, Lost: dp.Stats.Lost, Flows: dp.Stats.Flows, + }) + if err != nil { + t.Fatal(t) } - sb := *(*[sizeofDPStats]byte)(unsafe.Pointer(&s)) // #nosec G103 - ms := ovsh.DPMegaflowStats{ Mask_hit: dp.MegaflowStats.MaskHits, Masks: dp.MegaflowStats.Masks, // Pad already set to zero. } - msb := *(*[sizeofDPMegaflowStats]byte)(unsafe.Pointer(&ms)) // #nosec G103 + msb, err := ovsh.MarshalBinary(&ms) + if err != nil { + t.Fatal(t) + } ab := mustMarshalAttributes([]netlink.Attribute{ { @@ -207,7 +213,7 @@ func mustMarshalDatapath(dp Datapath) []byte { }, { Type: ovsh.DpAttrStats, - Data: sb[:], + Data: sb, }, { Type: ovsh.DpAttrMegaflowStats, diff --git a/ovsnl/internal/ovsh/binary.go b/ovsnl/internal/ovsh/binary.go new file mode 100644 index 0000000..005b5b0 --- /dev/null +++ b/ovsnl/internal/ovsh/binary.go @@ -0,0 +1,45 @@ +// Copyright 2017 DigitalOcean. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ovsh + +import ( + "bytes" + "encoding/binary" + "fmt" +) + +// ovsTypes defines a type set of the types defined in struct.go +type ovsTypes interface { + Header | DPStats | DPMegaflowStats | VportStats | FlowStats +} + +// MarshalBinary is a generic binary marshaling function for the type defined in struct.go +func MarshalBinary[T ovsTypes](data *T) ([]byte, error) { + var buf bytes.Buffer + err := binary.Write(&buf, binary.NativeEndian, data) + return buf.Bytes(), err +} + +// UnmarshalBinary is a generic binary unmarshaling function for the type defined in struct.go +func UnmarshalBinary[T ovsTypes](data []byte, dst *T) error { + // Verify that the byte slice has enough data before unmarshaling. + if want, got := binary.Size(*dst), len(data); got < want { + return fmt.Errorf("unexpected size of struct %T, want at least %d, got %d", *dst, want, got) + } + + *dst = *new(T) // reset the contents, just to be safe + buf := bytes.NewBuffer(data) + return binary.Read(buf, binary.NativeEndian, dst) +}