From a66e14955d30c6f0806ad60ca197e6926bcd0b10 Mon Sep 17 00:00:00 2001
From: Taku Fukushima <taku@soracom.jp>
Date: Mon, 15 May 2017 16:36:56 +0900
Subject: [PATCH] Fix byte order and reference bug of U32 filters

This patch fixes the bug of U32 filters which byte orders are not
appropriately updated based on the endianess of the host. Golang's
range returns copied values instead of their references when it
iterates through a map and the indices should be used to access the
specific value of the map by reference.

This patch also fixes the bug of netlink.FilterAdd that breaks the user
facing model changing  the type of cSel, the copied TcU32Sel, from its
pointer to the struct. Previously the pointer is copied and therefore
the data that is given by the users is modified if the endiannesses of
the fields in it need to be changed.

To validate these changes, I added the validation that the user facing
model is identical before and after netlink.FilterAdd. In addition to
that, the fix for the reference bug enables the endianness validations
in the same test case.

Signed-off-by: Taku Fukushima <taku@soracom.jp>
---
 filter_linux.go | 18 +++++++++---------
 filter_test.go  |  8 ++++++++
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/filter_linux.go b/filter_linux.go
index a0e000c..dc0f90a 100644
--- a/filter_linux.go
+++ b/filter_linux.go
@@ -141,19 +141,19 @@ func (h *Handle) FilterAdd(filter Filter) error {
 		}
 
 		if native != networkOrder {
-			// Copy Tcu32Sel.
-			cSel := sel
+			// Copy TcU32Sel.
+			cSel := *sel
 			keys := make([]nl.TcU32Key, cap(sel.Keys))
 			copy(keys, sel.Keys)
 			cSel.Keys = keys
-			sel = cSel
+			sel = &cSel
 
 			// Handle the endianness of attributes
 			sel.Offmask = native.Uint16(htons(sel.Offmask))
 			sel.Hmask = native.Uint32(htonl(sel.Hmask))
-			for _, key := range sel.Keys {
-				key.Mask = native.Uint32(htonl(key.Mask))
-				key.Val = native.Uint32(htonl(key.Val))
+			for i, key := range sel.Keys {
+				sel.Keys[i].Mask = native.Uint32(htonl(key.Mask))
+				sel.Keys[i].Val = native.Uint32(htonl(key.Val))
 			}
 		}
 		sel.Nkeys = uint8(len(sel.Keys))
@@ -453,9 +453,9 @@ func parseU32Data(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error)
 				// Handle the endianness of attributes
 				u32.Sel.Offmask = native.Uint16(htons(sel.Offmask))
 				u32.Sel.Hmask = native.Uint32(htonl(sel.Hmask))
-				for _, key := range u32.Sel.Keys {
-					key.Mask = native.Uint32(htonl(key.Mask))
-					key.Val = native.Uint32(htonl(key.Val))
+				for i, key := range u32.Sel.Keys {
+					u32.Sel.Keys[i].Mask = native.Uint32(htonl(key.Mask))
+					u32.Sel.Keys[i].Val = native.Uint32(htonl(key.Val))
 				}
 			}
 		case nl.TCA_U32_ACT:
diff --git a/filter_test.go b/filter_test.go
index 64ab757..3f4be0d 100644
--- a/filter_test.go
+++ b/filter_test.go
@@ -3,6 +3,7 @@
 package netlink
 
 import (
+	"reflect"
 	"syscall"
 	"testing"
 )
@@ -193,9 +194,16 @@ func TestAdvancedFilterAddDel(t *testing.T) {
 		ClassId: classId,
 		Actions: []Action{},
 	}
+	// Copy filter.
+	cFilter := *filter
 	if err := FilterAdd(filter); err != nil {
 		t.Fatal(err)
 	}
+	// Check if the filter is identical before and after FilterAdd.
+	if !reflect.DeepEqual(cFilter, *filter) {
+		t.Fatal("U32 %v and %v are not equal", cFilter, *filter)
+	}
+
 	filters, err := FilterList(link, qdiscHandle)
 	if err != nil {
 		t.Fatal(err)