Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync: dequeue newSize may be overflow on 32bit system #44532

Open
math345 opened this issue Feb 23, 2021 · 2 comments
Open

sync: dequeue newSize may be overflow on 32bit system #44532

math345 opened this issue Feb 23, 2021 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@math345
Copy link

math345 commented Feb 23, 2021

const dequeueBits = 32

// dequeueLimit is the maximum size of a poolDequeue.
//
// This must be at most (1<<dequeueBits)/2 because detecting fullness
// depends on wrapping around the ring buffer without wrapping around
// the index. We divide by 4 so this fits in an int on 32-bit.
const dequeueLimit = (1 << dequeueBits) / 4

func (c *poolChain) pushHead(val interface{}) {
	d := c.head
	if d == nil {
		// Initialize the chain.
		const initSize = 8 // Must be a power of 2
		d = new(poolChainElt)
		d.vals = make([]eface, initSize)
		c.head = d
		storePoolChainElt(&c.tail, d)
	}

	if d.pushHead(val) {
		return
	}

	// The current dequeue is full. Allocate a new one of twice
	// the size.
	newSize := len(d.vals) * 2
	if newSize >= dequeueLimit {
		// Can't make it any bigger.
		newSize = dequeueLimit
	}

	d2 := &poolChainElt{prev: d}
	d2.vals = make([]eface, newSize)
	c.head = d2
	storePoolChainElt(&d.next, d2)
	d2.pushHead(val)
}

len(d.vals) return type is int which on 32 bit system is 4 bytes. When d.vals reachs the max size 2^30, then newSize will be 2^31. But 2^31 is greater than the max int32 2^31 -1 , and if condition will test fail .

newSize := len(d.vals) * 2
@bcmills
Copy link
Contributor

bcmills commented Feb 23, 2021

Neat find!

Fortunately, I suspect your program would always crash due to address space exhaustion before you can actually hit the overflow. The elements in d.vals have type eface, which on a 32-bit platform is 8 bytes. A slice of 230 8-byte elements would be 233 bytes consumed by the slice's backing array, but since we're assuming a 32-bit platform we only have 232 bytes of address space to work with.

So AFAICT this overflow is only possible on platforms with a 32-bit int but an address space much larger than 32 bits, and to my knowledge we have no such platform today.

That said, that rationale should at least merit a comment in the code...

@cagedmantis cagedmantis changed the title sync/poolqueue.go dequeue newSize may be overflow on 32bit system sync: dequeue newSize may be overflow on 32bit system Feb 23, 2021
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 23, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Feb 23, 2021
@cagedmantis
Copy link
Contributor

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: Triage Backlog
Development

No branches or pull requests

4 participants