From e226733b2603a7cdfea326df0190a0a010e9be81 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 28 Jun 2021 18:19:44 -0400 Subject: [PATCH] fix 64-bit aligment for 32-bit platforms sync/atomic must be used with 64-bit aligned fields, and that alignment is difficult to ensure unless the field is the first one in the struct. https://golang.org/pkg/sync/atomic/#pkg-note-BUG. --- .changelog/10515.txt | 3 +++ agent/grpc/stats.go | 12 ++++++++---- tlsutil/config.go | 8 +++++--- 3 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 .changelog/10515.txt diff --git a/.changelog/10515.txt b/.changelog/10515.txt new file mode 100644 index 000000000..b83e5e154 --- /dev/null +++ b/.changelog/10515.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: fix a panic on 32-bit platforms caused by misaligned struct fields used with sync/atomic. +``` diff --git a/agent/grpc/stats.go b/agent/grpc/stats.go index 7ba96f91f..8af6ec400 100644 --- a/agent/grpc/stats.go +++ b/agent/grpc/stats.go @@ -52,8 +52,11 @@ var defaultMetrics = metrics.Default // statsHandler is a grpc/stats.StatsHandler which emits connection and // request metrics to go-metrics. type statsHandler struct { + // activeConns is used with sync/atomic and MUST be 64-bit aligned. To ensure + // alignment on 32-bit platforms this field must remain the first field in + // the struct. See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. + activeConns uint64 metrics *metrics.Metrics - activeConns uint64 // must be 8-byte aligned for atomic access } func newStatsHandler(m *metrics.Metrics) *statsHandler { @@ -103,10 +106,11 @@ func (c *statsHandler) HandleConn(_ context.Context, s stats.ConnStats) { } type activeStreamCounter struct { + // count is used with sync/atomic and MUST be 64-bit aligned. To ensure + // alignment on 32-bit platforms this field must remain the first field in + // the struct. See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. + count uint64 metrics *metrics.Metrics - // count of the number of open streaming RPCs on a server. It is accessed - // atomically. - count uint64 } // GRPCCountingStreamInterceptor is a grpc.ServerStreamInterceptor that emits a diff --git a/tlsutil/config.go b/tlsutil/config.go index 6fcdb1a2e..94053b439 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -175,6 +175,11 @@ type manual struct { // Configurator receives an initial TLS configuration from agent configuration, // and receives updates from config reloads, auto-encrypt, and auto-config. type Configurator struct { + // version is increased each time the Configurator is updated. Must be accessed + // using sync/atomic. Also MUST be the first field in this struct to ensure + // 64-bit alignment. See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. + version uint64 + // lock synchronizes access to all fields on this struct except for logger and version. lock sync.RWMutex base *Config @@ -188,9 +193,6 @@ type Configurator struct { // logger is not protected by a lock. It must never be changed after // Configurator is created. logger hclog.Logger - // version is increased each time the Configurator is updated. Must be accessed - // using sync/atomic. - version uint64 } // NewConfigurator creates a new Configurator and sets the provided