From 151a4325b1ee3c6d8480bfe448275c688da44f14 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 22 Dec 2017 16:32:11 +0100 Subject: [PATCH] htlcswitch: fix alignment of the packetQueue's fields for 32-bit systems (#507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we fix an existing issue that would cause lnd to panic on 32-bit systems. Within the packetQueue we utilize atomics heavily. However, it's the caller's job to ensure 64-bit alignment of 64-bit words accessed atomically. This is documented within the sync/atomic package as a set of known bugs. The old alignment of this struct was: ⛰ structlayout github.com/lightningnetwork/lnd/htlcswitch packetQueue packetQueue.queueLen int32: 0-4 (size 4, align 4) padding: 4-8 (size 4, align 0) packetQueue.totalHtlcAmt int64: 8-16 (size 8, align 8) packetQueue.queueCond *sync.Cond: 16-24 (size 8, align 8) packetQueue.queueMtx.state int32: 24-28 (size 4, align 4) packetQueue.queueMtx.sema uint32: 28-32 (size 4, align 4) packetQueue.queue []*github.com/lightningnetwork/lnd/htlcswitch.htlcPacket: 32-56 (size 24, align 8) packetQueue.outgoingPkts chan *github.com/lightningnetwork/lnd/htlcswitch.htlcPacket: 56-64 (size 8, align 8) packetQueue.freeSlots chan struct{}: 64-72 (size 8, align 8) packetQueue.wg.noCopy sync.noCopy: 72-72 (size 0, align 1) packetQueue.wg.state1 [12]byte: 72-84 (size 12, align 1) packetQueue.wg.sema uint32: 84-88 (size 4, align 4) packetQueue.quit chan struct{}: 88-96 (size 8, align 8) After this commit, the new alignment of this sturct is: ⛰ structlayout -json github.com/lightningnetwork/lnd/htlcswitch packetQueue | structlayout-optimize packetQueue.queue []*github.com/lightningnetwork/lnd/htlcswitch.htlcPacket: 0-24 (size 24, align 8) packetQueue.wg struct: 24-40 (size 16, align 8) packetQueue.freeSlots chan struct{}: 40-48 (size 8, align 8) packetQueue.queueCond *sync.Cond: 48-56 (size 8, align 8) packetQueue.queueMtx struct: 56-64 (size 8, align 8) packetQueue.outgoingPkts chan *github.com/lightningnetwork/lnd/htlcswitch.htlcPacket: 64-72 (size 8, align 8) packetQueue.totalHtlcAmt int64: 72-80 (size 8, align 8) packetQueue.quit chan struct{}: 80-88 (size 8, align 8) packetQueue.queueLen int32: 88-92 (size 4, align 8) padding: 92-96 (size 4, align 0) Fixes #505, and #463. --- htlcswitch/queue.go | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/htlcswitch/queue.go b/htlcswitch/queue.go index 22a7c382..0e80b0ce 100644 --- a/htlcswitch/queue.go +++ b/htlcswitch/queue.go @@ -17,6 +17,29 @@ import ( // to signal the number of slots available, and a condition variable to allow // the packetQueue to know when new items have been added to the queue. type packetQueue struct { + queue []*htlcPacket + + wg sync.WaitGroup + + // freeSlots serves as a semaphore who's current value signals the + // number of available slots on the commitment transaction. + freeSlots chan struct{} + + queueCond *sync.Cond + queueMtx sync.Mutex + + // outgoingPkts is a channel that the channelLink will receive on in + // order to drain the packetQueue as new slots become available on the + // commitment transaction. + outgoingPkts chan *htlcPacket + + // totalHtlcAmt is the sum of the value of all pending HTLC's currently + // residing within the overflow queue. This value should only read or + // modified *atomically*. + totalHtlcAmt int64 + + quit chan struct{} + // queueLen is an internal counter that reflects the size of the queue // at any given instance. This value is intended to be use atomically // as this value is used by internal methods to obtain the length of @@ -24,27 +47,6 @@ type packetQueue struct { // deadlock situation where the main goroutine is attempting a send // with the lock held. queueLen int32 - - // totalHtlcAmt is the sum of the value of all pending HTLC's currently - // residing within the overflow queue. This value should only read or - // modified *atomically*. - totalHtlcAmt int64 - - queueCond *sync.Cond - queueMtx sync.Mutex - queue []*htlcPacket - - // outgoingPkts is a channel that the channelLink will receive on in - // order to drain the packetQueue as new slots become available on the - // commitment transaction. - outgoingPkts chan *htlcPacket - - // freeSlots serves as a semaphore who's current value signals the - // number of available slots on the commitment transaction. - freeSlots chan struct{} - - wg sync.WaitGroup - quit chan struct{} } // newPacketQueue returns a new instance of the packetQueue. The maxFreeSlots