From d1d398adb7c39c4173313f26c3ab390c33990227 Mon Sep 17 00:00:00 2001 From: TheDiveO <6920158+thediveo@users.noreply.github.com> Date: Mon, 12 Dec 2022 08:51:36 +0100 Subject: [PATCH] alignedbuff: fix alignment test issue on 32-bit machines (#211) - fixes issue #209 where two unit tests for alignedbuff were incorrectly calculating the expected marshalled data length on 32bit machines (whereas actual padding/alignment itself was done correctly). - adds documentation reference to kernel's xtables.h UAPI regarding alignment. --- alignedbuff/alignedbuff.go | 14 +++++++++++++- alignedbuff/alignedbuff_test.go | 20 ++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/alignedbuff/alignedbuff.go b/alignedbuff/alignedbuff.go index 7bb09f8..a972146 100644 --- a/alignedbuff/alignedbuff.go +++ b/alignedbuff/alignedbuff.go @@ -1,5 +1,16 @@ // Package alignedbuff implements encoding and decoding aligned data elements // to/from buffers in native endianess. +// +// # Note +// +// The alignment/padding as implemented in this package must match that of +// kernel's and user space C implementations for a particular architecture (bit +// size). Please see also the "dummy structure" _xt_align +// (https://elixir.bootlin.com/linux/v5.17.7/source/include/uapi/linux/netfilter/x_tables.h#L93) +// as well as the associated XT_ALIGN C preprocessor macro. +// +// In particular, we rely on the Go compiler to follow the same architecture +// alignments as the C compiler(s) on Linux. package alignedbuff import ( @@ -144,7 +155,8 @@ func (a *AlignedBuff) String() (string, error) { return v, nil } -// Unmarshals a string of a given length (for non-null terminated strings) +// StringWithLength unmarshals a string of a given length (for non-null +// terminated strings) func (a *AlignedBuff) StringWithLength(len int) (string, error) { v := binaryutil.String(a.data[a.pos : a.pos+len]) a.pos += len diff --git a/alignedbuff/alignedbuff_test.go b/alignedbuff/alignedbuff_test.go index edee6c1..b3d82a8 100644 --- a/alignedbuff/alignedbuff_test.go +++ b/alignedbuff/alignedbuff_test.go @@ -103,7 +103,15 @@ func TestAlignedBuff32(t *testing.T) { b := NewWithData(b0.data) - if len(b0.Data()) != 4*4 { + // Sigh. The Linux kernel expects certain nftables payloads to be padded to + // the uint64 next alignment. Now, on 64bit platforms this will be a 64bit + // alignment, yet on 32bit platforms this will be a 32bit alignment. So, we + // should calculate the expected data length here separately from our + // implementation to be fail safe! However, this might be rather a recipe + // for a safe fail... + expectedlen := 2*(uint32AlignMask+1) + (uint64AlignMask + 1) + + if len(b0.Data()) != expectedlen { t.Fatalf("alignment padding failed") } @@ -214,7 +222,15 @@ func TestAlignedBuffInt32(t *testing.T) { b := NewWithData(b0.data) - if len(b0.Data()) != 4*4 { + // Sigh. The Linux kernel expects certain nftables payloads to be padded to + // the uint64 next alignment. Now, on 64bit platforms this will be a 64bit + // alignment, yet on 32bit platforms this will be a 32bit alignment. So, we + // should calculate the expected data length here separately from our + // implementation to be fail safe! However, this might be rather a recipe + // for a safe fail... + expectedlen := 2*(uint32AlignMask+1) + (uint64AlignMask + 1) + + if len(b0.Data()) != expectedlen { t.Fatalf("alignment padding failed") }