device: clear pointers when returning elems to pools

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>

diff --git a/device/pools.go b/device/pools.go
index e778d2e62143720b904366faa68ea705715718c6..6939eeb10618d1ff61ce015bca72c1f6cfa0d4e7 100644 (file)
--- a/device/pools.go
+++ b/device/pools.go
@@ -65,6 +65,7 @@ func (device *Device) GetInboundElement() *QueueInboundElement {
 }
 
 func (device *Device) PutInboundElement(msg *QueueInboundElement) {
+       msg.clearPointers()
        if PreallocatedBuffersPerPool == 0 {
                device.pool.inboundElementPool.Put(msg)
        } else {
@@ -81,6 +82,7 @@ func (device *Device) GetOutboundElement() *QueueOutboundElement {
 }
 
 func (device *Device) PutOutboundElement(msg *QueueOutboundElement) {
+       msg.clearPointers()
        if PreallocatedBuffersPerPool == 0 {
                device.pool.outboundElementPool.Put(msg)
        } else {

diff --git a/device/receive.go b/device/receive.go
index e4a94b51a17b9eebe5b589b9eb095e4ca87fc752..0a8228cf7f481e915961b95b280f0a75a6d3f4d0 100644 (file)
--- a/device/receive.go
+++ b/device/receive.go
@@ -37,6 +37,17 @@ type QueueInboundElement struct {
        endpoint conn.Endpoint
 }
 
+// clearPointers clears elem fields that contain pointers.
+// This makes the garbage collector's life easier and
+// avoids accidentally keeping other objects around unnecessarily.
+// It also reduces the possible collateral damage from use-after-free bugs.
+func (elem *QueueInboundElement) clearPointers() {
+       elem.buffer = nil
+       elem.packet = nil
+       elem.keypair = nil
+       elem.endpoint = nil
+}
+
 func (elem *QueueInboundElement) Drop() {
        atomic.StoreInt32(&elem.dropped, AtomicTrue)
 }

diff --git a/device/send.go b/device/send.go
index fa4da0e98e262cd58f50ff9b154d43f090f918e7..cb3e3f623ce3c5c8139a2918fcd2d7f7dcbfefd2 100644 (file)
--- a/device/send.go
+++ b/device/send.go
@@ -58,9 +58,19 @@ func (device *Device) NewOutboundElement() *QueueOutboundElement {
        elem.buffer = device.GetMessageBuffer()
        elem.Mutex = sync.Mutex{}
        elem.nonce = 0
+       // keypair and peer were cleared (if necessary) by clearPointers.
+       return elem
+}
+
+// clearPointers clears elem fields that contain pointers.
+// This makes the garbage collector's life easier and
+// avoids accidentally keeping other objects around unnecessarily.
+// It also reduces the possible collateral damage from use-after-free bugs.
+func (elem *QueueOutboundElement) clearPointers() {
+       elem.buffer = nil
+       elem.packet = nil
        elem.keypair = nil
        elem.peer = nil
-       return elem
 }
 
 func (elem *QueueOutboundElement) Drop() {